Loading Conpherence handles may recurse indefinitely
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	epriestley
	Jun 8 2015, 9:31 PM

Description

I thought I put a guard on this but seems like I missed something.

#0 /core/lib/phabricator/src/infrastructure/customfield/field/PhabricatorCustomField.php(51): __phutil_signal_handler__(1)
#1 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php(888): PhabricatorCustomField::getObjectFields(Object(PhabricatorUser), 'ApplicationSear...')
#2 /core/lib/phabricator/src/applications/people/query/PhabricatorPeopleQuery.php(365): PhabricatorCursorPagedPolicyAwareQuery->getOrderableColumns()
#3 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php(919): PhabricatorPeopleQuery->getOrderableColumns()
#4 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php(97): PhabricatorCursorPagedPolicyAwareQuery->buildOrderClause(Object(AphrontMySQLiDatabaseConnection))
#5 /core/lib/phabricator/src/applications/people/query/PhabricatorPeopleQuery.php(122): PhabricatorCursorPagedPolicyAwareQuery->loadStandardPageRows(Object(PhabricatorUser))
#6 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(228): PhabricatorPeopleQuery->loadPage()
#7 /core/lib/phabricator/src/applications/phid/type/PhabricatorPHIDType.php(106): PhabricatorPolicyAwareQuery->execute()
#8 /core/lib/phabricator/src/applications/phid/query/PhabricatorObjectQuery.php(141): PhabricatorPHIDType->loadObjects(Object(PhabricatorObjectQuery), Array)
#9 /core/lib/phabricator/src/applications/phid/query/PhabricatorObjectQuery.php(63): PhabricatorObjectQuery->loadObjectsByPHID(Array, Array)
#10 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(228): PhabricatorObjectQuery->loadPage()
#11 /core/lib/phabricator/src/applications/phid/query/PhabricatorHandleQuery.php(39): PhabricatorPolicyAwareQuery->execute()
#12 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(228): PhabricatorHandleQuery->loadPage()
#13 /core/lib/phabricator/src/applications/phid/handle/pool/PhabricatorHandlePool.php(73): PhabricatorPolicyAwareQuery->execute()
#14 /core/lib/phabricator/src/applications/phid/handle/pool/PhabricatorHandleList.php(44): PhabricatorHandlePool->loadPHIDs(Array)
#15 /core/lib/phabricator/src/applications/phid/handle/pool/PhabricatorHandleList.php(49): PhabricatorHandleList->loadHandles()
#16 /core/lib/phabricator/src/applications/phid/handle/pool/PhabricatorHandleList.php(112): PhabricatorHandleList->getHandle('PHID-USER-pouay...')
#17 [internal function]: PhabricatorHandleList->current()
#18 /core/lib/phabricator/src/applications/conpherence/query/ConpherenceThreadQuery.php(290): iterator_to_array(Object(PhabricatorHandleList))
#19 /core/lib/phabricator/src/applications/conpherence/query/ConpherenceThreadQuery.php(123): ConpherenceThreadQuery->loadCoreHandles(Array, 'getRecentPartic...')
#20 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(228): ConpherenceThreadQuery->loadPage()
#21 /core/lib/phabricator/src/applications/phid/type/PhabricatorPHIDType.php(106): PhabricatorPolicyAwareQuery->execute()
#22 /core/lib/phabricator/src/applications/phid/query/PhabricatorObjectQuery.php(141): PhabricatorPHIDType->loadObjects(Object(PhabricatorObjectQuery), Array)
#23 /core/lib/phabricator/src/applications/phid/query/PhabricatorObjectQuery.php(63): PhabricatorObjectQuery->loadObjectsByPHID(Array, Array)
#24 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(228): PhabricatorObjectQuery->loadPage()
#25 /core/lib/phabricator/src/applications/files/query/PhabricatorFileQuery.php(180): PhabricatorPolicyAwareQuery->execute()
#26 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(228): PhabricatorFileQuery->loadPage()
#27 /core/lib/phabricator/src/applications/people/query/PhabricatorPeopleQuery.php(183): PhabricatorPolicyAwareQuery->execute()
#28 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(261): PhabricatorPeopleQuery->didFilterPage(Array)
#29 /core/lib/phabricator/src/applications/phid/type/PhabricatorPHIDType.php(106): PhabricatorPolicyAwareQuery->execute()
#30 /core/lib/phabricator/src/applications/phid/query/PhabricatorObjectQuery.php(141): PhabricatorPHIDType->loadObjects(Object(PhabricatorObjectQuery), Array)
#31 /core/lib/phabricator/src/applications/phid/query/PhabricatorObjectQuery.php(63): PhabricatorObjectQuery->loadObjectsByPHID(Array, Array)
#32 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(228): PhabricatorObjectQuery->loadPage()
#33 /core/lib/phabricator/src/applications/phid/query/PhabricatorHandleQuery.php(39): PhabricatorPolicyAwareQuery->execute()
#34 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(228): PhabricatorHandleQuery->loadPage()
#35 /core/lib/phabricator/src/applications/phid/handle/pool/PhabricatorHandlePool.php(73): PhabricatorPolicyAwareQuery->execute()
#36 /core/lib/phabricator/src/applications/phid/handle/pool/PhabricatorHandleList.php(44): PhabricatorHandlePool->loadPHIDs(Array)
#37 /core/lib/phabricator/src/applications/phid/handle/pool/PhabricatorHandleList.php(49): PhabricatorHandleList->loadHandles()
#38 /core/lib/phabricator/src/applications/phid/handle/pool/PhabricatorHandleList.php(112): PhabricatorHandleList->getHandle('PHID-USER-pouay...')
#39 [internal function]: PhabricatorHandleList->current()
#40 /core/lib/phabricator/src/applications/conpherence/query/ConpherenceThreadQuery.php(290): iterator_to_array(Object(PhabricatorHandleList))
#41 /core/lib/phabricator/src/applications/conpherence/query/ConpherenceThreadQuery.php(123): ConpherenceThreadQuery->loadCoreHandles(Array, 'getRecentPartic...')
#42 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(228): ConpherenceThreadQuery->loadPage()
#43 /core/lib/phabricator/src/applications/phid/type/PhabricatorPHIDType.php(106): PhabricatorPolicyAwareQuery->execute()
#44 /core/lib/phabricator/src/applications/phid/query/PhabricatorObjectQuery.php(141): PhabricatorPHIDType->loadObjects(Object(PhabricatorObjectQuery), Array)
#45 /core/lib/phabricator/src/applications/phid/query/PhabricatorObjectQuery.php(63): PhabricatorObjectQuery->loadObjectsByPHID(Array, Array)
#46 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(228): PhabricatorObjectQuery->loadPage()
#47 /core/lib/phabricator/src/applications/files/query/PhabricatorFileQuery.php(180): PhabricatorPolicyAwareQuery->execute()
#48 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(228): PhabricatorFileQuery->loadPage()
#49 /core/lib/phabricator/src/applications/people/query/PhabricatorPeopleQuery.php(183): PhabricatorPolicyAwareQuery->execute()
#50 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(261): PhabricatorPeopleQuery->didFilterPage(Array)
#51 /core/lib/phabricator/src/applications/phid/type/PhabricatorPHIDType.php(106): PhabricatorPolicyAwareQuery->execute()
#52 /core/lib/phabricator/src/applications/phid/query/PhabricatorObjectQuery.php(141): PhabricatorPHIDType->loadObjects(Object(PhabricatorObjectQuery), Array)
#53 /core/lib/phabricator/src/applications/phid/query/PhabricatorObjectQuery.php(63): PhabricatorObjectQuery->loadObjectsByPHID(Array, Array)
#54 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(228): PhabricatorObjectQuery->loadPage()
#55 /core/lib/phabricator/src/applications/phid/query/PhabricatorHandleQuery.php(39): PhabricatorPolicyAwareQuery->execute()
#56 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(228): PhabricatorHandleQuery->loadPage()
#57 /core/lib/phabricator/src/applications/phid/handle/pool/PhabricatorHandlePool.php(73): PhabricatorPolicyAwareQuery->execute()
#58 /core/lib/phabricator/src/applications/phid/handle/pool/PhabricatorHandleList.php(44): PhabricatorHandlePool->loadPHIDs(Array)
#59 /core/lib/phabricator/src/applications/phid/handle/pool/PhabricatorHandleList.php(49): PhabricatorHandleList->loadHandles()
#60 /core/lib/phabricator/src/applications/phid/handle/pool/PhabricatorHandleList.php(112): PhabricatorHandleList->getHandle('PHID-USER-pouay...')
#61 [internal function]: PhabricatorHandleList->current()
#62 /core/lib/phabricator/src/applications/conpherence/query/ConpherenceThreadQuery.php(290): iterator_to_array(Object(PhabricatorHandleList))
#63 /core/lib/phabricator/src/applications/conpherence/query/ConpherenceThreadQuery.php(123): ConpherenceThreadQuery->loadCoreHandles(Array, 'getRecentPartic...')
#64 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(228): ConpherenceThreadQuery->loadPage()
#65 /core/lib/phabricator/src/applications/phid/type/PhabricatorPHIDType.php(106): PhabricatorPolicyAwareQuery->execute()
#66 /core/lib/phabricator/src/applications/phid/query/PhabricatorObjectQuery.php(141): PhabricatorPHIDType->loadObjects(Object(PhabricatorObjectQuery), Array)
#67 /core/lib/phabricator/src/applications/phid/query/PhabricatorObjectQuery.php(63): PhabricatorObjectQuery->loadObjectsByPHID(Array, Array)
#68 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(228): PhabricatorObjectQuery->loadPage()
#69 /core/lib/phabricator/src/applications/files/query/PhabricatorFileQuery.php(180): PhabricatorPolicyAwareQuery->execute()
#70 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(228): PhabricatorFileQuery->loadPage()
#71 /core/lib/phabricator/src/applications/people/query/PhabricatorPeopleQuery.php(183): PhabricatorPolicyAwareQuery->execute()
#72 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(261): PhabricatorPeopleQuery->didFilterPage(Array)
#73 /core/lib/phabricator/src/applications/phid/type/PhabricatorPHIDType.php(106): PhabricatorPolicyAwareQuery->execute()
#74 /core/lib/phabricator/src/applications/phid/query/PhabricatorObjectQuery.php(141): PhabricatorPHIDType->loadObjects(Object(PhabricatorObjectQuery), Array)
#75 /core/lib/phabricator/src/applications/phid/query/PhabricatorObjectQuery.php(63): PhabricatorObjectQuery->loadObjectsByPHID(Array, Array)
#76 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(228): PhabricatorObjectQuery->loadPage()
#77 /core/lib/phabricator/src/applications/phid/query/PhabricatorHandleQuery.php(39): PhabricatorPolicyAwareQuery->execute()
#78 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(228): PhabricatorHandleQuery->loadPage()
#79 /core/lib/phabricator/src/applications/phid/handle/pool/PhabricatorHandlePool.php(73): PhabricatorPolicyAwareQuery->execute()
#80 /core/lib/phabricator/src/applications/phid/handle/pool/PhabricatorHandleList.php(44): PhabricatorHandlePool->loadPHIDs(Array)
#81 /core/lib/phabricator/src/applications/phid/handle/pool/PhabricatorHandleList.php(49): PhabricatorHandleList->loadHandles()
#82 /core/lib/phabricator/src/applications/phid/handle/pool/PhabricatorHandleList.php(112): PhabricatorHandleList->getHandle('PHID-USER-pouay...')
#83 [internal function]: PhabricatorHandleList->current()
#84 /core/lib/phabricator/src/applications/conpherence/query/ConpherenceThreadQuery.php(290): iterator_to_array(Object(PhabricatorHandleList))
#85 /core/lib/phabricator/src/applications/conpherence/query/ConpherenceThreadQuery.php(123): ConpherenceThreadQuery->loadCoreHandles(Array, 'getRecentPartic...')
#86 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(228): ConpherenceThreadQuery->loadPage()
#87 /core/lib/phabricator/src/applications/phid/type/PhabricatorPHIDType.php(106): PhabricatorPolicyAwareQuery->execute()
#88 /core/lib/phabricator/src/applications/phid/query/PhabricatorObjectQuery.php(141): PhabricatorPHIDType->loadObjects(Object(PhabricatorObjectQuery), Array)
#89 /core/lib/phabricator/src/applications/phid/query/PhabricatorObjectQuery.php(63): PhabricatorObjectQuery->loadObjectsByPHID(Array, Array)
#90 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(228): PhabricatorObjectQuery->loadPage()
#91 /core/lib/phabricator/src/applications/files/query/PhabricatorFileQuery.php(180): PhabricatorPolicyAwareQuery->execute()
#92 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(228): PhabricatorFileQuery->loadPage()
#93 /core/lib/phabricator/src/applications/people/query/PhabricatorPeopleQuery.php(183): PhabricatorPolicyAwareQuery->execute()
#94 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(261): PhabricatorPeopleQuery->didFilterPage(Array)
#95 /core/lib/phabricator/src/applications/phid/type/PhabricatorPHIDType.php(106): PhabricatorPolicyAwareQuery->execute()
#96 /core/lib/phabricator/src/applications/phid/query/PhabricatorObjectQuery.php(141): PhabricatorPHIDType->loadObjects(Object(PhabricatorObjectQuery), Array)
#97 /core/lib/phabricator/src/applications/phid/query/PhabricatorObjectQuery.php(63): PhabricatorObjectQuery->loadObjectsByPHID(Array, Array)
#98 /core/lib/phabricator/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php(228): PhabricatorObjectQuery->loadPage()
#99 /core/lib/phabricator/src/applications/phid/query/PhabricatorHandleQuery.php(39): PhabricatorPolicyAwareQuery->execute()

