From 67ececf6d9a0871203f3792b227206f0e4c1c337 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Thu, 30 Nov 2023 16:00:59 +0000 Subject: [PATCH] Remove cache dependency --- services.yaml | 8 +++- src/Service/Api.php | 67 ++++------------------------ src/Service/FileLock.php | 92 +++++++++++++++++++++++++++++++++++++++ src/SshCert/Certifier.php | 9 ++-- 4 files changed, 112 insertions(+), 64 deletions(-) create mode 100644 src/Service/FileLock.php diff --git a/services.yaml b/services.yaml index b6021c3674..880f124271 100644 --- a/services.yaml +++ b/services.yaml @@ -26,7 +26,7 @@ services: api: class: '\Platformsh\Cli\Service\Api' - arguments: ['@config', '@cache', '@output', '@token_config'] + arguments: ['@config', '@cache', '@output', '@token_config', '@file_lock'] app_finder: class: '\Platformsh\Cli\Local\ApplicationFinder' @@ -43,7 +43,7 @@ services: certifier: class: '\Platformsh\Cli\SshCert\Certifier' - arguments: ['@api', '@config', '@shell', '@fs', '@output'] + arguments: ['@api', '@config', '@shell', '@fs', '@output', '@file_lock'] config: class: '\Platformsh\Cli\Service\Config' @@ -56,6 +56,10 @@ services: class: '\Platformsh\Cli\Service\Drush' arguments: ['@config', '@shell', '@local.project', '@api', '@app_finder'] + file_lock: + class: '\Platformsh\Cli\Service\FileLock' + arguments: ['@config'] + fs: class: '\Platformsh\Cli\Service\Filesystem' arguments: ['@shell'] diff --git a/src/Service/Api.php b/src/Service/Api.php index 7d7a761082..3279d1b3b8 100644 --- a/src/Service/Api.php +++ b/src/Service/Api.php @@ -64,6 +64,9 @@ class Api /** @var TokenConfig */ private $tokenConfig; + /** @var FileLock */ + private $fileLock; + /** * The library's API client object. * @@ -138,18 +141,21 @@ class Api * @param OutputInterface|null $output * @param TokenConfig|null $tokenConfig * @param EventDispatcherInterface|null $dispatcher + * @param FileLock|null $fileLock */ public function __construct( Config $config = null, CacheProvider $cache = null, OutputInterface $output = null, TokenConfig $tokenConfig = null, + FileLock $fileLock = null, EventDispatcherInterface $dispatcher = null ) { $this->config = $config ?: new Config(); $this->output = $output ?: new ConsoleOutput(); $this->stdErr = $this->output instanceof ConsoleOutputInterface ? $this->output->getErrorOutput(): $this->output; $this->tokenConfig = $tokenConfig ?: new TokenConfig($this->config); + $this->fileLock = $fileLock ?: new FileLock($this->config); $this->dispatcher = $dispatcher ?: new EventDispatcher(); $this->cache = $cache ?: CacheFactory::createCacheProvider($this->config); } @@ -287,7 +293,7 @@ private function getConnectorOptions() { $refreshLockName = 'refresh:' . $this->config->getSessionId(); $connectorOptions['on_refresh_start'] = function ($originalRefreshToken) use ($refreshLockName) { $connector = $this->getClient(false)->getConnector(); - return $this->lock($refreshLockName, function () { + return $this->fileLock->acquireOrWait($refreshLockName, function () { $this->stdErr->writeln('Waiting for token refresh lock', OutputInterface::VERBOSITY_VERBOSE); }, function () use ($connector, $originalRefreshToken) { $session = $connector->getSession(); @@ -298,7 +304,7 @@ private function getConnectorOptions() { }); }; $connectorOptions['on_refresh_end'] = function () use ($refreshLockName) { - $this->removeLock($refreshLockName); + $this->fileLock->release($refreshLockName); }; $connectorOptions['on_refresh_error'] = function (BadResponseException $e) { @@ -311,63 +317,6 @@ private function getConnectorOptions() { return $connectorOptions; } - /** - * Waits for a lock, if it exists, or creates one. - * - * @param string $lockName - * A unique name for the lock. - * @param callable|null $onWait - * A function to run when waiting starts. - * @param callable|null $check - * A function to run each time the interval has passed. If it returns a - * non-null value, waiting will stop, and the value will be returned - * from this method. - * @param int $intervalMs - * A waiting interval in milliseconds. - * @param int $timeLimit - * A time limit in seconds. - * - * @return mixed|null - */ - public function lock($lockName, callable $onWait = null, callable $check = null, $intervalMs = 300, $timeLimit = 15) - { - $runOnWait = false; - $cacheKey = 'lock:' . $lockName; - $start = \time(); - while (\time() - $start < $timeLimit) { - $cached = $this->cache->fetch($cacheKey); - if ($cached === false || $cached === '') { - break; - } - if ($onWait !== null && !$runOnWait) { - $onWait(); - $runOnWait = true; - } - \usleep($intervalMs * 1000); - if ($check !== null) { - $result = $check(); - if ($result !== null) { - $this->removeLock($lockName); - return $result; - } - } - } - $this->cache->save($cacheKey, 'locked by ' . \getmypid(), $timeLimit); - return null; - } - - /** - * Removes a lock that was created by lock(). - * - * @param string $lockName - */ - public function removeLock($lockName) - { - // Avoid deleting the cache entry, just clear it. - // This appears to help avoid race conditions. - $this->cache->save('lock:' . $lockName, ''); - } - /** * Returns the path to the CA bundle or file detected by Composer. * diff --git a/src/Service/FileLock.php b/src/Service/FileLock.php new file mode 100644 index 0000000000..34034e900a --- /dev/null +++ b/src/Service/FileLock.php @@ -0,0 +1,92 @@ +config = $config; + $this->fs = new \Symfony\Component\Filesystem\Filesystem(); + $this->checkIntervalMs = 300; + $this->timeLimit = 30; + } + + /** + * Acquires a lock, or waits for one if it already exists. + * + * @param string $lockName + * A unique name for the lock. + * @param callable|null $onWait + * A function to run when waiting starts. + * @param callable|null $check + * A function to run each time the interval has passed. If it returns a + * non-null value, waiting will stop, and the value will be returned + * from this method. + * + * @return mixed|null + */ + public function acquireOrWait($lockName, callable $onWait = null, callable $check = null) + { + $runOnWait = false; + $filename = $this->filename($lockName); + $start = \time(); + while (\time() - $start < $this->timeLimit) { + if (!\file_exists($filename)) { + break; + } + $content = \file_get_contents($filename); + if ($content === false || $content === '') { + break; + } + $lockedAt = \intval($content); + if ($lockedAt === 0 || \time() >= $lockedAt + $this->timeLimit) { + break; + } + if ($onWait !== null && !$runOnWait) { + $onWait(); + $runOnWait = true; + } + \usleep($this->checkIntervalMs * 1000); + if ($check !== null) { + $result = $check(); + if ($result !== null) { + $this->release($lockName); + return $result; + } + } + } + $this->fs->dumpFile($filename, (string) \time()); + return null; + } + + /** + * Releases a lock that was created by acquire(). + * + * @param string $lockName + */ + public function release($lockName) + { + // Truncate the file (don't delete it). + $this->fs->dumpFile($this->filename($lockName), ''); + } + + /** + * @param string $lockName + * @return string + */ + private function filename($lockName) + { + return $this->config->getWritableUserDir() + . DIRECTORY_SEPARATOR . 'locks' + . DIRECTORY_SEPARATOR + . preg_replace('/[^\w_-]+/', '-', $lockName) + . '.lock'; + } +} diff --git a/src/SshCert/Certifier.php b/src/SshCert/Certifier.php index c660846493..0669155645 100644 --- a/src/SshCert/Certifier.php +++ b/src/SshCert/Certifier.php @@ -4,6 +4,7 @@ use Platformsh\Cli\Service\Api; use Platformsh\Cli\Service\Config; +use Platformsh\Cli\Service\FileLock; use Platformsh\Cli\Service\Filesystem; use Platformsh\Cli\Service\Shell; use Platformsh\Cli\Util\Jwt; @@ -20,14 +21,16 @@ class Certifier private $shell; private $fs; private $stdErr; + private $fileLock; - public function __construct(Api $api, Config $config, Shell $shell, Filesystem $fs, OutputInterface $output) + public function __construct(Api $api, Config $config, Shell $shell, Filesystem $fs, OutputInterface $output, FileLock $fileLock) { $this->api = $api; $this->config = $config; $this->shell = $shell; $this->fs = $fs; $this->stdErr = $output instanceof ConsoleOutputInterface ? $output->getErrorOutput() : $output; + $this->fileLock = $fileLock; } /** @@ -51,7 +54,7 @@ public function generateCertificate() // files are changed at the same time in different CLI processes. $lockName = 'ssh-cert:' . $this->config->getSessionId(); $start = time(); - $result = $this->api->lock($lockName, function () { + $result = $this->fileLock->acquireOrWait($lockName, function () { $this->stdErr->writeln('Waiting for SSH certificate generation lock', OutputInterface::VERBOSITY_VERBOSE); }, function () use ($start) { // While waiting for the lock, check if a new certificate has @@ -67,7 +70,7 @@ public function generateCertificate() try { return $this->doGenerateCertificate(); } finally { - $this->api->removeLock($lockName); + $this->fileLock->release($lockName); } }