diff --git a/src/applications/diffusion/protocol/DiffusionRepositoryClusterEngine.php b/src/applications/diffusion/protocol/DiffusionRepositoryClusterEngine.php --- a/src/applications/diffusion/protocol/DiffusionRepositoryClusterEngine.php +++ b/src/applications/diffusion/protocol/DiffusionRepositoryClusterEngine.php @@ -311,7 +311,7 @@ $write_lock = PhabricatorRepositoryWorkingCopyVersion::getWriteLock( $repository_phid); - $write_lock->useSpecificConnection($locked_connection); + $write_lock->setExternalConnection($locked_connection); $this->logLine( pht( diff --git a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementWorkflow.php b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementWorkflow.php --- a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementWorkflow.php +++ b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementWorkflow.php @@ -1229,7 +1229,7 @@ // log table yet, or may need to adjust it. return PhabricatorGlobalLock::newLock('adjust', $parameters) - ->useSpecificConnection($api->getConn(null)) + ->setExternalConnection($api->getConn(null)) ->setDisableLogging(true) ->lock(); } diff --git a/src/infrastructure/util/PhabricatorGlobalLock.php b/src/infrastructure/util/PhabricatorGlobalLock.php --- a/src/infrastructure/util/PhabricatorGlobalLock.php +++ b/src/infrastructure/util/PhabricatorGlobalLock.php @@ -91,8 +91,13 @@ * @param AphrontDatabaseConnection * @return this */ - public function useSpecificConnection(AphrontDatabaseConnection $conn) { - $this->conn = $conn; + public function setExternalConnection(AphrontDatabaseConnection $conn) { + if ($this->conn) { + throw new Exception( + pht( + 'Lock is already held, and must be released before the '. + 'connection may be changed.')); + } $this->externalConnection = $conn; return $this; } @@ -118,6 +123,12 @@ protected function doLock($wait) { $conn = $this->conn; + if (!$conn) { + if ($this->externalConnection) { + $conn = $this->externalConnection; + } + } + if (!$conn) { // Try to reuse a connection from the connection pool. $conn = array_pop(self::$pool); diff --git a/src/infrastructure/util/__tests__/PhabricatorGlobalLockTestCase.php b/src/infrastructure/util/__tests__/PhabricatorGlobalLockTestCase.php --- a/src/infrastructure/util/__tests__/PhabricatorGlobalLockTestCase.php +++ b/src/infrastructure/util/__tests__/PhabricatorGlobalLockTestCase.php @@ -54,7 +54,7 @@ $lock_name = $this->newLockName(); $lock = PhabricatorGlobalLock::newLock($lock_name); - $lock->useSpecificConnection($conn); + $lock->setExternalConnection($conn); $lock->lock(); $this->assertEqual( @@ -85,6 +85,30 @@ PhabricatorGlobalLock::clearConnectionPool(); } + public function testExternalConnectionMutationScope() { + $conn = id(new HarbormasterScratchTable()) + ->establishConnection('w'); + + $lock_name = $this->newLockName(); + $lock = PhabricatorGlobalLock::newLock($lock_name); + $lock->lock(); + + $caught = null; + try { + $lock->setExternalConnection($conn); + } catch (Exception $ex) { + $caught = $ex; + } catch (Throwable $ex) { + $caught = $ex; + } + + $lock->unlock(); + + $this->assertTrue( + ($caught instanceof Exception), + pht('Changing connection while locked is forbidden.')); + } + private function newLockName() { return 'testlock-'.Filesystem::readRandomCharacters(16); }