diff --git a/src/Core/Lock/FlockLock.php b/src/Core/Lock/FlockLock.php index 28cf566c134..eedfbb3f17b 100644 --- a/src/Core/Lock/FlockLock.php +++ b/src/Core/Lock/FlockLock.php @@ -42,16 +42,32 @@ class FlockLock implements LockInterface */ private $handle; + /** + * @var bool If true, we should acquire an exclusive lock. + */ + private $exclusive; + /** * @param string $fileName The name of the file to use as a lock. + * @param array $options [optional] { + * Configuration options. + * + * @type bool $exclusive If true, acquire an excluse (write) lock. If + * false, acquire a shared (read) lock. **Defaults to** true. + * } * @throws \InvalidArgumentException If an invalid fileName is provided. */ - public function __construct($fileName) + public function __construct($fileName, array $options = []) { if (!is_string($fileName)) { throw new \InvalidArgumentException('$fileName must be a string.'); } + $options += [ + 'exclusive' => true + ]; + $this->exclusive = $options['exclusive']; + $this->filePath = sprintf( self::FILE_PATH_TEMPLATE, sys_get_temp_dir(), @@ -62,10 +78,16 @@ public function __construct($fileName) /** * Acquires a lock that will block until released. * + * @param array $options [optional] { + * Configuration options. + * + * @type bool $blocking Whether the process should block while waiting + * to acquire the lock. **Defaults to** true. + * } * @return bool * @throws \RuntimeException If the lock fails to be acquired. */ - public function acquire() + public function acquire(array $options = []) { if ($this->handle) { return true; @@ -73,7 +95,7 @@ public function acquire() $this->handle = $this->initializeHandle(); - if (!flock($this->handle, LOCK_EX)) { + if (!flock($this->handle, $this->lockType($options))) { fclose($this->handle); $this->handle = null; @@ -117,4 +139,16 @@ private function initializeHandle() return $handle; } + + private function lockType(array $options) + { + $options += [ + 'blocking' => true + ]; + $lockType = $this->exclusive ? LOCK_EX : LOCK_SH; + if (!$options['blocking']) { + $lockType = $lockType | LOCK_UN; + } + return $lockType; + } } diff --git a/src/Core/Lock/LockInterface.php b/src/Core/Lock/LockInterface.php index b328de036a9..1039362cfea 100644 --- a/src/Core/Lock/LockInterface.php +++ b/src/Core/Lock/LockInterface.php @@ -23,12 +23,18 @@ interface LockInterface { /** - * Acquires a lock that will block until released. + * Acquires a lock. * + * @param array $options [optional] { + * Configuration options. + * + * @type bool $blocking Whether the process should block while waiting + * to acquire the lock. **Defaults to** true. + * } * @return bool - * @throws \RuntimeException + * @throws \RuntimeException If the lock fails to be acquired. */ - public function acquire(); + public function acquire(array $options = []); /** * Releases the lock. @@ -41,7 +47,13 @@ public function release(); * Execute a callable within a lock. * * @param callable $func The callable to execute. + * @param array $options [optional] { + * Configuration options. + * + * @type bool $blocking Whether the process should block while waiting + * to acquire the lock. **Defaults to** true. + * } * @return mixed */ - public function synchronize(callable $func); + public function synchronize(callable $func, array $options = []); } diff --git a/src/Core/Lock/LockTrait.php b/src/Core/Lock/LockTrait.php index e20a0fda9f3..dfa5add3a7b 100644 --- a/src/Core/Lock/LockTrait.php +++ b/src/Core/Lock/LockTrait.php @@ -25,10 +25,16 @@ trait LockTrait /** * Acquires a lock that will block until released. * + * @param array $options [optional] { + * Configuration options. + * + * @type bool $blocking Whether the process should block while waiting + * to acquire the lock. **Defaults to** true. + * } * @return bool - * @throws \RuntimeException + * @throws \RuntimeException If the lock fails to be acquired. */ - abstract public function acquire(); + abstract public function acquire(array $options = []); /** * Releases the lock. @@ -43,14 +49,20 @@ abstract public function release(); * it. * * @param callable $func The callable to execute. + * @param array $options [optional] { + * Configuration options. + * + * @type bool $blocking Whether the process should block while waiting + * to acquire the lock. **Defaults to** true. + * } * @return mixed */ - public function synchronize(callable $func) + public function synchronize(callable $func, array $options = []) { $result = null; $exception = null; - if ($this->acquire()) { + if ($this->acquire($options)) { try { $result = $func(); } catch (\Exception $ex) { diff --git a/src/Core/Lock/SemaphoreLock.php b/src/Core/Lock/SemaphoreLock.php index c960cc9b09a..f97d4b209df 100644 --- a/src/Core/Lock/SemaphoreLock.php +++ b/src/Core/Lock/SemaphoreLock.php @@ -64,18 +64,28 @@ public function __construct($key) /** * Acquires a lock that will block until released. * + * @param array $options [optional] { + * Configuration options. + * + * @type bool $blocking Whether the process should block while waiting + * to acquire the lock. **Defaults to** true. + * } * @return bool * @throws \RuntimeException If the lock fails to be acquired. */ - public function acquire() + public function acquire(array $options = []) { + $options += [ + 'blocking' => true + ]; + if ($this->semaphoreId) { return true; } $this->semaphoreId = $this->initializeId(); - if (!sem_acquire($this->semaphoreId)) { + if (!sem_acquire($this->semaphoreId, !$options['blocking'])) { $this->semaphoreId = null; throw new \RuntimeException('Failed to acquire lock.'); diff --git a/src/Core/Lock/SymfonyLockAdapter.php b/src/Core/Lock/SymfonyLockAdapter.php index 2fc761a9eab..7736b8bb9a2 100644 --- a/src/Core/Lock/SymfonyLockAdapter.php +++ b/src/Core/Lock/SymfonyLockAdapter.php @@ -42,13 +42,23 @@ public function __construct(SymfonyLockInterface $lock) /** * Acquires a lock that will block until released. * + * @param array $options [optional] { + * Configuration options. + * + * @type bool $blocking Whether the process should block while waiting + * to acquire the lock. **Defaults to** true. + * } * @return bool - * @throws \RuntimeException + * @throws \RuntimeException If the lock fails to be acquired. */ - public function acquire() + public function acquire(array $options = []) { + $options += [ + 'blocking' => true + ]; + try { - return $this->lock->acquire(true); + return $this->lock->acquire($options['blocking']); } catch (\Exception $ex) { throw new \RuntimeException($ex->getMessage()); } diff --git a/src/Debugger/Agent.php b/src/Debugger/Agent.php index 1d1bef8fff2..d277e4ff4ad 100644 --- a/src/Debugger/Agent.php +++ b/src/Debugger/Agent.php @@ -19,7 +19,9 @@ use Google\Cloud\Core\Batch\BatchRunner; use Google\Cloud\Core\Batch\BatchTrait; +use Google\Cloud\Core\SysvTrait; use Google\Cloud\Debugger\BreakpointStorage\BreakpointStorageInterface; +use Google\Cloud\Debugger\BreakpointStorage\FileBreakpointStorage; use Google\Cloud\Debugger\BreakpointStorage\SysvBreakpointStorage; use Google\Cloud\Logging\LoggingClient; use Psr\Log\LoggerInterface; @@ -39,6 +41,7 @@ class Agent { use BatchTrait; + use SysvTrait; /** * @var Debuggee @@ -190,7 +193,9 @@ protected function getCallback() private function defaultStorage() { - return new SysvBreakpointStorage(); + return $this->isSysvIPCLoaded() + ? new SysvBreakpointStorage() + : new FileBreakpointStorage(); } private function defaultDebuggee() diff --git a/src/Debugger/BreakpointStorage/FileBreakpointStorage.php b/src/Debugger/BreakpointStorage/FileBreakpointStorage.php new file mode 100644 index 00000000000..03428675a5b --- /dev/null +++ b/src/Debugger/BreakpointStorage/FileBreakpointStorage.php @@ -0,0 +1,113 @@ +filename = implode(DIRECTORY_SEPARATOR, [sys_get_temp_dir(), $filename]); + $this->lockFilename = $filename . '.lock'; + } + + /** + * Save the given debugger breakpoints. + * + * @param Debuggee $debuggee + * @param Breakpoint[] $breakpoints + * @return bool + */ + public function save(Debuggee $debuggee, array $breakpoints) + { + $data = [ + 'debuggeeId' => $debuggee->id(), + 'breakpoints' => $breakpoints + ]; + + $success = false; + + // Acquire an exclusive write lock (blocking). There should only be a + // single Daemon that can call this. + try { + $success = $this->getLock(true)->synchronize(function () use ($data) { + return file_put_contents($this->filename, serialize($data)) !== false; + }); + } catch (\RuntimeException $e) { + // Do nothing + } + return $success; + } + + /** + * Load debugger breakpoints from the storage. Returns a 2-arity array + * with the debuggee id and the list of breakpoints. + * + * @return array + */ + public function load() + { + $debuggeeId = null; + $breakpoints = []; + + // Acquire a read lock (non-blocking). If we fail (file is locked + // for writing), then we return an empty list of breakpoints and + // skip debugging for this request. + try { + $contents = $this->getLock()->synchronize(function () { + return file_get_contents($this->filename); + }, [ + 'blocking' => false + ]); + $data = unserialize($contents); + $debuggeeId = $data['debuggeeId']; + $breakpoints = $data['breakpoints']; + } catch (\RuntimeException $e) { + // Do nothing + } + return [$debuggeeId, $breakpoints]; + } + + private function getLock($exclusive = false) + { + return new FlockLock($this->lockFilename, [ + 'exclusive' => $exclusive + ]); + } +} diff --git a/src/Debugger/BreakpointStorage/SysvBreakpointStorage.php b/src/Debugger/BreakpointStorage/SysvBreakpointStorage.php index d88b9e1a3f3..6822d24d883 100644 --- a/src/Debugger/BreakpointStorage/SysvBreakpointStorage.php +++ b/src/Debugger/BreakpointStorage/SysvBreakpointStorage.php @@ -80,7 +80,7 @@ public function load() ); } if (!shm_has_var($shmid, self::VAR_KEY)) { - $result = []; + $result = [null, []]; } else { $result = shm_get_var($shmid, self::VAR_KEY); } diff --git a/src/Debugger/Daemon.php b/src/Debugger/Daemon.php index 326733ecb56..87f92775aad 100644 --- a/src/Debugger/Daemon.php +++ b/src/Debugger/Daemon.php @@ -17,10 +17,12 @@ namespace Google\Cloud\Debugger; +use Google\Cloud\Core\SysvTrait; use Google\Cloud\Core\Report\MetadataProviderInterface; use Google\Cloud\Core\Report\MetadataProviderUtils; use Google\Cloud\Core\Exception\ConflictException; use Google\Cloud\Debugger\BreakpointStorage\BreakpointStorageInterface; +use Google\Cloud\Debugger\BreakpointStorage\FileBreakpointStorage; use Google\Cloud\Debugger\BreakpointStorage\SysvBreakpointStorage; /** @@ -39,6 +41,8 @@ */ class Daemon { + use SysvTrait; + /** * @var Debuggee */ @@ -122,7 +126,7 @@ public function __construct($sourceRoot, array $options = []) $this->storage = array_key_exists('storage', $options) ? $options['storage'] - : new SysvBreakpointStorage(); + : $this->defaultStorage(); } /** @@ -222,4 +226,11 @@ private function defaultLabels(MetadataProviderInterface $metadataProvider = nul } return $labels; } + + private function defaultStorage() + { + return $this->isSysvIPCLoaded() + ? new SysvBreakpointStorage() + : new FileBreakpointStorage(); + } } diff --git a/tests/unit/Debugger/BreakpointStorage/FileBreakpointStorageTest.php b/tests/unit/Debugger/BreakpointStorage/FileBreakpointStorageTest.php new file mode 100644 index 00000000000..44fc726eede --- /dev/null +++ b/tests/unit/Debugger/BreakpointStorage/FileBreakpointStorageTest.php @@ -0,0 +1,48 @@ +storage = new FileBreakpointStorage(); + } + + public function testSaveAndLoad() + { + $connection = $this->prophesize(ConnectionInterface::class); + $debuggee = new Debuggee($connection->reveal(), ['id' => 'debuggee1', 'project' => 'project1']); + $breakpoints = [ + new Breakpoint(['id' => 'breakpoint1']) + ]; + $this->storage->save($debuggee, $breakpoints); + $this->assertEquals(['debuggee1', $breakpoints], $this->storage->load()); + } +} diff --git a/tests/unit/Debugger/BreakpointStorage/SysvBreakpointStorageTest.php b/tests/unit/Debugger/BreakpointStorage/SysvBreakpointStorageTest.php index 59f0114de4e..fc9220f92be 100644 --- a/tests/unit/Debugger/BreakpointStorage/SysvBreakpointStorageTest.php +++ b/tests/unit/Debugger/BreakpointStorage/SysvBreakpointStorageTest.php @@ -35,7 +35,7 @@ class SysvBreakpointStorageTest extends TestCase public function setUp() { - if (! $this->isSysvIPCLOaded()) { + if (!$this->isSysvIPCLoaded()) { $this->markTestSkipped( 'Skipping because SystemV IPC extensions are not loaded'); }