From 7678f412be22d6b26ebff847fd752f4fe8a65953 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 20 Oct 2016 11:36:31 -0700
Subject: [PATCH] Hold a lock while collecting garbage

Summary:
Fixes T11771. Adds a lock around each GC process so we don't try to, e.g., delete old files on two machines at once just because they're both running trigger daemons.

The other aspects of this daemon (actual triggers; nuance importers) already have separate locks.

Test Plan: Ran `bin/phd debug trigger --trace`, saw daemon acquire locks and collect garbage.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11771

Differential Revision: https://secure.phabricator.com/D16739
---
 .../PhabricatorGarbageCollector.php           | 22 ++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/src/infrastructure/daemon/garbagecollector/PhabricatorGarbageCollector.php b/src/infrastructure/daemon/garbagecollector/PhabricatorGarbageCollector.php
index 3e5b70cc05..f5960e9504 100644
--- a/src/infrastructure/daemon/garbagecollector/PhabricatorGarbageCollector.php
+++ b/src/infrastructure/daemon/garbagecollector/PhabricatorGarbageCollector.php
@@ -98,7 +98,27 @@ abstract class PhabricatorGarbageCollector extends Phobject {
       }
     }
 
-    return $this->collectGarbage();
+    // Hold a lock while performing collection to avoid racing other daemons
+    // running the same collectors.
+    $lock_name = 'gc:'.$this->getCollectorConstant();
+    $lock = PhabricatorGlobalLock::newLock($lock_name);
+
+    try {
+      $lock->lock(5);
+    } catch (PhutilLockException $ex) {
+      return false;
+    }
+
+    try {
+      $result = $this->collectGarbage();
+    } catch (Exception $ex) {
+      $lock->unlock();
+      throw $ex;
+    }
+
+    $lock->unlock();
+
+    return $result;
   }