diff --git a/src/Symfony/Component/Lock/Factory.php b/src/Symfony/Component/Lock/Factory.php index c050145f90978..e9fdde97e8b35 100644 --- a/src/Symfony/Component/Lock/Factory.php +++ b/src/Symfony/Component/Lock/Factory.php @@ -36,14 +36,15 @@ public function __construct(StoreInterface $store) /** * Creates a lock for the given resource. * - * @param string $resource The resource to lock - * @param float $ttl maximum expected lock duration + * @param string $resource The resource to lock + * @param float $ttl Maximum expected lock duration in seconds + * @param bool $autoRelease Whether to automatically release the lock or not when the lock instance is destroyed * * @return Lock */ - public function createLock($resource, $ttl = 300.0) + public function createLock($resource, $ttl = 300.0, $autoRelease = true) { - $lock = new Lock(new Key($resource), $this->store, $ttl); + $lock = new Lock(new Key($resource), $this->store, $ttl, $autoRelease); $lock->setLogger($this->logger); return $lock; diff --git a/src/Symfony/Component/Lock/Lock.php b/src/Symfony/Component/Lock/Lock.php index d8271569d8c05..e81878a43725c 100644 --- a/src/Symfony/Component/Lock/Lock.php +++ b/src/Symfony/Component/Lock/Lock.php @@ -32,21 +32,37 @@ final class Lock implements LockInterface, LoggerAwareInterface private $store; private $key; private $ttl; + private $autoRelease; + private $dirty = false; /** - * @param Key $key - * @param StoreInterface $store - * @param float|null $ttl + * @param Key $key Resource to lock + * @param StoreInterface $store Store used to handle lock persistence + * @param float|null $ttl Maximum expected lock duration in seconds + * @param bool $autoRelease Whether to automatically release the lock or not when the lock instance is destroyed */ - public function __construct(Key $key, StoreInterface $store, $ttl = null) + public function __construct(Key $key, StoreInterface $store, $ttl = null, $autoRelease = true) { $this->store = $store; $this->key = $key; $this->ttl = $ttl; + $this->autoRelease = (bool) $autoRelease; $this->logger = new NullLogger(); } + /** + * Automatically releases the underlying lock when the object is destructed. + */ + public function __destruct() + { + if (!$this->autoRelease || !$this->dirty || !$this->isAcquired()) { + return; + } + + $this->release(); + } + /** * {@inheritdoc} */ @@ -59,6 +75,7 @@ public function acquire($blocking = false) $this->store->waitAndSave($this->key); } + $this->dirty = true; $this->logger->info('Successfully acquired the "{resource}" lock.', array('resource' => $this->key)); if ($this->ttl) { @@ -71,6 +88,7 @@ public function acquire($blocking = false) return true; } catch (LockConflictedException $e) { + $this->dirty = false; $this->logger->warning('Failed to acquire the "{resource}" lock. Someone else already acquired the lock.', array('resource' => $this->key)); if ($blocking) { @@ -96,6 +114,7 @@ public function refresh() try { $this->key->resetLifetime(); $this->store->putOffExpiration($this->key, $this->ttl); + $this->dirty = true; if ($this->key->isExpired()) { throw new LockExpiredException(sprintf('Failed to put off the expiration of the "%s" lock within the specified time.', $this->key)); @@ -103,6 +122,7 @@ public function refresh() $this->logger->info('Expiration defined for "{resource}" lock for "{ttl}" seconds.', array('resource' => $this->key, 'ttl' => $this->ttl)); } catch (LockConflictedException $e) { + $this->dirty = false; $this->logger->warning('Failed to define an expiration for the "{resource}" lock, someone else acquired the lock.', array('resource' => $this->key)); throw $e; } catch (\Exception $e) { @@ -116,7 +136,7 @@ public function refresh() */ public function isAcquired() { - return $this->store->exists($this->key); + return $this->dirty = $this->store->exists($this->key); } /** @@ -125,6 +145,7 @@ public function isAcquired() public function release() { $this->store->delete($this->key); + $this->dirty = false; if ($this->store->exists($this->key)) { $this->logger->warning('Failed to release the "{resource}" lock.', array('resource' => $this->key)); diff --git a/src/Symfony/Component/Lock/Store/SemaphoreStore.php b/src/Symfony/Component/Lock/Store/SemaphoreStore.php index fe9455cccea09..a6cc9ea8b9f1d 100644 --- a/src/Symfony/Component/Lock/Store/SemaphoreStore.php +++ b/src/Symfony/Component/Lock/Store/SemaphoreStore.php @@ -116,7 +116,7 @@ public function delete(Key $key) */ public function putOffExpiration(Key $key, $ttl) { - // do nothing, the flock locks forever. + // do nothing, the semaphore locks forever. } /** diff --git a/src/Symfony/Component/Lock/Tests/LockTest.php b/src/Symfony/Component/Lock/Tests/LockTest.php index 97b376621169a..ece5cf667963a 100644 --- a/src/Symfony/Component/Lock/Tests/LockTest.php +++ b/src/Symfony/Component/Lock/Tests/LockTest.php @@ -103,10 +103,10 @@ public function testIsAquired() $lock = new Lock($key, $store, 10); $store - ->expects($this->once()) + ->expects($this->any()) ->method('exists') ->with($key) - ->willReturn(true); + ->will($this->onConsecutiveCalls(true, false)); $this->assertTrue($lock->isAcquired()); } @@ -131,6 +131,44 @@ public function testRelease() $lock->release(); } + public function testReleaseOnDestruction() + { + $key = new Key(uniqid(__METHOD__, true)); + $store = $this->getMockBuilder(StoreInterface::class)->getMock(); + $lock = new Lock($key, $store, 10); + + $store + ->method('exists') + ->willReturnOnConsecutiveCalls(array(true, false)) + ; + $store + ->expects($this->once()) + ->method('delete') + ; + + $lock->acquire(false); + unset($lock); + } + + public function testNoAutoReleaseWhenNotConfigured() + { + $key = new Key(uniqid(__METHOD__, true)); + $store = $this->getMockBuilder(StoreInterface::class)->getMock(); + $lock = new Lock($key, $store, 10, false); + + $store + ->method('exists') + ->willReturnOnConsecutiveCalls(array(true, false)) + ; + $store + ->expects($this->never()) + ->method('delete') + ; + + $lock->acquire(false); + unset($lock); + } + /** * @expectedException \Symfony\Component\Lock\Exception\LockReleasingException */