[Lock] Release PostgreSqlStore connection lock on failure

Author: simon-watiau

Store/PostgreSqlStore.php

@@ -80,18 +80,28 @@ public function save(Key $key)
         // prevent concurrency within the same connection
         $this->getInternalStore()->save($key);
 
-        $sql = 'SELECT pg_try_advisory_lock(:key)';
-        $stmt = $this->getConnection()->prepare($sql);
-        $stmt->bindValue(':key', $this->getHashedKey($key));
-        $result = $stmt->execute();
+        $lockAcquired = false;
 
-        // Check if lock is acquired
-        if (true === (\is_object($result) ? $result->fetchOne() : $stmt->fetchColumn())) {
-            $key->markUnserializable();
-            // release sharedLock in case of promotion
-            $this->unlockShared($key);
+        try {
+            $sql = 'SELECT pg_try_advisory_lock(:key)';
+            $stmt = $this->getConnection()->prepare($sql);
+            $stmt->bindValue(':key', $this->getHashedKey($key));
+            $result = $stmt->execute();
 
-            return;
+            // Check if lock is acquired
+            if (true === (\is_object($result) ? $result->fetchOne() : $stmt->fetchColumn())) {
+                $key->markUnserializable();
+                // release sharedLock in case of promotion
+                $this->unlockShared($key);
+
+                $lockAcquired = true;
+
+                return;
+            }
+        } finally {
+            if (!$lockAcquired) {
+                $this->getInternalStore()->delete($key);
+            }
         }
 
         throw new LockConflictedException();
@@ -102,19 +112,29 @@ public function saveRead(Key $key)
         // prevent concurrency within the same connection
         $this->getInternalStore()->saveRead($key);
 
-        $sql = 'SELECT pg_try_advisory_lock_shared(:key)';
-        $stmt = $this->getConnection()->prepare($sql);
+        $lockAcquired = false;
 
-        $stmt->bindValue(':key', $this->getHashedKey($key));
-        $result = $stmt->execute();
+        try {
+            $sql = 'SELECT pg_try_advisory_lock_shared(:key)';
+            $stmt = $this->getConnection()->prepare($sql);
+
+            $stmt->bindValue(':key', $this->getHashedKey($key));
+            $result = $stmt->execute();
 
-        // Check if lock is acquired
-        if (true === (\is_object($result) ? $result->fetchOne() : $stmt->fetchColumn())) {
-            $key->markUnserializable();
-            // release lock in case of demotion
-            $this->unlock($key);
+            // Check if lock is acquired
+            if (true === (\is_object($result) ? $result->fetchOne() : $stmt->fetchColumn())) {
+                $key->markUnserializable();
+                // release lock in case of demotion
+                $this->unlock($key);
 
-            return;
+                $lockAcquired = true;
+
+                return;
+            }
+        } finally {
+            if (!$lockAcquired) {
+                $this->getInternalStore()->delete($key);
+            }
         }
 
         throw new LockConflictedException();

Tests/Store/PostgreSqlStoreTest.php

@@ -12,6 +12,7 @@
 namespace Symfony\Component\Lock\Tests\Store;
 
 use Symfony\Component\Lock\Exception\InvalidArgumentException;
+use Symfony\Component\Lock\Exception\LockConflictedException;
 use Symfony\Component\Lock\Key;
 use Symfony\Component\Lock\PersistingStoreInterface;
 use Symfony\Component\Lock\Store\PostgreSqlStore;
@@ -50,4 +51,31 @@ public function testInvalidDriver()
         $this->expectExceptionMessage('The adapter "Symfony\Component\Lock\Store\PostgreSqlStore" does not support');
         $store->exists(new Key('foo'));
     }
+
+    public function testSaveAfterConflict()
+    {
+        $store1 = $this->getStore();
+        $store2 = $this->getStore();
+
+        $key = new Key(uniqid(__METHOD__, true));
+
+        $store1->save($key);
+        $this->assertTrue($store1->exists($key));
+
+        $lockConflicted = false;
+
+        try {
+            $store2->save($key);
+        } catch (LockConflictedException $lockConflictedException) {
+            $lockConflicted = true;
+        }
+
+        $this->assertTrue($lockConflicted);
+        $this->assertFalse($store2->exists($key));
+
+        $store1->delete($key);
+
+        $store2->save($key);
+        $this->assertTrue($store2->exists($key));
+    }
 }