From 7f5919de1c1c22077bb67e9dd838dfac54256554 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 8 Jun 2015 15:39:47 -0700 Subject: [PATCH] Don't load attached files for profile images Summary: Ref T8478. I think the cycle is: - Conpherence Thread > Loads handle for participant > loads file for profile image > loads attached files to check visibility > loads conpherence thread > ... So, specifically, someone attached their profile image to a thread or message somewhere. This breaks the cycle by stopping the attached-files visibility check from happening, since we don't need it. This seemed like the easiest link in the chain to break. //Ideally//, I think the longer-term and more complete fix here is to stop Conpherence from requiring handles in order to load thread handles (and, generally, having a "handles must not load other handles" rule), but that's not trivial and might not be especially practical. Test Plan: Will test in production. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T8478 Differential Revision: https://secure.phabricator.com/D13216 --- src/applications/files/query/PhabricatorFileQuery.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/applications/files/query/PhabricatorFileQuery.php b/src/applications/files/query/PhabricatorFileQuery.php index ad13908d7d..ba72156061 100644 --- a/src/applications/files/query/PhabricatorFileQuery.php +++ b/src/applications/files/query/PhabricatorFileQuery.php @@ -142,6 +142,14 @@ final class PhabricatorFileQuery foreach ($files as $file) { $phids = array_keys($edges[$file->getPHID()][$edge_type]); $file->attachObjectPHIDs($phids); + + if ($file->getIsProfileImage()) { + // If this is a profile image, don't bother loading related files. + // It will always be visible, and we can get into trouble if we try + // to load objects and end up stuck in a cycle. See T8478. + continue; + } + foreach ($phids as $phid) { $object_phids[$phid] = true; }