feat: add semaphore locking to Sysv cache

src/Cache/SysVCacheItemPool.php

@@ -18,6 +18,8 @@
 
 use Psr\Cache\CacheItemInterface;
 use Psr\Cache\CacheItemPoolInterface;
+use SysvSemaphore;
+use SysvSharedMemory;
 
 /**
  * SystemV shared memory based CacheItemPool implementation.
@@ -32,6 +34,8 @@ class SysVCacheItemPool implements CacheItemPoolInterface
 
     const DEFAULT_PROJ = 'A';
 
+    const DEFAULT_SEM_PROJ = 'B';
+
     const DEFAULT_MEMSIZE = 10000;
 
     const DEFAULT_PERM = 0600;
@@ -61,6 +65,18 @@ class SysVCacheItemPool implements CacheItemPoolInterface
      */
     private $hasLoadedItems = false;
 
+    /**
+     * @var SysvSemaphore|false
+     */
+    private SysvSemaphore|false $semId = false;
+
+    /**
+     * Maintain the process which is currently holding the semaphore to prevent deadlock.
+     *
+     * @var int|null
+     */
+    private ?int $lockOwnerPid = null;
+
     /**
      * Create a SystemV shared memory based CacheItemPool.
      *
@@ -70,26 +86,37 @@ class SysVCacheItemPool implements CacheItemPoolInterface
      *     @type int    $variableKey The variable key for getting the data from the shared memory. **Defaults to** 1.
      *     @type string $proj        The project identifier for ftok. This needs to be a one character string.
      *                               **Defaults to** 'A'.
+     *     @type string $semProj     The project identifier for ftok to provide to `sem_get`. This needs to be a one
+     *                               character string.
+     *                               **Defaults to** 'B'.
      *     @type int    $memsize     The memory size in bytes for shm_attach. **Defaults to** 10000.
      *     @type int    $perm        The permission for shm_attach. **Defaults to** 0600.
      * }
      */
     public function __construct($options = [])
     {
-        if (! extension_loaded('sysvshm')) {
+        if (!extension_loaded('sysvshm')) {
             throw new \RuntimeException(
                 'sysvshm extension is required to use this ItemPool'
             );
         }
         $this->options = $options + [
             'variableKey' => self::VAR_KEY,
             'proj' => self::DEFAULT_PROJ,
+            'semProj' => self::DEFAULT_SEM_PROJ,
             'memsize' => self::DEFAULT_MEMSIZE,
             'perm' => self::DEFAULT_PERM
         ];
         $this->items = [];
         $this->deferredItems = [];
         $this->sysvKey = ftok(__FILE__, $this->options['proj']);
+
+        // gracefully handle when `sysvsem` isn't loaded
+        // @TODO(v2): throw an exception when the extension isn't loaded
+        if (extension_loaded('sysvsem')) {
+            $semKey = ftok(__FILE__, $this->options['semProj']);
+            $this->semId = sem_get($semKey, 1, $this->options['perm'], true);
+        }
     }
 
     /**
@@ -132,9 +159,17 @@ public function hasItem($key): bool
      */
     public function clear(): bool
     {
+        if (!$this->acquireLock()) {
+            return false;
+        }
+
         $this->items = [];
         $this->deferredItems = [];
-        return $this->saveCurrentItems();
+        $ret = $this->saveCurrentItems();
+
+        $this->resetShm();
+        $this->releaseLock();
+        return $ret;
     }
 
     /**
@@ -150,27 +185,41 @@ public function deleteItem($key): bool
      */
     public function deleteItems(array $keys): bool
     {
+        if (!$this->acquireLock()) {
+            return false;
+        }
+
         if (!$this->hasLoadedItems) {
             $this->loadItems();
         }
 
         foreach ($keys as $key) {
             unset($this->items[$key]);
         }
-        return $this->saveCurrentItems();
+        $ret = $this->saveCurrentItems();
+
+        $this->resetShm();
+        $this->releaseLock();
+        return $ret;
     }
 
     /**
      * {@inheritdoc}
      */
     public function save(CacheItemInterface $item): bool
     {
+        if (!$this->acquireLock()) {
+            return false;
+        }
+
         if (!$this->hasLoadedItems) {
             $this->loadItems();
         }
 
         $this->items[$item->getKey()] = $item;
-        return $this->saveCurrentItems();
+        $ret = $this->saveCurrentItems();
+        $this->releaseLock();
+        return $ret;
     }
 
     /**
@@ -187,12 +236,18 @@ public function saveDeferred(CacheItemInterface $item): bool
      */
     public function commit(): bool
     {
+        if (!$this->acquireLock()) {
+            return false;
+        }
+
         foreach ($this->deferredItems as $item) {
             if ($this->save($item) === false) {
+                $this->releaseLock();
                 return false;
             }
         }
         $this->deferredItems = [];
+        $this->releaseLock();
         return true;
     }
 
@@ -203,20 +258,21 @@ public function commit(): bool
      */
     private function saveCurrentItems()
     {
-        $shmid = shm_attach(
-            $this->sysvKey,
-            $this->options['memsize'],
-            $this->options['perm']
-        );
-        if ($shmid !== false) {
-            $ret = shm_put_var(
+        if (!$this->acquireLock()) {
+            return false;
+        }
+
+        if (false !== $shmid = $this->attachShm()) {
+            $success = shm_put_var(
                 $shmid,
                 $this->options['variableKey'],
                 $this->items
             );
             shm_detach($shmid);
-            return $ret;
+            $this->releaseLock();
+            return $success;
         }
+        $this->releaseLock();
         return false;
     }
 
@@ -227,22 +283,70 @@ private function saveCurrentItems()
      */
     private function loadItems()
     {
-        $shmid = shm_attach(
-            $this->sysvKey,
-            $this->options['memsize'],
-            $this->options['perm']
-        );
-        if ($shmid !== false) {
+        if (!$this->acquireLock()) {
+            return false;
+        }
+
+        if (false !== $shmid = $this->attachShm()) {
             $data = @shm_get_var($shmid, $this->options['variableKey']);
-            if (!empty($data)) {
-                $this->items = $data;
-            } else {
-                $this->items = [];
-            }
+            $this->items = $data ?: [];
             shm_detach($shmid);
             $this->hasLoadedItems = true;
+            $this->releaseLock();
+            return true;
+        }
+        $this->releaseLock();
+        return false;
+    }
+
+    private function acquireLock(): bool
+    {
+        if ($this->semId === false) {
+            // if `sysvsem` isn't loaded, or if `sem_get` fails, return true
+            // this ensures BC with previous versions of the auth library.
+            // @TODO consider better handling when `sem_get` fails.
+            return true;
+        }
+
+        $currentPid = getmypid();
+        if ($this->lockOwnerPid === $currentPid) {
+            // We already have the lock
+            return true;
+        }
+
+        if (sem_acquire($this->semId)) {
+            $this->lockOwnerPid = (int) $currentPid;
             return true;
         }
         return false;
     }
+
+    private function releaseLock(): bool
+    {
+        if ($this->semId === false || $this->lockOwnerPid !== getmypid()) {
+            return true;
+        }
+
+        $this->lockOwnerPid = null;
+        return sem_release($this->semId);
+    }
+
+    private function resetShm(): void
+    {
+        // Remove the shared memory segment and semaphore when clearing the cache
+        $shmid = @shm_attach($this->sysvKey);
+        if ($shmid !== false) {
+            @shm_remove($shmid);
+            @shm_detach($shmid);
+        }
+    }
+
+    private function attachShm(): SysvSharedMemory|false
+    {
+        return shm_attach(
+            $this->sysvKey,
+            $this->options['memsize'],
+            $this->options['perm']
+        );
+    }
 }

tests/Cache/RaceConditionTest.php

@@ -45,8 +45,7 @@ public function testRaceCondition(string $cacheClass)
         if (!function_exists('pcntl_fork')) {
             $this->markTestSkipped('pcntl_fork is not available');
         }
-        for ($i = 0; $i < 100; $i++) {
-
+        for ($i = 0; $i < 50; $i++) {
             $pids = [];
             for ($j = 0; $j < 4; $j++) {
                 $pid = pcntl_fork();
@@ -56,7 +55,7 @@ public function testRaceCondition(string $cacheClass)
                 $pool = $this->createCacheItemPool($cacheClass);
                 $item = $pool->getItem('foo');
                 $item->set('bar');
-                $pool->save($item);
+                $this->assertTrue($pool->save($item));
 
                 if ($pid) {
                     // parent
@@ -68,7 +67,7 @@ public function testRaceCondition(string $cacheClass)
             }
 
             // parent
-            $pool->save($item);
+            $this->assertTrue($pool->save($item));
 
             foreach ($pids as $pid) {
                 pcntl_waitpid($pid, $status);
@@ -78,6 +77,8 @@ public function testRaceCondition(string $cacheClass)
             $this->assertTrue($pool->hasItem('foo'));
             $cachedItem = $pool->getItem('foo');
             $this->assertEquals('bar', $cachedItem->get());
+
+            $pool->clear();
         }
     }
 

tests/Cache/SysVCacheItemPoolTest.php

@@ -23,6 +23,8 @@
 
 class SysVCacheItemPoolTest extends BaseTest
 {
+    const VARIABLE_KEY = 99;
+
     private $pool;
 
     public function setUp(): void
@@ -32,10 +34,17 @@ public function setUp(): void
                 'sysvshm extension is required for running the test'
             );
         }
-        $this->pool = new SysVCacheItemPool(['variableKey' => 99]);
+        $this->pool = new SysVCacheItemPool(['variableKey' => self::VARIABLE_KEY]);
         $this->pool->clear();
     }
 
+    public function tearDown(): void
+    {
+        if (extension_loaded('sysvshm')) {
+            $this->pool->clear();
+        }
+    }
+
     public function saveItem($key, $value)
     {
         $item = $this->pool->getItem($key);
@@ -158,4 +167,39 @@ public function testCommitsDeferredItems()
             $this->pool->getItem($keys[1])->get()
         );
     }
+
+    public function testRaceCondition()
+    {
+        if (!extension_loaded('sysvsem')) {
+            $this->markTestSkipped(
+                'sysvsem extension is required for running the race condition test'
+            );
+        }
+
+        $key = 'race-item';
+        $initialValue = 0;
+        $this->saveItem($key, $initialValue);
+
+        $numProcesses = 100;
+        $processes = [];
+        for ($i = 0; $i < $numProcesses; $i++) {
+            $command = sprintf(
+                'php %s/sysv_cache_race_condition_writer.php %s %s',
+                __DIR__,
+                $key,
+                self::VARIABLE_KEY
+            );
+            $processes[] = proc_open($command, [], $pipes);
+        }
+
+        foreach ($processes as $process) {
+            // proc_close waits for the process to terminate and returns its exit code.
+            // This ensures that all child processes have completed their writes
+            // before the parent process proceeds to read the final value.
+            proc_close($process);
+        }
+
+        $finalValue = $this->pool->getItem($key)->get();
+        $this->assertEquals($numProcesses, $finalValue);
+    }
 }