bug #27357 [Lock] Remove released semaphore (jderusse)

This PR was merged into the 3.4 branch. Discussion ---------- [Lock] Remove released semaphore | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #27356 | License | MIT | Doc PR | NA This PR remove the semaphore with `sem_remove`. By removing without releasing the semaphore, all pending blocking acquiring will fail that's why the acquire method has also been update to handle such case Commits ------- 77b9f90 Remove released semaphore
symfony · Jun 8, 2018 · 7f2cb73 · 7f2cb73
2 parents 88098f3 + 77b9f90
commit 7f2cb73
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 9 deletions.
diff --git a/src/Symfony/Component/Lock/Store/SemaphoreStore.php b/src/Symfony/Component/Lock/Store/SemaphoreStore.php
@@ -75,16 +75,20 @@ private function lock(Key $key, $blocking)
             return;
         }
 
-        $resource = sem_get(crc32($key));
+        $keyId = crc32($key);
+        $resource = sem_get($keyId);
 
-        if (\PHP_VERSION_ID < 50601) {
-            if (!$blocking) {
-                throw new NotSupportedException(sprintf('The store "%s" does not supports non blocking locks.', get_class($this)));
-            }
-
-            $acquired = sem_acquire($resource);
+        if (\PHP_VERSION_ID >= 50601) {
+            $acquired = @sem_acquire($resource, !$blocking);
+        } elseif (!$blocking) {
+            throw new NotSupportedException(sprintf('The store "%s" does not supports non blocking locks.', get_class($this)));
         } else {
-            $acquired = sem_acquire($resource, !$blocking);
+            $acquired = @sem_acquire($resource);
+        }
+
+        while ($blocking && !$acquired) {
+            $resource = sem_get($keyId);
+            $acquired = @sem_acquire($resource);
         }
 
         if (!$acquired) {
@@ -106,7 +110,7 @@ public function delete(Key $key)
 
         $resource = $key->getState(__CLASS__);
 
-        sem_release($resource);
+        sem_remove($resource);
 
         $key->removeState(__CLASS__);
     }

diff --git a/src/Symfony/Component/Lock/Tests/Store/SemaphoreStoreTest.php b/src/Symfony/Component/Lock/Tests/Store/SemaphoreStoreTest.php
@@ -11,6 +11,7 @@
 
 namespace Symfony\Component\Lock\Tests\Store;
 
+use Symfony\Component\Lock\Key;
 use Symfony\Component\Lock\Store\SemaphoreStore;
 
 /**
@@ -33,4 +34,31 @@ protected function getStore()
 
         return new SemaphoreStore();
     }
+
+    public function testResourceRemoval()
+    {
+        $initialCount = $this->getOpenedSemaphores();
+        $store = new SemaphoreStore();
+        $key = new Key(uniqid(__METHOD__, true));
+        $store->waitAndSave($key);
+
+        $this->assertGreaterThan($initialCount, $this->getOpenedSemaphores(), 'Semaphores should have been created');
+
+        $store->delete($key);
+        $this->assertEquals($initialCount, $this->getOpenedSemaphores(), 'All semaphores should be removed');
+    }
+
+    private function getOpenedSemaphores()
+    {
+        $lines = explode(PHP_EOL, trim(`ipcs -su`));
+        if ('------ Semaphore Status --------' !== $lines[0]) {
+            throw new \Exception('Failed to extract list of opend semaphores. Expect a Semaphore status, got '.implode(PHP_EOL, $lines));
+        }
+        list($key, $value) = explode(' = ', $lines[1]);
+        if ('used arrays' !== $key) {
+            throw new \Exception('Failed to extract list of opend semaphores. Expect a used arrays key, got '.implode(PHP_EOL, $lines));
+        }
+
+        return (int) $value;
+    }
 }