From f6829d50f2460f259e172e7a97de70b3f56e98f5 Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Thu, 11 Jun 2015 23:22:54 +0200 Subject: [PATCH 01/32] Add expire for Memcache and Memcached lock storage --- src/NinjaMutex/Lock/MemcacheLock.php | 77 ++++++++++++++++++- src/NinjaMutex/Lock/MemcacheLockAbstract.php | 77 ------------------- src/NinjaMutex/Lock/MemcachedLock.php | 78 +++++++++++++++++++- 3 files changed, 149 insertions(+), 83 deletions(-) delete mode 100644 src/NinjaMutex/Lock/MemcacheLockAbstract.php diff --git a/src/NinjaMutex/Lock/MemcacheLock.php b/src/NinjaMutex/Lock/MemcacheLock.php index f6bf998..f43434b 100644 --- a/src/NinjaMutex/Lock/MemcacheLock.php +++ b/src/NinjaMutex/Lock/MemcacheLock.php @@ -16,13 +16,84 @@ * * @author Kamil Dziedzic */ -class MemcacheLock extends MemcacheLockAbstract +class MemcacheLock extends LockAbstract { + /** + * Maximum expiration time in seconds (30 days) + * http://php.net/manual/en/memcache.add.php + */ + const MAX_EXPIRATION = 2592000; + + /** + * Memcache connection + * + * @var Memcache + */ + protected $memcache; + + /** + * @var int Expiration time of the lock in seconds + */ + protected $expiration = 0; + /** * @param Memcache $memcache + * @param int $expiration Expiration time of the lock in seconds. If it's equal to zero (default), the lock will never expire. + * Max 2592000s (30 days), if greater it will be capped to 2592000 without throwing an error. + * WARNING: Using value higher than 0 may lead to race conditions. If you set too low expiration time + * e.g. 30s and critical section will run for 31s another process will gain lock at the same time, + * leading to unpredicted behaviour. Use with caution. + */ + public function __construct(Memcache $memcache, $expiration = 0) + { + parent::__construct(); + + $this->memcache = $memcache; + if ($expiration > static::MAX_EXPIRATION) { + $expiration = static::MAX_EXPIRATION; + } + $this->timeout = $expiration; + } + + /** + * @param string $name name of lock + * @param bool $blocking + * @return bool + */ + protected function getLock($name, $blocking) + { + if (!$this->memcache->add($name, serialize($this->getLockInformation()), 0, $this->expiration)) { + return false; + } + + return true; + } + + /** + * Release lock + * + * @param string $name name of lock + * @return bool + */ + public function releaseLock($name) + { + if (isset($this->locks[$name]) && $this->memcache->delete($name)) { + unset($this->locks[$name]); + + return true; + } + + return false; + } + + /** + * Check if lock is locked + * + * @param string $name name of lock + * @return bool */ - public function __construct(Memcache $memcache) + public function isLocked($name) { - parent::__construct($memcache); + return false !== $this->memcache->get($name); } } diff --git a/src/NinjaMutex/Lock/MemcacheLockAbstract.php b/src/NinjaMutex/Lock/MemcacheLockAbstract.php deleted file mode 100644 index 88a6a20..0000000 --- a/src/NinjaMutex/Lock/MemcacheLockAbstract.php +++ /dev/null @@ -1,77 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ -namespace NinjaMutex\Lock; - -/** - * Abstract for lock implementor using Memcache or Memcached - * - * @author Kamil Dziedzic - */ -abstract class MemcacheLockAbstract extends LockAbstract -{ - /** - * Memcache connection - * - * @var \Memcached|\Memcache - */ - protected $memcache; - - /** - * @param \Memcached|\Memcache $memcache - */ - public function __construct($memcache) - { - parent::__construct(); - - $this->memcache = $memcache; - } - - /** - * @param string $name - * @param bool $blocking - * @return bool - */ - protected function getLock($name, $blocking) - { - if (!$this->memcache->add($name, serialize($this->getLockInformation()))) { - return false; - } - - return true; - } - - /** - * Release lock - * - * @param string $name name of lock - * @return bool - */ - public function releaseLock($name) - { - if (isset($this->locks[$name]) && $this->memcache->delete($name)) { - unset($this->locks[$name]); - - return true; - } - - return false; - } - - /** - * Check if lock is locked - * - * @param string $name name of lock - * @return bool - */ - public function isLocked($name) - { - return false !== $this->memcache->get($name); - } -} diff --git a/src/NinjaMutex/Lock/MemcachedLock.php b/src/NinjaMutex/Lock/MemcachedLock.php index 639b449..40471e3 100644 --- a/src/NinjaMutex/Lock/MemcachedLock.php +++ b/src/NinjaMutex/Lock/MemcachedLock.php @@ -16,13 +16,85 @@ * * @author Kamil Dziedzic */ -class MemcachedLock extends MemcacheLockAbstract +class MemcachedLock extends LockAbstract { + /** + * Maximum expiration time in seconds (30 days) + * http://php.net/manual/en/memcached.add.php + */ + const MAX_EXPIRATION = 2592000; + + /** + * Memcache connection + * + * @var Memcached + */ + protected $memcached; + + /** + * @var int Expiration time of the lock in seconds + */ + protected $expiration = 0; + /** * @param Memcached $memcached + * @param int $expiration Expiration time of the lock in seconds. If it's equal to zero (default), the lock will never expire. + * Max 2592000s (30 days), if greater it will be capped to 2592000 without throwing an error. + * WARNING: Using value higher than 0 may lead to race conditions. If you set too low expiration time + * e.g. 30s and critical section will run for 31s another process will gain lock at the same time, + * leading to unpredicted behaviour. Use with caution. */ - public function __construct(Memcached $memcached) + public function __construct(Memcached $memcached, $expiration = 0) { - parent::__construct($memcached); + parent::__construct(); + + $this->memcached = $memcached; + if ($expiration > static::MAX_EXPIRATION) { + $expiration = static::MAX_EXPIRATION; + } + $this->timeout = $expiration; + } + + /** + * @param string $name name of lock + * @param bool $blocking + * @return bool + */ + protected function getLock($name, $blocking) + { + if (!$this->memcached->add($name, serialize($this->getLockInformation()), $this->expiration)) { + return false; + } + + return true; + } + + /** + * Release lock + * + * @param string $name name of lock + * @return bool + */ + public function releaseLock($name) + { + if (isset($this->locks[$name]) && $this->memcached->delete($name)) { + unset($this->locks[$name]); + + return true; + } + + return false; + } + + /** + * Check if lock is locked + * + * @param string $name name of lock + * @return bool + */ + public function isLocked($name) + { + return false !== $this->memcached->get($name); } } + From 3159f51bc6594c98d0e95093f25bbbf3d0e58e57 Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Fri, 12 Jun 2015 00:59:56 +0200 Subject: [PATCH 02/32] Let's try to test this --- tests/NinjaMutex/AbstractTest.php | 30 ++++++++++++---- tests/NinjaMutex/Lock/MemcachedLockTest.php | 38 +++++++++++++++++++++ 2 files changed, 62 insertions(+), 6 deletions(-) create mode 100644 tests/NinjaMutex/Lock/MemcachedLockTest.php diff --git a/tests/NinjaMutex/AbstractTest.php b/tests/NinjaMutex/AbstractTest.php index 3bb0ec5..298e866 100644 --- a/tests/NinjaMutex/AbstractTest.php +++ b/tests/NinjaMutex/AbstractTest.php @@ -127,25 +127,43 @@ protected function providePredisRedisMockLock() } /** - * @return array + * @param int $expiration + * @return MemcacheLock */ - protected function provideMemcacheLock() + protected function createMemcacheLock($expiration = 0) { $memcache = new Memcache(); $memcache->connect('127.0.0.1', 11211); - return array(new MemcacheLock($memcache)); + return new MemcacheLock($memcache, $expiration); } /** - * @return array + * @param int $expiration + * @return MemcachedLock */ - protected function provideMemcachedLock() + protected function createMemcachedLock($expiration = 0) { $memcached = new Memcached(); $memcached->addServer('127.0.0.1', 11211); - return array(new MemcachedLock($memcached)); + return new MemcachedLock($memcached, $expiration); + } + + /** + * @return array + */ + protected function provideMemcacheLock() + { + return array($this->createMemcacheLock()); + } + + /** + * @return array + */ + protected function provideMemcachedLock() + { + return array($this->createMemcachedLock()); } /** diff --git a/tests/NinjaMutex/Lock/MemcachedLockTest.php b/tests/NinjaMutex/Lock/MemcachedLockTest.php new file mode 100644 index 0000000..47ded98 --- /dev/null +++ b/tests/NinjaMutex/Lock/MemcachedLockTest.php @@ -0,0 +1,38 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ +namespace NinjaMutex\Lock; + +use NinjaMutex\AbstractTest; + +/** + * Tests for Locks + * + * @author Kamil Dziedzic + */ +class MemcachedLockTest extends AbstractTest +{ + public function testExpiration() + { + $name = "lock"; + $expiration = 2; // in seconds + + $lock1 = $this->createMemcachedLock($expiration); + $lock2 = $this->createMemcachedLock($expiration); + $this->assertTrue($lock1->acquireLock($name, 0)); + // We hope code was fast enough so $expiration time didn't pass yet and lock still should be held + $this->assertFalse($lock2->acquireLock($name, 0)); + + // Let's wait for lock to expire + sleep($expiration); + + // Let's try again to lock + $this->assertTrue($lock2->acquireLock($name, 0)); + } +} From 8be6e14bac1364bd4559cc1ba93e0799f9772510 Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Fri, 12 Jun 2015 01:21:45 +0200 Subject: [PATCH 03/32] Better method to write this test; make the test run as @medium --- tests/NinjaMutex/AbstractTest.php | 22 ++++++++++++ tests/NinjaMutex/Lock/LockTest.php | 27 +++++++++++++++ tests/NinjaMutex/Lock/MemcachedLockTest.php | 38 --------------------- 3 files changed, 49 insertions(+), 38 deletions(-) delete mode 100644 tests/NinjaMutex/Lock/MemcachedLockTest.php diff --git a/tests/NinjaMutex/AbstractTest.php b/tests/NinjaMutex/AbstractTest.php index 298e866..937feef 100644 --- a/tests/NinjaMutex/AbstractTest.php +++ b/tests/NinjaMutex/AbstractTest.php @@ -48,6 +48,9 @@ public function tearDown() rmdir('/tmp/mutex/'); } + /** + * @return array + */ public function lockImplementorProvider() { $data = array( @@ -68,6 +71,9 @@ public function lockImplementorProvider() return $data; } + /** + * @return array + */ public function lockImplementorWithBackendProvider() { $data = array( @@ -80,6 +86,22 @@ public function lockImplementorWithBackendProvider() return $data; } + /** + * @return array + */ + public function lockImplementorWithExpirationProvider() + { + $expiration = 2; // in seconds + + $data = array( + // Just mocks + array($this->createMemcacheLock(), $this->createMemcacheLock($expiration), $expiration), + array($this->createMemcachedLock(), $this->createMemcachedLock($expiration), $expiration), + ); + + return $data; + } + /** * @return array */ diff --git a/tests/NinjaMutex/Lock/LockTest.php b/tests/NinjaMutex/Lock/LockTest.php index 5291449..abf5001 100644 --- a/tests/NinjaMutex/Lock/LockTest.php +++ b/tests/NinjaMutex/Lock/LockTest.php @@ -160,4 +160,31 @@ public function testIfLockDestructorThrowsWhenBackendIsUnavailable(LockInterface $this->fail('An expected exception has not been raised.'); } + /** + * @issue https://github.com/arvenil/ninja-mutex/issues/12 + * @medium + * + * @dataProvider lockImplementorWithExpirationProvider + * @param LockInterface $lockImplementor + * @param LockInterface $lockImplementorWithExpiration + * @param int $expiration + */ + public function testExpiration(LockInterface $lockImplementor, LockInterface $lockImplementorWithExpiration, $expiration) + { + $name = "lock"; + + // Aquire lock on implementor with lock expiration + $this->assertTrue($lockImplementorWithExpiration->acquireLock($name, 0)); + // We hope code was fast enough so $expiration time didn't pass yet and lock still should be held + $this->assertFalse($lockImplementor->acquireLock($name, 0)); + + // Let's wait for lock to expire + sleep($expiration); + + // Let's try again to lock + $this->assertTrue($lockImplementor->acquireLock($name, 0)); + + // Cleanup + $this->assertTrue($lockImplementor->releaseLock($name, 0)); + } } diff --git a/tests/NinjaMutex/Lock/MemcachedLockTest.php b/tests/NinjaMutex/Lock/MemcachedLockTest.php deleted file mode 100644 index 47ded98..0000000 --- a/tests/NinjaMutex/Lock/MemcachedLockTest.php +++ /dev/null @@ -1,38 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ -namespace NinjaMutex\Lock; - -use NinjaMutex\AbstractTest; - -/** - * Tests for Locks - * - * @author Kamil Dziedzic - */ -class MemcachedLockTest extends AbstractTest -{ - public function testExpiration() - { - $name = "lock"; - $expiration = 2; // in seconds - - $lock1 = $this->createMemcachedLock($expiration); - $lock2 = $this->createMemcachedLock($expiration); - $this->assertTrue($lock1->acquireLock($name, 0)); - // We hope code was fast enough so $expiration time didn't pass yet and lock still should be held - $this->assertFalse($lock2->acquireLock($name, 0)); - - // Let's wait for lock to expire - sleep($expiration); - - // Let's try again to lock - $this->assertTrue($lock2->acquireLock($name, 0)); - } -} From 22e522755198b95e5762e2f59481059dc5f7d78c Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Fri, 12 Jun 2015 01:49:47 +0200 Subject: [PATCH 04/32] Hmm --- composer.json | 4 ++-- tests/NinjaMutex/Lock/LockTest.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/composer.json b/composer.json index 15b7a56..9bb9b8d 100644 --- a/composer.json +++ b/composer.json @@ -16,12 +16,12 @@ } }, "require": { - "php": ">=5.3.0" + "php": ">=5.3.0", + "mikey179/vfsStream": "v1.4.0" }, "require-dev": { "codeclimate/php-test-reporter": "v0.1.2", "scrutinizer/ocular": "1.1.1", - "mikey179/vfsStream": "v1.4.0", "predis/predis": "v1.0.0", "ext-memcache": "*", "ext-memcached": "*", diff --git a/tests/NinjaMutex/Lock/LockTest.php b/tests/NinjaMutex/Lock/LockTest.php index abf5001..e068246 100644 --- a/tests/NinjaMutex/Lock/LockTest.php +++ b/tests/NinjaMutex/Lock/LockTest.php @@ -179,7 +179,7 @@ public function testExpiration(LockInterface $lockImplementor, LockInterface $lo $this->assertFalse($lockImplementor->acquireLock($name, 0)); // Let's wait for lock to expire - sleep($expiration); + sleep($expiration+2); // Let's try again to lock $this->assertTrue($lockImplementor->acquireLock($name, 0)); From f028cf0ba14c0a18cb9a869460ed574387b1acd5 Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Fri, 12 Jun 2015 02:01:02 +0200 Subject: [PATCH 05/32] Avoid interfering locks with each other by adding uuid to lock name --- tests/NinjaMutex/Lock/LockTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/NinjaMutex/Lock/LockTest.php b/tests/NinjaMutex/Lock/LockTest.php index e068246..8f14110 100644 --- a/tests/NinjaMutex/Lock/LockTest.php +++ b/tests/NinjaMutex/Lock/LockTest.php @@ -162,7 +162,7 @@ public function testIfLockDestructorThrowsWhenBackendIsUnavailable(LockInterface /** * @issue https://github.com/arvenil/ninja-mutex/issues/12 - * @medium + * @medium Timeout for test increased to ~5s http://stackoverflow.com/a/10535787/916440 * * @dataProvider lockImplementorWithExpirationProvider * @param LockInterface $lockImplementor @@ -171,7 +171,7 @@ public function testIfLockDestructorThrowsWhenBackendIsUnavailable(LockInterface */ public function testExpiration(LockInterface $lockImplementor, LockInterface $lockImplementorWithExpiration, $expiration) { - $name = "lock"; + $name = "lockWithExpiration_" . uniqid(); // Aquire lock on implementor with lock expiration $this->assertTrue($lockImplementorWithExpiration->acquireLock($name, 0)); From 3ebd315e707ab7f3170e8d5c3bb6eafa92f47854 Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Fri, 12 Jun 2015 02:06:51 +0200 Subject: [PATCH 06/32] Bugfix, this reminds me why I love strict typing --- src/NinjaMutex/Lock/MemcacheLock.php | 2 +- src/NinjaMutex/Lock/MemcachedLock.php | 2 +- tests/NinjaMutex/Lock/LockTest.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/NinjaMutex/Lock/MemcacheLock.php b/src/NinjaMutex/Lock/MemcacheLock.php index f43434b..8da2d7b 100644 --- a/src/NinjaMutex/Lock/MemcacheLock.php +++ b/src/NinjaMutex/Lock/MemcacheLock.php @@ -52,7 +52,7 @@ public function __construct(Memcache $memcache, $expiration = 0) if ($expiration > static::MAX_EXPIRATION) { $expiration = static::MAX_EXPIRATION; } - $this->timeout = $expiration; + $this->expiration = $expiration; } /** diff --git a/src/NinjaMutex/Lock/MemcachedLock.php b/src/NinjaMutex/Lock/MemcachedLock.php index 40471e3..1dd2c29 100644 --- a/src/NinjaMutex/Lock/MemcachedLock.php +++ b/src/NinjaMutex/Lock/MemcachedLock.php @@ -52,7 +52,7 @@ public function __construct(Memcached $memcached, $expiration = 0) if ($expiration > static::MAX_EXPIRATION) { $expiration = static::MAX_EXPIRATION; } - $this->timeout = $expiration; + $this->expiration = $expiration; } /** diff --git a/tests/NinjaMutex/Lock/LockTest.php b/tests/NinjaMutex/Lock/LockTest.php index 8f14110..f7523a3 100644 --- a/tests/NinjaMutex/Lock/LockTest.php +++ b/tests/NinjaMutex/Lock/LockTest.php @@ -179,7 +179,7 @@ public function testExpiration(LockInterface $lockImplementor, LockInterface $lo $this->assertFalse($lockImplementor->acquireLock($name, 0)); // Let's wait for lock to expire - sleep($expiration+2); + sleep($expiration); // Let's try again to lock $this->assertTrue($lockImplementor->acquireLock($name, 0)); From 8762483ed9f1a8fb2be2b4f6191a7b3a380d958d Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Fri, 12 Jun 2015 02:47:39 +0200 Subject: [PATCH 07/32] Let's try to fix hhvm build --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index 3465114..9293d48 100644 --- a/.travis.yml +++ b/.travis.yml @@ -21,6 +21,8 @@ before_script: - ./tests/travis/memcached-setup.sh - ./tests/travis/composer-setup.sh - ./tests/travis/mysql-setup.sh + - mkdir -p ~/.phpenv/versions/$(phpenv version-name)/etc/ + - echo 'hhvm.enable_obj_destruct_call = true' >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini" script: phpunit From 83914197f2764b77dc0ff166021c593d70f3e31e Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Fri, 12 Jun 2015 02:55:25 +0200 Subject: [PATCH 08/32] Yet another try to fix hhvm --- .travis.hhvm.ini | 2 ++ .travis.php.ini | 2 ++ .travis.yml | 6 ++---- tests/travis/memcache-setup.sh | 11 ----------- tests/travis/memcached-setup.sh | 11 ----------- 5 files changed, 6 insertions(+), 26 deletions(-) create mode 100644 .travis.hhvm.ini create mode 100644 .travis.php.ini delete mode 100755 tests/travis/memcache-setup.sh delete mode 100755 tests/travis/memcached-setup.sh diff --git a/.travis.hhvm.ini b/.travis.hhvm.ini new file mode 100644 index 0000000..fa38050 --- /dev/null +++ b/.travis.hhvm.ini @@ -0,0 +1,2 @@ +hhvm.enable_obj_destruct_call = true + diff --git a/.travis.php.ini b/.travis.php.ini new file mode 100644 index 0000000..5121d16 --- /dev/null +++ b/.travis.php.ini @@ -0,0 +1,2 @@ +extension=memcache.so +extension=memcached.so diff --git a/.travis.yml b/.travis.yml index 9293d48..b8e48a5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,12 +17,10 @@ services: - redis-server before_script: - - ./tests/travis/memcache-setup.sh - - ./tests/travis/memcached-setup.sh - ./tests/travis/composer-setup.sh - ./tests/travis/mysql-setup.sh - - mkdir -p ~/.phpenv/versions/$(phpenv version-name)/etc/ - - echo 'hhvm.enable_obj_destruct_call = true' >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini" + - if [[ "$TRAVIS_PHP_VERSION" = "php*" ]]; then phpenv config-add .travis.php.ini ; fi + - if [[ "$TRAVIS_PHP_VERSION" = "hhv*" ]]; then cat .travis.hhvm.ini >> /etc/hhvm/php.ini ; fi script: phpunit diff --git a/tests/travis/memcache-setup.sh b/tests/travis/memcache-setup.sh deleted file mode 100755 index b0e419f..0000000 --- a/tests/travis/memcache-setup.sh +++ /dev/null @@ -1,11 +0,0 @@ -#!/bin/sh - -install_memcache() { - if [ $(expr "${TRAVIS_PHP_VERSION}" "!=" "hhvm") -eq 1 ] && [ $(expr "${TRAVIS_PHP_VERSION}" "!=" "hhvm-nightly") -eq 1 ]; then - echo "extension=memcache.so" >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini - fi - - return $? -} - -install_memcache > ~/memcache.log || ( echo "=== MEMCACHE INSTALL FAILED ==="; cat ~/memcache.log; exit 1 ) diff --git a/tests/travis/memcached-setup.sh b/tests/travis/memcached-setup.sh deleted file mode 100755 index 14c9905..0000000 --- a/tests/travis/memcached-setup.sh +++ /dev/null @@ -1,11 +0,0 @@ -#!/bin/sh - -install_memcached() { - if [ $(expr "${TRAVIS_PHP_VERSION}" "!=" "hhvm") -eq 1 ] && [ $(expr "${TRAVIS_PHP_VERSION}" "!=" "hhvm-nightly") -eq 1 ]; then - echo "extension=memcached.so" >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini - fi - - return $? -} - -install_memcached > ~/memcached.log || ( echo "=== MEMCACHED INSTALL FAILED ==="; cat ~/memcached.log; exit 1 ) From 9e02b64abefc7a106f8aa7daa435b9dae826002f Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Fri, 12 Jun 2015 03:13:40 +0200 Subject: [PATCH 09/32] Let's see what will happen if I will directly clear objects --- .travis.yml | 4 ++-- tests/NinjaMutex/Lock/LockTest.php | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index b8e48a5..e6be98f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,10 +17,10 @@ services: - redis-server before_script: - - ./tests/travis/composer-setup.sh - - ./tests/travis/mysql-setup.sh - if [[ "$TRAVIS_PHP_VERSION" = "php*" ]]; then phpenv config-add .travis.php.ini ; fi - if [[ "$TRAVIS_PHP_VERSION" = "hhv*" ]]; then cat .travis.hhvm.ini >> /etc/hhvm/php.ini ; fi + - ./tests/travis/composer-setup.sh + - ./tests/travis/mysql-setup.sh script: phpunit diff --git a/tests/NinjaMutex/Lock/LockTest.php b/tests/NinjaMutex/Lock/LockTest.php index f7523a3..716fffa 100644 --- a/tests/NinjaMutex/Lock/LockTest.php +++ b/tests/NinjaMutex/Lock/LockTest.php @@ -186,5 +186,8 @@ public function testExpiration(LockInterface $lockImplementor, LockInterface $lo // Cleanup $this->assertTrue($lockImplementor->releaseLock($name, 0)); + + $lockImplementor = null; // for hhvm to run __destructor + $lockImplementorWithExpiration = null; // for hhvm to run __destructor } } From 8adb2fd1f626945c176162c74287ebdf6edbd961 Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Fri, 12 Jun 2015 03:31:04 +0200 Subject: [PATCH 10/32] Let's try to fix php builds --- .travis.hhvm.ini | 2 -- .travis.yml | 3 +-- tests/NinjaMutex/Lock/LockTest.php | 2 ++ 3 files changed, 3 insertions(+), 4 deletions(-) delete mode 100644 .travis.hhvm.ini diff --git a/.travis.hhvm.ini b/.travis.hhvm.ini deleted file mode 100644 index fa38050..0000000 --- a/.travis.hhvm.ini +++ /dev/null @@ -1,2 +0,0 @@ -hhvm.enable_obj_destruct_call = true - diff --git a/.travis.yml b/.travis.yml index e6be98f..434f8ff 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,8 +17,7 @@ services: - redis-server before_script: - - if [[ "$TRAVIS_PHP_VERSION" = "php*" ]]; then phpenv config-add .travis.php.ini ; fi - - if [[ "$TRAVIS_PHP_VERSION" = "hhv*" ]]; then cat .travis.hhvm.ini >> /etc/hhvm/php.ini ; fi + - if [[ "$TRAVIS_PHP_VERSION" = "php*" ]]; then cat .travis.php.ini >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini ; fi - ./tests/travis/composer-setup.sh - ./tests/travis/mysql-setup.sh diff --git a/tests/NinjaMutex/Lock/LockTest.php b/tests/NinjaMutex/Lock/LockTest.php index 716fffa..71bf4a5 100644 --- a/tests/NinjaMutex/Lock/LockTest.php +++ b/tests/NinjaMutex/Lock/LockTest.php @@ -127,6 +127,8 @@ public function testIfLockIsReleasedAfterLockImplementorIsDestroyed(LockInterfac /** * @issue https://github.com/arvenil/ninja-mutex/pull/4 + * It's not working for hhvm, see below link to understand limitation + * https://github.com/facebook/hhvm/blob/af329776c9f740cc1c8c4791f673ba5aa49042ce/hphp/doc/inconsistencies#L40-L45 * * @dataProvider lockImplementorWithBackendProvider * @param LockInterface $lockImplementor From a29e5d74509d740d2c2692e26cbaa8990c586d20 Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Fri, 12 Jun 2015 03:33:08 +0200 Subject: [PATCH 11/32] Let's try to catch and discard exceptions that are thrown while destroying lock --- tests/NinjaMutex/Lock/LockTest.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/NinjaMutex/Lock/LockTest.php b/tests/NinjaMutex/Lock/LockTest.php index 71bf4a5..0e279bc 100644 --- a/tests/NinjaMutex/Lock/LockTest.php +++ b/tests/NinjaMutex/Lock/LockTest.php @@ -189,7 +189,10 @@ public function testExpiration(LockInterface $lockImplementor, LockInterface $lo // Cleanup $this->assertTrue($lockImplementor->releaseLock($name, 0)); - $lockImplementor = null; // for hhvm to run __destructor - $lockImplementorWithExpiration = null; // for hhvm to run __destructor + try { + $lockImplementor = null; + $lockImplementorWithExpiration = null; + } catch (UnrecoverableMutexException $e) { + } } } From d1e87d96a52e3c458e6125fccae498d26a1fdabc Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Fri, 12 Jun 2015 03:50:14 +0200 Subject: [PATCH 12/32] Let's try this way --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 434f8ff..16d9bb9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,7 +17,7 @@ services: - redis-server before_script: - - if [[ "$TRAVIS_PHP_VERSION" = "php*" ]]; then cat .travis.php.ini >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini ; fi + - if [[ "$TRAVIS_PHP_VERSION" != "hhvm" ]]; then cat .travis.php.ini >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini ; echo "Loading additional config for version: $TRAVIS_PHP_VERSION" ; fi - ./tests/travis/composer-setup.sh - ./tests/travis/mysql-setup.sh From aa145d798490171add1f7dad6740187350c61390 Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Fri, 12 Jun 2015 03:53:04 +0200 Subject: [PATCH 13/32] Ups --- composer.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 9bb9b8d..15b7a56 100644 --- a/composer.json +++ b/composer.json @@ -16,12 +16,12 @@ } }, "require": { - "php": ">=5.3.0", - "mikey179/vfsStream": "v1.4.0" + "php": ">=5.3.0" }, "require-dev": { "codeclimate/php-test-reporter": "v0.1.2", "scrutinizer/ocular": "1.1.1", + "mikey179/vfsStream": "v1.4.0", "predis/predis": "v1.0.0", "ext-memcache": "*", "ext-memcached": "*", From 4a36c6b95d5fbbe30cd2e7440757f9ce25691554 Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Fri, 12 Jun 2015 03:55:56 +0200 Subject: [PATCH 14/32] WTH --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 16d9bb9..9e0fdc6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,7 +17,7 @@ services: - redis-server before_script: - - if [[ "$TRAVIS_PHP_VERSION" != "hhvm" ]]; then cat .travis.php.ini >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini ; echo "Loading additional config for version: $TRAVIS_PHP_VERSION" ; fi + - if [[ "$TRAVIS_PHP_VERSION" != "hhvm" ]]; then cat .travis.php.ini >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini ; echo "Loading additional config for version $TRAVIS_PHP_VERSION" ; fi - ./tests/travis/composer-setup.sh - ./tests/travis/mysql-setup.sh From 0875db4bd229485a50a27c7e2fe4ba9b5970630a Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Fri, 12 Jun 2015 12:07:06 +0200 Subject: [PATCH 15/32] Let's try this trick --- src/NinjaMutex/Mutex.php | 2 +- tests/NinjaMutex/AbstractTest.php | 12 ++++++++---- tests/NinjaMutex/Lock/LockTest.php | 10 ++++++---- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/NinjaMutex/Mutex.php b/src/NinjaMutex/Mutex.php index 93cf4eb..3069ba7 100644 --- a/src/NinjaMutex/Mutex.php +++ b/src/NinjaMutex/Mutex.php @@ -94,7 +94,7 @@ public function releaseLock() */ public function __destruct() { - while ($this->isAcquired()) { + if ($this->isAcquired()) { $released = $this->releaseLock(); if (!$released) { throw new UnrecoverableMutexException(sprintf( diff --git a/tests/NinjaMutex/AbstractTest.php b/tests/NinjaMutex/AbstractTest.php index 937feef..9b6aaf2 100644 --- a/tests/NinjaMutex/AbstractTest.php +++ b/tests/NinjaMutex/AbstractTest.php @@ -91,12 +91,16 @@ public function lockImplementorWithBackendProvider() */ public function lockImplementorWithExpirationProvider() { - $expiration = 2; // in seconds + $memcacheLockFabric = function($expiration = 0) { + return $this->createMemcacheLock($expiration); + }; + $memcachedLockFabric = function($expiration = 0) { + return $this->createMemcachedLock($expiration); + }; $data = array( - // Just mocks - array($this->createMemcacheLock(), $this->createMemcacheLock($expiration), $expiration), - array($this->createMemcachedLock(), $this->createMemcachedLock($expiration), $expiration), + array($memcacheLockFabric), + array($memcachedLockFabric), ); return $data; diff --git a/tests/NinjaMutex/Lock/LockTest.php b/tests/NinjaMutex/Lock/LockTest.php index 0e279bc..3b03119 100644 --- a/tests/NinjaMutex/Lock/LockTest.php +++ b/tests/NinjaMutex/Lock/LockTest.php @@ -167,13 +167,14 @@ public function testIfLockDestructorThrowsWhenBackendIsUnavailable(LockInterface * @medium Timeout for test increased to ~5s http://stackoverflow.com/a/10535787/916440 * * @dataProvider lockImplementorWithExpirationProvider - * @param LockInterface $lockImplementor - * @param LockInterface $lockImplementorWithExpiration - * @param int $expiration + * @param $lockImplementorFabric */ - public function testExpiration(LockInterface $lockImplementor, LockInterface $lockImplementorWithExpiration, $expiration) + public function testExpiration($lockImplementorFabric) { + $expiration = 2; // in seconds $name = "lockWithExpiration_" . uniqid(); + $lockImplementorWithExpiration = $lockImplementorFabric($expiration); + $lockImplementor = $lockImplementorFabric(); // Aquire lock on implementor with lock expiration $this->assertTrue($lockImplementorWithExpiration->acquireLock($name, 0)); @@ -189,6 +190,7 @@ public function testExpiration(LockInterface $lockImplementor, LockInterface $lo // Cleanup $this->assertTrue($lockImplementor->releaseLock($name, 0)); + // Now the Mutex with lock expiration is in unre try { $lockImplementor = null; $lockImplementorWithExpiration = null; From b6f35167f2cb0040c9bd8aeb810e946201b9238e Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Fri, 12 Jun 2015 12:15:58 +0200 Subject: [PATCH 16/32] `--dev` param is obsolete in new composer --- tests/travis/composer-setup.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/travis/composer-setup.sh b/tests/travis/composer-setup.sh index 10a6a39..08d5476 100755 --- a/tests/travis/composer-setup.sh +++ b/tests/travis/composer-setup.sh @@ -1,3 +1,3 @@ #!/bin/sh -wget -nc http://getcomposer.org/composer.phar && php composer.phar install --dev --prefer-source +wget -nc http://getcomposer.org/composer.phar && php composer.phar install --prefer-source From f7313aa6d2e86015a0520ff599bec1fdcad605ce Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Fri, 12 Jun 2015 12:21:33 +0200 Subject: [PATCH 17/32] Ups, don't push dev code --- src/NinjaMutex/Mutex.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NinjaMutex/Mutex.php b/src/NinjaMutex/Mutex.php index 3069ba7..93cf4eb 100644 --- a/src/NinjaMutex/Mutex.php +++ b/src/NinjaMutex/Mutex.php @@ -94,7 +94,7 @@ public function releaseLock() */ public function __destruct() { - if ($this->isAcquired()) { + while ($this->isAcquired()) { $released = $this->releaseLock(); if (!$released) { throw new UnrecoverableMutexException(sprintf( From 0157c7e057045dfdaa92c921ad5d2e0c1019b312 Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Fri, 12 Jun 2015 12:56:05 +0200 Subject: [PATCH 18/32] Try to make tests run for php 5.3 --- tests/NinjaMutex/AbstractTest.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/NinjaMutex/AbstractTest.php b/tests/NinjaMutex/AbstractTest.php index 9b6aaf2..8447231 100644 --- a/tests/NinjaMutex/AbstractTest.php +++ b/tests/NinjaMutex/AbstractTest.php @@ -91,11 +91,15 @@ public function lockImplementorWithBackendProvider() */ public function lockImplementorWithExpirationProvider() { - $memcacheLockFabric = function($expiration = 0) { - return $this->createMemcacheLock($expiration); + // $self = $this is for compatibility with php 5.3 + // $this in lambda functions was introduced in php 5.4 + // http://php.net/manual/en/functions.anonymous.php + $self = $this; + $memcacheLockFabric = function($expiration = 0) use ($self) { + return $self->createMemcacheLock($expiration); }; - $memcachedLockFabric = function($expiration = 0) { - return $this->createMemcachedLock($expiration); + $memcachedLockFabric = function($expiration = 0) use ($self) { + return $self->createMemcachedLock($expiration); }; $data = array( From faaba395b5b103fbd63b15557716ac490cf27e7e Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Fri, 12 Jun 2015 12:59:33 +0200 Subject: [PATCH 19/32] Let's check if now maybe exceptions are properly raised --- tests/NinjaMutex/Lock/LockTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/NinjaMutex/Lock/LockTest.php b/tests/NinjaMutex/Lock/LockTest.php index 3b03119..3e40b42 100644 --- a/tests/NinjaMutex/Lock/LockTest.php +++ b/tests/NinjaMutex/Lock/LockTest.php @@ -195,6 +195,9 @@ public function testExpiration($lockImplementorFabric) $lockImplementor = null; $lockImplementorWithExpiration = null; } catch (UnrecoverableMutexException $e) { + return; } + + $this->fail('An expected exception has not been raised.'); } } From 4a903541101b71e1ddb6c3ce3d19a211f4078821 Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Fri, 12 Jun 2015 20:23:47 +0200 Subject: [PATCH 20/32] That php 5.3:/ --- tests/NinjaMutex/AbstractTest.php | 4 ++-- tests/NinjaMutex/Lock/LockTest.php | 13 ++++++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/tests/NinjaMutex/AbstractTest.php b/tests/NinjaMutex/AbstractTest.php index 8447231..a667ac7 100644 --- a/tests/NinjaMutex/AbstractTest.php +++ b/tests/NinjaMutex/AbstractTest.php @@ -160,7 +160,7 @@ protected function providePredisRedisMockLock() * @param int $expiration * @return MemcacheLock */ - protected function createMemcacheLock($expiration = 0) + public function createMemcacheLock($expiration = 0) { $memcache = new Memcache(); $memcache->connect('127.0.0.1', 11211); @@ -172,7 +172,7 @@ protected function createMemcacheLock($expiration = 0) * @param int $expiration * @return MemcachedLock */ - protected function createMemcachedLock($expiration = 0) + public function createMemcachedLock($expiration = 0) { $memcached = new Memcached(); $memcached->addServer('127.0.0.1', 11211); diff --git a/tests/NinjaMutex/Lock/LockTest.php b/tests/NinjaMutex/Lock/LockTest.php index 3e40b42..213559a 100644 --- a/tests/NinjaMutex/Lock/LockTest.php +++ b/tests/NinjaMutex/Lock/LockTest.php @@ -165,6 +165,7 @@ public function testIfLockDestructorThrowsWhenBackendIsUnavailable(LockInterface /** * @issue https://github.com/arvenil/ninja-mutex/issues/12 * @medium Timeout for test increased to ~5s http://stackoverflow.com/a/10535787/916440 + * @runInSeparateProcess * * @dataProvider lockImplementorWithExpirationProvider * @param $lockImplementorFabric @@ -190,14 +191,16 @@ public function testExpiration($lockImplementorFabric) // Cleanup $this->assertTrue($lockImplementor->releaseLock($name, 0)); - // Now the Mutex with lock expiration is in unre + // Now we set null to the Mutex with lock expiration to invoke __destructor try { - $lockImplementor = null; $lockImplementorWithExpiration = null; } catch (UnrecoverableMutexException $e) { - return; + // hhvm doesn't throw an exception here, it rather raises a fatal error, + // so I can't check here if Exception was really raised for all builds. + // Looks like I should always raise fatal error in __destructor for all versions rather than trying to raise exception + // https://github.com/facebook/hhvm/blob/af329776c9f740cc1c8c4791f673ba5aa49042ce/hphp/doc/inconsistencies#L40-L48 + // http://docs.hhvm.com/manual/en/language.oop5.decon.php#language.oop5.decon.destructor + // https://github.com/sebastianbergmann/phpunit/issues/1640 } - - $this->fail('An expected exception has not been raised.'); } } From 8c9a77fc2b6a1d1060954eb080cb10c446e87ea6 Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Fri, 12 Jun 2015 22:32:05 +0200 Subject: [PATCH 21/32] Let's see what happens --- .../Lock/LockExpirationInterface.php | 23 +++++++ src/NinjaMutex/Lock/MemcacheLock.php | 20 ++++-- src/NinjaMutex/Lock/MemcachedLock.php | 20 ++++-- tests/NinjaMutex/AbstractTest.php | 65 +++---------------- .../LockFabricWithExpirationInterface.php | 25 +++++++ .../Lock/Fabric/MemcacheLockFabric.php | 25 +++++++ .../Lock/Fabric/MemcachedLockFabric.php | 25 +++++++ tests/NinjaMutex/Lock/LockTest.php | 11 ++-- 8 files changed, 140 insertions(+), 74 deletions(-) create mode 100644 src/NinjaMutex/Lock/LockExpirationInterface.php create mode 100644 tests/NinjaMutex/Lock/Fabric/LockFabricWithExpirationInterface.php create mode 100644 tests/NinjaMutex/Lock/Fabric/MemcacheLockFabric.php create mode 100644 tests/NinjaMutex/Lock/Fabric/MemcachedLockFabric.php diff --git a/src/NinjaMutex/Lock/LockExpirationInterface.php b/src/NinjaMutex/Lock/LockExpirationInterface.php new file mode 100644 index 0000000..eeb8e83 --- /dev/null +++ b/src/NinjaMutex/Lock/LockExpirationInterface.php @@ -0,0 +1,23 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ +namespace NinjaMutex\Lock; + +/** + * Lock implementor + * + * @author Kamil Dziedzic + */ +interface LockExpirationInterface +{ + /** + * @param int $expiration Expiration time of the lock in seconds. + */ + public function setExpiration($expiration); +} diff --git a/src/NinjaMutex/Lock/MemcacheLock.php b/src/NinjaMutex/Lock/MemcacheLock.php index 8da2d7b..f306c8e 100644 --- a/src/NinjaMutex/Lock/MemcacheLock.php +++ b/src/NinjaMutex/Lock/MemcacheLock.php @@ -16,7 +16,7 @@ * * @author Kamil Dziedzic */ -class MemcacheLock extends LockAbstract +class MemcacheLock extends LockAbstract implements LockExpirationInterface { /** * Maximum expiration time in seconds (30 days) @@ -38,17 +38,23 @@ class MemcacheLock extends LockAbstract /** * @param Memcache $memcache - * @param int $expiration Expiration time of the lock in seconds. If it's equal to zero (default), the lock will never expire. - * Max 2592000s (30 days), if greater it will be capped to 2592000 without throwing an error. - * WARNING: Using value higher than 0 may lead to race conditions. If you set too low expiration time - * e.g. 30s and critical section will run for 31s another process will gain lock at the same time, - * leading to unpredicted behaviour. Use with caution. */ - public function __construct(Memcache $memcache, $expiration = 0) + public function __construct(Memcache $memcache) { parent::__construct(); $this->memcache = $memcache; + } + + /** + * @param int $expiration Expiration time of the lock in seconds. If it's equal to zero (default), the lock will never expire. + * Max 2592000s (30 days), if greater it will be capped to 2592000 without throwing an error. + * WARNING: Using value higher than 0 may lead to race conditions. If you set too low expiration time + * e.g. 30s and critical section will run for 31s another process will gain lock at the same time, + * leading to unpredicted behaviour. Use with caution. + */ + public function setExpiration($expiration) + { if ($expiration > static::MAX_EXPIRATION) { $expiration = static::MAX_EXPIRATION; } diff --git a/src/NinjaMutex/Lock/MemcachedLock.php b/src/NinjaMutex/Lock/MemcachedLock.php index 1dd2c29..0d14669 100644 --- a/src/NinjaMutex/Lock/MemcachedLock.php +++ b/src/NinjaMutex/Lock/MemcachedLock.php @@ -16,7 +16,7 @@ * * @author Kamil Dziedzic */ -class MemcachedLock extends LockAbstract +class MemcachedLock extends LockAbstract implements LockExpirationInterface { /** * Maximum expiration time in seconds (30 days) @@ -38,17 +38,23 @@ class MemcachedLock extends LockAbstract /** * @param Memcached $memcached - * @param int $expiration Expiration time of the lock in seconds. If it's equal to zero (default), the lock will never expire. - * Max 2592000s (30 days), if greater it will be capped to 2592000 without throwing an error. - * WARNING: Using value higher than 0 may lead to race conditions. If you set too low expiration time - * e.g. 30s and critical section will run for 31s another process will gain lock at the same time, - * leading to unpredicted behaviour. Use with caution. */ - public function __construct(Memcached $memcached, $expiration = 0) + public function __construct(Memcached $memcached) { parent::__construct(); $this->memcached = $memcached; + } + + /** + * @param int $expiration Expiration time of the lock in seconds. If it's equal to zero (default), the lock will never expire. + * Max 2592000s (30 days), if greater it will be capped to 2592000 without throwing an error. + * WARNING: Using value higher than 0 may lead to race conditions. If you set too low expiration time + * e.g. 30s and critical section will run for 31s another process will gain lock at the same time, + * leading to unpredicted behaviour. Use with caution. + */ + public function setExpiration($expiration) + { if ($expiration > static::MAX_EXPIRATION) { $expiration = static::MAX_EXPIRATION; } diff --git a/tests/NinjaMutex/AbstractTest.php b/tests/NinjaMutex/AbstractTest.php index a667ac7..0c35655 100644 --- a/tests/NinjaMutex/AbstractTest.php +++ b/tests/NinjaMutex/AbstractTest.php @@ -9,12 +9,12 @@ */ namespace NinjaMutex; -use Memcache; -use Memcached; use NinjaMutex\Lock\FlockLock; use NinjaMutex\Lock\MemcacheLock; use NinjaMutex\Lock\MemcachedLock; use NinjaMutex\Lock\MySqlLock; +use NinjaMutex\Lock\Fabric\MemcacheLockFabric; +use NinjaMutex\Lock\Fabric\MemcachedLockFabric; use NinjaMutex\Mock\MockMemcache; use NinjaMutex\Mock\MockMemcached; use NinjaMutex\Mock\MockPredisClient; @@ -53,6 +53,9 @@ public function tearDown() */ public function lockImplementorProvider() { + $memcacheLockFabric = new MemcacheLockFabric(); + $memcachedLockFabric = new MemcachedLockFabric(); + $data = array( // Just mocks $this->provideFlockMockLock(), @@ -62,8 +65,8 @@ public function lockImplementorProvider() $this->providePredisRedisMockLock(), // Real locks $this->provideFlockLock(), - $this->provideMemcacheLock(), - $this->provideMemcachedLock(), + $memcacheLockFabric->create(), + $memcachedLockFabric->create(), $this->provideMysqlLock(), $this->providePredisRedisLock(), ); @@ -89,18 +92,10 @@ public function lockImplementorWithBackendProvider() /** * @return array */ - public function lockImplementorWithExpirationProvider() + public function lockFabricWithExpirationProvider() { - // $self = $this is for compatibility with php 5.3 - // $this in lambda functions was introduced in php 5.4 - // http://php.net/manual/en/functions.anonymous.php - $self = $this; - $memcacheLockFabric = function($expiration = 0) use ($self) { - return $self->createMemcacheLock($expiration); - }; - $memcachedLockFabric = function($expiration = 0) use ($self) { - return $self->createMemcachedLock($expiration); - }; + $memcacheLockFabric = new MemcacheLockFabric(); + $memcachedLockFabric = new MemcachedLockFabric(); $data = array( array($memcacheLockFabric), @@ -156,46 +151,6 @@ protected function providePredisRedisMockLock() return array(new PredisRedisLock($predisMock), $predisMock); } - /** - * @param int $expiration - * @return MemcacheLock - */ - public function createMemcacheLock($expiration = 0) - { - $memcache = new Memcache(); - $memcache->connect('127.0.0.1', 11211); - - return new MemcacheLock($memcache, $expiration); - } - - /** - * @param int $expiration - * @return MemcachedLock - */ - public function createMemcachedLock($expiration = 0) - { - $memcached = new Memcached(); - $memcached->addServer('127.0.0.1', 11211); - - return new MemcachedLock($memcached, $expiration); - } - - /** - * @return array - */ - protected function provideMemcacheLock() - { - return array($this->createMemcacheLock()); - } - - /** - * @return array - */ - protected function provideMemcachedLock() - { - return array($this->createMemcachedLock()); - } - /** * @return array */ diff --git a/tests/NinjaMutex/Lock/Fabric/LockFabricWithExpirationInterface.php b/tests/NinjaMutex/Lock/Fabric/LockFabricWithExpirationInterface.php new file mode 100644 index 0000000..2ba96f2 --- /dev/null +++ b/tests/NinjaMutex/Lock/Fabric/LockFabricWithExpirationInterface.php @@ -0,0 +1,25 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ +namespace NinjaMutex\Lock\Fabric; +use NinjaMutex\Lock\LockInterface; +use NinjaMutex\Lock\LockExpirationInterface; + +/** + * Lock Fabric interface + * + * @author Kamil Dziedzic + */ +interface LockFabricWithExpirationInterface +{ + /** + * @return LockInterface|LockExpirationInterface + */ + public function create(); +} diff --git a/tests/NinjaMutex/Lock/Fabric/MemcacheLockFabric.php b/tests/NinjaMutex/Lock/Fabric/MemcacheLockFabric.php new file mode 100644 index 0000000..70928f9 --- /dev/null +++ b/tests/NinjaMutex/Lock/Fabric/MemcacheLockFabric.php @@ -0,0 +1,25 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ +namespace NinjaMutex\Lock\Fabric; + +use Memcache; +use NinjaMutex\Lock\MemcacheLock; + +class MemcacheLockFabric { + /** + * @return MemcacheLock + */ + public function create() { + $memcache = new Memcache(); + $memcache->connect('127.0.0.1', 11211); + + return new MemcacheLock($memcache); + } +} diff --git a/tests/NinjaMutex/Lock/Fabric/MemcachedLockFabric.php b/tests/NinjaMutex/Lock/Fabric/MemcachedLockFabric.php new file mode 100644 index 0000000..b845a57 --- /dev/null +++ b/tests/NinjaMutex/Lock/Fabric/MemcachedLockFabric.php @@ -0,0 +1,25 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ +namespace NinjaMutex\Lock\Fabric; + +use Memcached; +use NinjaMutex\Lock\MemcachedLock; + +class MemcachedLockFabric { + /** + * @return MemcachedLock + */ + public function create() { + $memcached = new Memcached(); + $memcached->connect('127.0.0.1', 11211); + + return new MemcachedLock($memcached); + } +} diff --git a/tests/NinjaMutex/Lock/LockTest.php b/tests/NinjaMutex/Lock/LockTest.php index 213559a..9d6123e 100644 --- a/tests/NinjaMutex/Lock/LockTest.php +++ b/tests/NinjaMutex/Lock/LockTest.php @@ -10,6 +10,7 @@ namespace NinjaMutex\Lock; use NinjaMutex\AbstractTest; +use NinjaMutex\Lock\Fabric\LockFabricWithExpirationInterface; use NinjaMutex\Mock\PermanentServiceInterface; use NinjaMutex\UnrecoverableMutexException; @@ -167,15 +168,15 @@ public function testIfLockDestructorThrowsWhenBackendIsUnavailable(LockInterface * @medium Timeout for test increased to ~5s http://stackoverflow.com/a/10535787/916440 * @runInSeparateProcess * - * @dataProvider lockImplementorWithExpirationProvider - * @param $lockImplementorFabric + * @dataProvider lockFabricWithExpirationProvider + * @param LockFabricWithExpirationInterface $lockFabricWithExpiration */ - public function testExpiration($lockImplementorFabric) + public function testExpiration(LockFabricWithExpirationInterface $lockFabricWithExpiration) { $expiration = 2; // in seconds $name = "lockWithExpiration_" . uniqid(); - $lockImplementorWithExpiration = $lockImplementorFabric($expiration); - $lockImplementor = $lockImplementorFabric(); + $lockImplementorWithExpiration = $lockFabricWithExpiration->create()->setExpiration($expiration); + $lockImplementor = $lockFabricWithExpiration->create(); // Aquire lock on implementor with lock expiration $this->assertTrue($lockImplementorWithExpiration->acquireLock($name, 0)); From dc7a6c18f6a3130ba12fcaa90ec1e22b03fc889f Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Fri, 12 Jun 2015 22:41:32 +0200 Subject: [PATCH 22/32] Bugfix, bad copy & paste --- tests/NinjaMutex/Lock/Fabric/MemcachedLockFabric.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/NinjaMutex/Lock/Fabric/MemcachedLockFabric.php b/tests/NinjaMutex/Lock/Fabric/MemcachedLockFabric.php index b845a57..d240551 100644 --- a/tests/NinjaMutex/Lock/Fabric/MemcachedLockFabric.php +++ b/tests/NinjaMutex/Lock/Fabric/MemcachedLockFabric.php @@ -18,7 +18,7 @@ class MemcachedLockFabric { */ public function create() { $memcached = new Memcached(); - $memcached->connect('127.0.0.1', 11211); + $memcached->addServer('127.0.0.1', 11211); return new MemcachedLock($memcached); } From d9517d3965d9cb51e76a70fb84cccef0f0b6e6b5 Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Fri, 12 Jun 2015 22:48:21 +0200 Subject: [PATCH 23/32] bugfix: invalid provider --- tests/NinjaMutex/AbstractTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/NinjaMutex/AbstractTest.php b/tests/NinjaMutex/AbstractTest.php index 0c35655..9b94711 100644 --- a/tests/NinjaMutex/AbstractTest.php +++ b/tests/NinjaMutex/AbstractTest.php @@ -65,8 +65,8 @@ public function lockImplementorProvider() $this->providePredisRedisMockLock(), // Real locks $this->provideFlockLock(), - $memcacheLockFabric->create(), - $memcachedLockFabric->create(), + array($memcacheLockFabric->create()), + array($memcachedLockFabric->create()), $this->provideMysqlLock(), $this->providePredisRedisLock(), ); From 7fda3f7236de39b7d44a1f10c7190e2b27cfac9c Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Fri, 12 Jun 2015 23:08:41 +0200 Subject: [PATCH 24/32] Let's try to be compatibile with travis version --- tests/NinjaMutex/Mock/MockMemcached.php | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/NinjaMutex/Mock/MockMemcached.php b/tests/NinjaMutex/Mock/MockMemcached.php index 77f6f27..593fe72 100644 --- a/tests/NinjaMutex/Mock/MockMemcached.php +++ b/tests/NinjaMutex/Mock/MockMemcached.php @@ -34,12 +34,13 @@ public function __construct() } /** - * @param string $key - * @param mixed $value - * @param null $expiration + * @param string $key + * @param mixed $value + * @param int|null $expiration + * @param null $udf_flags * @return bool */ - public function add($key, $value, $expiration = null) + public function add($key, $value, $expiration = NULL, &$udf_flags = NULL) { if (!$this->available) { return false; @@ -58,9 +59,10 @@ public function add($key, $value, $expiration = null) * @param string $key * @param null $cache_cb * @param null $cas_token + * @param null $udf_flags * @return bool|mixed|string */ - public function get($key, $cache_cb = null, &$cas_token = null) + public function get($key, $cache_cb = NULL, &$cas_token = NULL, &$udf_flags = NULL) { if (!$this->available) { return false; From 97d4b566971b66e43337b65da247673577f5eba9 Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Fri, 12 Jun 2015 23:18:10 +0200 Subject: [PATCH 25/32] No idea what's still not compatibile --- tests/NinjaMutex/Mock/MockMemcached.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/NinjaMutex/Mock/MockMemcached.php b/tests/NinjaMutex/Mock/MockMemcached.php index 593fe72..1b02115 100644 --- a/tests/NinjaMutex/Mock/MockMemcached.php +++ b/tests/NinjaMutex/Mock/MockMemcached.php @@ -40,7 +40,7 @@ public function __construct() * @param null $udf_flags * @return bool */ - public function add($key, $value, $expiration = NULL, &$udf_flags = NULL) + public function add($key, $value, $expiration = null, $udf_flags = null) { if (!$this->available) { return false; @@ -62,7 +62,7 @@ public function add($key, $value, $expiration = NULL, &$udf_flags = NULL) * @param null $udf_flags * @return bool|mixed|string */ - public function get($key, $cache_cb = NULL, &$cas_token = NULL, &$udf_flags = NULL) + public function get($key, $cache_cb = null, $cas_token = null, $udf_flags = null) { if (!$this->available) { return false; From 4d474f9b342ce1e3318babc58b50470e94e9326d Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Sat, 13 Jun 2015 00:17:24 +0200 Subject: [PATCH 26/32] Let's try to trick this broken memcached extension https://github.com/php-memcached-dev/php-memcached/issues/126 --- src/NinjaMutex/Lock/MemcachedLock.php | 2 +- tests/NinjaMutex/Mock/MockMemcached.php | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/NinjaMutex/Lock/MemcachedLock.php b/src/NinjaMutex/Lock/MemcachedLock.php index 0d14669..21c1ca1 100644 --- a/src/NinjaMutex/Lock/MemcachedLock.php +++ b/src/NinjaMutex/Lock/MemcachedLock.php @@ -39,7 +39,7 @@ class MemcachedLock extends LockAbstract implements LockExpirationInterface /** * @param Memcached $memcached */ - public function __construct(Memcached $memcached) + public function __construct($memcached) { parent::__construct(); diff --git a/tests/NinjaMutex/Mock/MockMemcached.php b/tests/NinjaMutex/Mock/MockMemcached.php index 1b02115..b6c02d7 100644 --- a/tests/NinjaMutex/Mock/MockMemcached.php +++ b/tests/NinjaMutex/Mock/MockMemcached.php @@ -9,14 +9,13 @@ */ namespace NinjaMutex\Mock; -use Memcached; /** * Mock memcached to mimic mutex functionality * * @author Kamil Dziedzic */ -class MockMemcached extends Memcached implements PermanentServiceInterface +class MockMemcached implements PermanentServiceInterface { /** * @var string[] @@ -40,7 +39,7 @@ public function __construct() * @param null $udf_flags * @return bool */ - public function add($key, $value, $expiration = null, $udf_flags = null) + public function add($key, $value, $expiration = null, &$udf_flags = null) { if (!$this->available) { return false; @@ -62,7 +61,7 @@ public function add($key, $value, $expiration = null, $udf_flags = null) * @param null $udf_flags * @return bool|mixed|string */ - public function get($key, $cache_cb = null, $cas_token = null, $udf_flags = null) + public function get($key, $cache_cb = null, &$cas_token = null, &$udf_flags = null) { if (!$this->available) { return false; From d85075a18efaa6d647c76f1e4e83a0d3829a6d60 Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Sat, 13 Jun 2015 00:32:57 +0200 Subject: [PATCH 27/32] Implement proper interfaces --- tests/NinjaMutex/Lock/Fabric/MemcacheLockFabric.php | 2 +- tests/NinjaMutex/Lock/Fabric/MemcachedLockFabric.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/NinjaMutex/Lock/Fabric/MemcacheLockFabric.php b/tests/NinjaMutex/Lock/Fabric/MemcacheLockFabric.php index 70928f9..1ceea5d 100644 --- a/tests/NinjaMutex/Lock/Fabric/MemcacheLockFabric.php +++ b/tests/NinjaMutex/Lock/Fabric/MemcacheLockFabric.php @@ -12,7 +12,7 @@ use Memcache; use NinjaMutex\Lock\MemcacheLock; -class MemcacheLockFabric { +class MemcacheLockFabric implements LockFabricWithExpirationInterface { /** * @return MemcacheLock */ diff --git a/tests/NinjaMutex/Lock/Fabric/MemcachedLockFabric.php b/tests/NinjaMutex/Lock/Fabric/MemcachedLockFabric.php index d240551..11772c0 100644 --- a/tests/NinjaMutex/Lock/Fabric/MemcachedLockFabric.php +++ b/tests/NinjaMutex/Lock/Fabric/MemcachedLockFabric.php @@ -12,7 +12,7 @@ use Memcached; use NinjaMutex\Lock\MemcachedLock; -class MemcachedLockFabric { +class MemcachedLockFabric implements LockFabricWithExpirationInterface { /** * @return MemcachedLock */ From f42fccbc4b6024ba063fdcc2aac960ef1684fc0d Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Sat, 13 Jun 2015 00:40:53 +0200 Subject: [PATCH 28/32] It's not chainable --- tests/NinjaMutex/Lock/LockTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/NinjaMutex/Lock/LockTest.php b/tests/NinjaMutex/Lock/LockTest.php index 9d6123e..a7a4697 100644 --- a/tests/NinjaMutex/Lock/LockTest.php +++ b/tests/NinjaMutex/Lock/LockTest.php @@ -175,8 +175,9 @@ public function testExpiration(LockFabricWithExpirationInterface $lockFabricWith { $expiration = 2; // in seconds $name = "lockWithExpiration_" . uniqid(); - $lockImplementorWithExpiration = $lockFabricWithExpiration->create()->setExpiration($expiration); $lockImplementor = $lockFabricWithExpiration->create(); + $lockImplementorWithExpiration = $lockFabricWithExpiration->create(); + $lockImplementorWithExpiration->setExpiration($expiration); // Aquire lock on implementor with lock expiration $this->assertTrue($lockImplementorWithExpiration->acquireLock($name, 0)); From 5bd91ec82386836497094c377197023fdf121526 Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Sat, 13 Jun 2015 00:54:09 +0200 Subject: [PATCH 29/32] Let's finally try to expect Fatal Error --- tests/NinjaMutex/Lock/LockTest.php | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/NinjaMutex/Lock/LockTest.php b/tests/NinjaMutex/Lock/LockTest.php index a7a4697..6000d6e 100644 --- a/tests/NinjaMutex/Lock/LockTest.php +++ b/tests/NinjaMutex/Lock/LockTest.php @@ -168,6 +168,7 @@ public function testIfLockDestructorThrowsWhenBackendIsUnavailable(LockInterface * @medium Timeout for test increased to ~5s http://stackoverflow.com/a/10535787/916440 * @runInSeparateProcess * + * @expectedException PHPUnit_Framework_Error * @dataProvider lockFabricWithExpirationProvider * @param LockFabricWithExpirationInterface $lockFabricWithExpiration */ @@ -194,15 +195,14 @@ public function testExpiration(LockFabricWithExpirationInterface $lockFabricWith $this->assertTrue($lockImplementor->releaseLock($name, 0)); // Now we set null to the Mutex with lock expiration to invoke __destructor - try { - $lockImplementorWithExpiration = null; - } catch (UnrecoverableMutexException $e) { - // hhvm doesn't throw an exception here, it rather raises a fatal error, - // so I can't check here if Exception was really raised for all builds. - // Looks like I should always raise fatal error in __destructor for all versions rather than trying to raise exception - // https://github.com/facebook/hhvm/blob/af329776c9f740cc1c8c4791f673ba5aa49042ce/hphp/doc/inconsistencies#L40-L48 - // http://docs.hhvm.com/manual/en/language.oop5.decon.php#language.oop5.decon.destructor - // https://github.com/sebastianbergmann/phpunit/issues/1640 - } + // which throws UnrecoverableMutexException + $lockImplementorWithExpiration = null; + + // hhvm doesn't throw an exception here, it rather raises a fatal error, + // so I can't check here if Exception was really raised for all builds but I can check if script ended with Fatal Error. + // Looks like I should always raise fatal error in __destructor for all versions rather than trying to raise exception + // https://github.com/facebook/hhvm/blob/af329776c9f740cc1c8c4791f673ba5aa49042ce/hphp/doc/inconsistencies#L40-L48 + // http://docs.hhvm.com/manual/en/language.oop5.decon.php#language.oop5.decon.destructor + // https://github.com/sebastianbergmann/phpunit/issues/1640 } } From 030f32ba7a356f5972891819577176c8b2b5caf6 Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Sat, 13 Jun 2015 00:56:45 +0200 Subject: [PATCH 30/32] Test alternative approach --- tests/NinjaMutex/Lock/LockTest.php | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/NinjaMutex/Lock/LockTest.php b/tests/NinjaMutex/Lock/LockTest.php index 6000d6e..6919130 100644 --- a/tests/NinjaMutex/Lock/LockTest.php +++ b/tests/NinjaMutex/Lock/LockTest.php @@ -168,7 +168,6 @@ public function testIfLockDestructorThrowsWhenBackendIsUnavailable(LockInterface * @medium Timeout for test increased to ~5s http://stackoverflow.com/a/10535787/916440 * @runInSeparateProcess * - * @expectedException PHPUnit_Framework_Error * @dataProvider lockFabricWithExpirationProvider * @param LockFabricWithExpirationInterface $lockFabricWithExpiration */ @@ -195,14 +194,15 @@ public function testExpiration(LockFabricWithExpirationInterface $lockFabricWith $this->assertTrue($lockImplementor->releaseLock($name, 0)); // Now we set null to the Mutex with lock expiration to invoke __destructor - // which throws UnrecoverableMutexException - $lockImplementorWithExpiration = null; - - // hhvm doesn't throw an exception here, it rather raises a fatal error, - // so I can't check here if Exception was really raised for all builds but I can check if script ended with Fatal Error. - // Looks like I should always raise fatal error in __destructor for all versions rather than trying to raise exception - // https://github.com/facebook/hhvm/blob/af329776c9f740cc1c8c4791f673ba5aa49042ce/hphp/doc/inconsistencies#L40-L48 - // http://docs.hhvm.com/manual/en/language.oop5.decon.php#language.oop5.decon.destructor - // https://github.com/sebastianbergmann/phpunit/issues/1640 + try { + $lockImplementorWithExpiration = null; + } catch (Exception $e) { + // hhvm doesn't throw an exception here, it rather raises a fatal error, + // so I can't check here if Exception was really raised for all builds. + // Looks like I should always raise fatal error in __destructor for all versions rather than trying to raise exception + // https://github.com/facebook/hhvm/blob/af329776c9f740cc1c8c4791f673ba5aa49042ce/hphp/doc/inconsistencies#L40-L48 + // http://docs.hhvm.com/manual/en/language.oop5.decon.php#language.oop5.decon.destructor + // https://github.com/sebastianbergmann/phpunit/issues/1640 + } } } From 3f0b3c5240e77e24832c8eeb1d09cbf9a72c1efd Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Sat, 13 Jun 2015 01:21:12 +0200 Subject: [PATCH 31/32] Let's try this --- src/NinjaMutex/Lock/MemcacheLock.php | 2 +- src/NinjaMutex/Lock/MemcachedLock.php | 2 +- tests/NinjaMutex/Lock/LockTest.php | 14 +------------- 3 files changed, 3 insertions(+), 15 deletions(-) diff --git a/src/NinjaMutex/Lock/MemcacheLock.php b/src/NinjaMutex/Lock/MemcacheLock.php index f306c8e..e225d93 100644 --- a/src/NinjaMutex/Lock/MemcacheLock.php +++ b/src/NinjaMutex/Lock/MemcacheLock.php @@ -83,7 +83,7 @@ protected function getLock($name, $blocking) */ public function releaseLock($name) { - if (isset($this->locks[$name]) && $this->memcache->delete($name)) { + if (isset($this->locks[$name]) && ($this->memcache->delete($name) || !$this->isLocked($name))) { unset($this->locks[$name]); return true; diff --git a/src/NinjaMutex/Lock/MemcachedLock.php b/src/NinjaMutex/Lock/MemcachedLock.php index 21c1ca1..61cd465 100644 --- a/src/NinjaMutex/Lock/MemcachedLock.php +++ b/src/NinjaMutex/Lock/MemcachedLock.php @@ -83,7 +83,7 @@ protected function getLock($name, $blocking) */ public function releaseLock($name) { - if (isset($this->locks[$name]) && $this->memcached->delete($name)) { + if (isset($this->locks[$name]) && ($this->memcached->delete($name) || !$this->isLocked($name))) { unset($this->locks[$name]); return true; diff --git a/tests/NinjaMutex/Lock/LockTest.php b/tests/NinjaMutex/Lock/LockTest.php index 6919130..7f78180 100644 --- a/tests/NinjaMutex/Lock/LockTest.php +++ b/tests/NinjaMutex/Lock/LockTest.php @@ -166,7 +166,6 @@ public function testIfLockDestructorThrowsWhenBackendIsUnavailable(LockInterface /** * @issue https://github.com/arvenil/ninja-mutex/issues/12 * @medium Timeout for test increased to ~5s http://stackoverflow.com/a/10535787/916440 - * @runInSeparateProcess * * @dataProvider lockFabricWithExpirationProvider * @param LockFabricWithExpirationInterface $lockFabricWithExpiration @@ -192,17 +191,6 @@ public function testExpiration(LockFabricWithExpirationInterface $lockFabricWith // Cleanup $this->assertTrue($lockImplementor->releaseLock($name, 0)); - - // Now we set null to the Mutex with lock expiration to invoke __destructor - try { - $lockImplementorWithExpiration = null; - } catch (Exception $e) { - // hhvm doesn't throw an exception here, it rather raises a fatal error, - // so I can't check here if Exception was really raised for all builds. - // Looks like I should always raise fatal error in __destructor for all versions rather than trying to raise exception - // https://github.com/facebook/hhvm/blob/af329776c9f740cc1c8c4791f673ba5aa49042ce/hphp/doc/inconsistencies#L40-L48 - // http://docs.hhvm.com/manual/en/language.oop5.decon.php#language.oop5.decon.destructor - // https://github.com/sebastianbergmann/phpunit/issues/1640 - } + $this->assertTrue($lockImplementorWithExpiration->releaseLock($name, 0)); } } From d4f516f7faa27ebb2871fa145de51047a8ef080d Mon Sep 17 00:00:00 2001 From: Kamil Dziedzic Date: Sat, 13 Jun 2015 01:34:10 +0200 Subject: [PATCH 32/32] Looks like we need separate method --- .../Lock/LockExpirationInterface.php | 6 ++++++ src/NinjaMutex/Lock/MemcacheLock.php | 19 ++++++++++++++++++- src/NinjaMutex/Lock/MemcachedLock.php | 19 ++++++++++++++++++- tests/NinjaMutex/Lock/LockTest.php | 4 +++- 4 files changed, 45 insertions(+), 3 deletions(-) diff --git a/src/NinjaMutex/Lock/LockExpirationInterface.php b/src/NinjaMutex/Lock/LockExpirationInterface.php index eeb8e83..d169acb 100644 --- a/src/NinjaMutex/Lock/LockExpirationInterface.php +++ b/src/NinjaMutex/Lock/LockExpirationInterface.php @@ -20,4 +20,10 @@ interface LockExpirationInterface * @param int $expiration Expiration time of the lock in seconds. */ public function setExpiration($expiration); + + /** + * @param string $name + * @return bool + */ + public function clearLock($name); } diff --git a/src/NinjaMutex/Lock/MemcacheLock.php b/src/NinjaMutex/Lock/MemcacheLock.php index e225d93..b6b2f53 100644 --- a/src/NinjaMutex/Lock/MemcacheLock.php +++ b/src/NinjaMutex/Lock/MemcacheLock.php @@ -61,6 +61,23 @@ public function setExpiration($expiration) $this->expiration = $expiration; } + /** + * Clear lock without releasing it + * Do not use this method unless you know what you do + * + * @param string $name name of lock + * @return bool + */ + public function clearLock($name) + { + if (!isset($this->locks[$name])) { + return false; + } + + unset($this->locks[$name]); + return true; + } + /** * @param string $name name of lock * @param bool $blocking @@ -83,7 +100,7 @@ protected function getLock($name, $blocking) */ public function releaseLock($name) { - if (isset($this->locks[$name]) && ($this->memcache->delete($name) || !$this->isLocked($name))) { + if (isset($this->locks[$name]) && $this->memcache->delete($name)) { unset($this->locks[$name]); return true; diff --git a/src/NinjaMutex/Lock/MemcachedLock.php b/src/NinjaMutex/Lock/MemcachedLock.php index 61cd465..88a2051 100644 --- a/src/NinjaMutex/Lock/MemcachedLock.php +++ b/src/NinjaMutex/Lock/MemcachedLock.php @@ -61,6 +61,23 @@ public function setExpiration($expiration) $this->expiration = $expiration; } + /** + * Clear lock without releasing it + * Do not use this method unless you know what you do + * + * @param string $name name of lock + * @return bool + */ + public function clearLock($name) + { + if (!isset($this->locks[$name])) { + return false; + } + + unset($this->locks[$name]); + return true; + } + /** * @param string $name name of lock * @param bool $blocking @@ -83,7 +100,7 @@ protected function getLock($name, $blocking) */ public function releaseLock($name) { - if (isset($this->locks[$name]) && ($this->memcached->delete($name) || !$this->isLocked($name))) { + if (isset($this->locks[$name]) && $this->memcached->delete($name)) { unset($this->locks[$name]); return true; diff --git a/tests/NinjaMutex/Lock/LockTest.php b/tests/NinjaMutex/Lock/LockTest.php index 7f78180..5249c20 100644 --- a/tests/NinjaMutex/Lock/LockTest.php +++ b/tests/NinjaMutex/Lock/LockTest.php @@ -191,6 +191,8 @@ public function testExpiration(LockFabricWithExpirationInterface $lockFabricWith // Cleanup $this->assertTrue($lockImplementor->releaseLock($name, 0)); - $this->assertTrue($lockImplementorWithExpiration->releaseLock($name, 0)); + // Expired lock is unusable, we need to clean it's lock state or otherwise + // it will invoke in __destruct Exception (php*) or Fatal Error (hhvm) + $this->assertTrue($lockImplementorWithExpiration->clearLock($name, 0)); } }