Revisions and Commits

rP Phabricator
	D13220	rP5397779ee248 Try using a less-rich but less-risky thread title in Conpherence handles
	D13216	rP7f5919de1c1c Don't load attached files for profile images

Related Objects

Mentioned Here: D13216: Don't load attached files for profile images

Event Timeline

epriestley created this task.Jun 8 2015, 9:31 PM

epriestley raised the priority of this task from to Normal.

epriestley updated the task description. (Show Details)

epriestley added a subscriber: epriestley.

epriestley added a revision: D13216: Don't load attached files for profile images.Jun 8 2015, 9:56 PM

epriestley updated the task description. (Show Details)Jun 8 2015, 9:56 PM

@btrahan, per D13216, I'd ideally like to stop this class of problem by getting rid of the need for Conpherence threads to load other handles in order to load their own handles (I believe they're the only case where we do that) but that seems a bit tricky since we need them to generate the titles of some threads.

A possible compromise would be to use a static title for untitled threads, like "Private Correspondence", so you'd see Z123: Private Correspondence for the {Z123} form of a thread with no explicit title. This seems maybe-reasonable and not too messy to me, from a product perspective -- does it seem OK to you? Or do you have any other ideas, or plans to use thread handles in the future in a way that would make this messier? (Might be a little messy from a technical perspective, of course.)

static title for untitled threads

Specifically, we'd only do this for handles, not in the normal UI. So all the UI would work as it does now, we just wouldn't try as hard when using handles.

epriestley added a commit: rP7f5919de1c1c: Don't load attached files for profile images.Jun 8 2015, 10:39 PM

That sounds fine to me, but I think I iterated to @chad's dream state... I actually think we used to have "(No Title)" or something for a time...

We could also require a title and have it defaulted to something like the "(No Title)"?

I think the current state is good and would want to keep it in all the non-handle UIs (menu, Conpherence, column, etc.). I'm pretty sure the only places you actually see a Conpherence handle right now are:

If you explicitly {Z123} in remarkup.
Files > Attached, if you have a file attached to a thread.
Transaction on tasks, etc., if the object was mentioned in the thread.

So I'd only change those three cases, and only if the thread has no title.

This is a little inconsistent, but feels so un-invasive that I'm not sure anyone would ever notice/care. But maybe I'm forgetting more places where we use them or soemthing like that.

I think I can cheat it in in like 3 lines of code so maybe we can try it and see if it sticks out.

epriestley added a revision: D13220: Try using a less-rich but less-risky thread title in Conpherence handles.Jun 9 2015, 1:38 AM

epriestley added a commit: rP5397779ee248: Try using a less-rich but less-risky thread title in Conpherence handles.Jun 9 2015, 12:50 PM

I'm tentatively going to close this out, but yell if the "Private Correspondence" thing sucks and we can find another approach or find ways to refine it. I sleep better at night knowing handles aren't loading other handles, though.

But how will you get SEV0 bugs if you don't dabble in infinite recursion?

I'll find a way!

Loading Conpherence handles may recurse indefinitelyClosed, ResolvedPublicActions

Description

Revisions and Commits

Related Objects

Event Timeline

Loading Conpherence handles may recurse indefinitely
Closed, ResolvedPublic
Actions