From aac94d1d7445f2287f89a3559f4b3988e39edbdb Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Wed, 4 Jul 2018 23:37:22 +0200 Subject: [PATCH] Adding multihost - locking Adding Unit-Tests for it --- boot.php | 2 +- src/Core/Cache/ArrayCache.php | 83 ++++++++++++++++ src/Core/Cache/DatabaseCacheDriver.php | 4 +- src/Core/Cache/ICacheDriver.php | 15 ++- src/Core/Cache/IMemoryCacheDriver.php | 45 +++++++++ src/Core/Cache/MemcacheCacheDriver.php | 48 ++++++++-- src/Core/Cache/MemcachedCacheDriver.php | 38 ++++++-- src/Core/Cache/RedisCacheDriver.php | 92 ++++++++++++++++-- src/Core/Cache/TraitCompareDelete.php | 45 +++++++++ src/Core/Cache/TraitCompareSet.php | 48 ++++++++++ src/Core/Lock.php | 17 +++- src/Core/Lock/AbstractLockDriver.php | 7 +- src/Core/Lock/CacheLockDriver.php | 66 +++++++------ src/Core/Lock/DatabaseLockDriver.php | 53 +++++----- src/Core/Lock/ILockDriver.php | 8 ++ src/Core/Lock/SemaphoreLockDriver.php | 31 ++++-- src/Database/DBStructure.php | 7 +- tests/datasets/api.yml | 96 +++++++++---------- tests/src/Core/Lock/CacheLockDriverTest.php | 27 ++++++ .../src/Core/Lock/DatabaseLockDriverTest.php | 67 +++++++++++++ tests/src/Core/Lock/LockTest.php | 80 ++++++++++++++++ .../src/Core/Lock/SemaphoreLockDriverTest.php | 14 +++ 22 files changed, 740 insertions(+), 153 deletions(-) create mode 100644 src/Core/Cache/ArrayCache.php create mode 100644 src/Core/Cache/IMemoryCacheDriver.php create mode 100644 src/Core/Cache/TraitCompareDelete.php create mode 100644 src/Core/Cache/TraitCompareSet.php create mode 100644 tests/src/Core/Lock/CacheLockDriverTest.php create mode 100644 tests/src/Core/Lock/DatabaseLockDriverTest.php create mode 100644 tests/src/Core/Lock/LockTest.php create mode 100644 tests/src/Core/Lock/SemaphoreLockDriverTest.php diff --git a/boot.php b/boot.php index 864e7804c9..786a846f32 100644 --- a/boot.php +++ b/boot.php @@ -41,7 +41,7 @@ define('FRIENDICA_PLATFORM', 'Friendica'); define('FRIENDICA_CODENAME', 'The Tazmans Flax-lily'); define('FRIENDICA_VERSION', '2018.08-dev'); define('DFRN_PROTOCOL_VERSION', '2.23'); -define('DB_UPDATE_VERSION', 1272); +define('DB_UPDATE_VERSION', 1273); define('NEW_UPDATE_ROUTINE_VERSION', 1170); /** diff --git a/src/Core/Cache/ArrayCache.php b/src/Core/Cache/ArrayCache.php new file mode 100644 index 0000000000..d4fe8bc7f7 --- /dev/null +++ b/src/Core/Cache/ArrayCache.php @@ -0,0 +1,83 @@ +cachedData[$key])) { + return $this->cachedData[$key]; + } + return null; + } + + /** + * (@inheritdoc) + */ + public function set($key, $value, $ttl = Cache::FIVE_MINUTES) + { + $this->cachedData[$key] = $value; + return true; + } + + /** + * (@inheritdoc) + */ + public function delete($key) + { + unset($this->cachedData[$key]); + return true; + } + + /** + * (@inheritdoc) + */ + public function clear() + { + $this->cachedData = []; + return true; + } + + /** + * (@inheritdoc) + */ + public function add($key, $value, $ttl = Cache::FIVE_MINUTES) + { + if (isset($this->cachedData[$key])) { + return false; + } else { + return $this->set($key, $value, $ttl); + } + } + + /** + * (@inheritdoc) + */ + public function compareSet($key, $oldValue, $newValue, $ttl = Cache::FIVE_MINUTES) + { + if ($this->get($key) === $oldValue) { + return $this->set($key, $newValue); + } else { + return false; + } + } +} \ No newline at end of file diff --git a/src/Core/Cache/DatabaseCacheDriver.php b/src/Core/Cache/DatabaseCacheDriver.php index 7248e0b349..0838a66c7a 100644 --- a/src/Core/Cache/DatabaseCacheDriver.php +++ b/src/Core/Cache/DatabaseCacheDriver.php @@ -33,11 +33,11 @@ class DatabaseCacheDriver implements ICacheDriver return null; } - public function set($key, $value, $duration = Cache::MONTH) + public function set($key, $value, $ttl = Cache::FIVE_MINUTES) { $fields = [ 'v' => serialize($value), - 'expires' => DateTimeFormat::utc('now + ' . $duration . ' seconds'), + 'expires' => DateTimeFormat::utc('now + ' . $ttl . ' seconds'), 'updated' => DateTimeFormat::utcNow() ]; diff --git a/src/Core/Cache/ICacheDriver.php b/src/Core/Cache/ICacheDriver.php index be896edf7f..ff329f34eb 100644 --- a/src/Core/Cache/ICacheDriver.php +++ b/src/Core/Cache/ICacheDriver.php @@ -12,7 +12,7 @@ use Friendica\Core\Cache; interface ICacheDriver { /** - * Fetches cached data according to the key + * @brief Fetches cached data according to the key * * @param string $key The key to the cached data * @@ -21,28 +21,27 @@ interface ICacheDriver public function get($key); /** - * Stores data in the cache identified by the key. The input $value can have multiple formats. + * @brief Stores data in the cache identified by the key. The input $value can have multiple formats. * * @param string $key The cache key * @param mixed $value The value to store - * @param integer $duration The cache lifespan, must be one of the Cache constants + * @param integer $ttl The cache lifespan, must be one of the Cache constants * * @return bool */ - public function set($key, $value, $duration = Cache::MONTH); - + public function set($key, $value, $ttl = Cache::FIVE_MINUTES); /** - * Delete a key from the cache + * @brief Delete a key from the cache * - * @param string $key + * @param string $key The cache key * * @return bool */ public function delete($key); /** - * Remove outdated data from the cache + * @brief Remove outdated data from the cache * * @return bool */ diff --git a/src/Core/Cache/IMemoryCacheDriver.php b/src/Core/Cache/IMemoryCacheDriver.php new file mode 100644 index 0000000000..7843ca7b5f --- /dev/null +++ b/src/Core/Cache/IMemoryCacheDriver.php @@ -0,0 +1,45 @@ + */ -class MemcacheCacheDriver extends BaseObject implements ICacheDriver +class MemcacheCacheDriver extends BaseObject implements IMemoryCacheDriver { + use TraitCompareSet; + use TraitCompareDelete; + /** - * @var Memcache + * @var \Memcache */ private $memcache; @@ -30,6 +33,9 @@ class MemcacheCacheDriver extends BaseObject implements ICacheDriver } } + /** + * (@inheritdoc) + */ public function get($key) { $return = null; @@ -54,17 +60,31 @@ class MemcacheCacheDriver extends BaseObject implements ICacheDriver return $return; } - public function set($key, $value, $duration = Cache::MONTH) + /** + * (@inheritdoc) + */ + public function set($key, $value, $ttl = Cache::FIVE_MINUTES) { // We store with the hostname as key to avoid problems with other applications - return $this->memcache->set( - self::getApp()->get_hostname() . ":" . $key, - serialize($value), - MEMCACHE_COMPRESSED, - time() + $duration - ); + if ($ttl > 0) { + return $this->memcache->set( + self::getApp()->get_hostname() . ":" . $key, + serialize($value), + MEMCACHE_COMPRESSED, + time() + $ttl + ); + } else { + return $this->memcache->set( + self::getApp()->get_hostname() . ":" . $key, + serialize($value), + MEMCACHE_COMPRESSED + ); + } } + /** + * (@inheritdoc) + */ public function delete($key) { return $this->memcache->delete($key); @@ -72,6 +92,14 @@ class MemcacheCacheDriver extends BaseObject implements ICacheDriver public function clear() { - return true; + return $this->memcache->flush(); + } + + /** + * (@inheritdoc) + */ + public function add($key, $value, $ttl = Cache::FIVE_MINUTES) + { + return $this->memcache->add(self::getApp()->get_hostname() . ":" . $key, $value, $ttl); } } diff --git a/src/Core/Cache/MemcachedCacheDriver.php b/src/Core/Cache/MemcachedCacheDriver.php index 78a4a6bdfe..9c860711f8 100644 --- a/src/Core/Cache/MemcachedCacheDriver.php +++ b/src/Core/Cache/MemcachedCacheDriver.php @@ -10,8 +10,11 @@ use Friendica\Core\Cache; * * @author Hypolite Petovan */ -class MemcachedCacheDriver extends BaseObject implements ICacheDriver +class MemcachedCacheDriver extends BaseObject implements IMemoryCacheDriver { + use TraitCompareSet; + use TraitCompareDelete; + /** * @var Memcached */ @@ -46,14 +49,22 @@ class MemcachedCacheDriver extends BaseObject implements ICacheDriver return $return; } - public function set($key, $value, $duration = Cache::MONTH) + public function set($key, $value, $ttl = Cache::FIVE_MINUTES) { // We store with the hostname as key to avoid problems with other applications - return $this->memcached->set( - self::getApp()->get_hostname() . ':' . $key, - $value, - time() + $duration - ); + if ($ttl > 0) { + return $this->memcached->set( + self::getApp()->get_hostname() . ':' . $key, + $value, + time() + $ttl + ); + } else { + return $this->memcached->set( + self::getApp()->get_hostname() . ':' . $key, + $value + ); + } + } public function delete($key) @@ -67,4 +78,17 @@ class MemcachedCacheDriver extends BaseObject implements ICacheDriver { return true; } + + /** + * @brief Sets a value if it's not already stored + * + * @param string $key The cache key + * @param mixed $value The old value we know from the cache + * @param int $ttl The cache lifespan, must be one of the Cache constants + * @return bool + */ + public function add($key, $value, $ttl = Cache::FIVE_MINUTES) + { + return $this->memcached->add(self::getApp()->get_hostname() . ":" . $key, $value, $ttl); + } } diff --git a/src/Core/Cache/RedisCacheDriver.php b/src/Core/Cache/RedisCacheDriver.php index fa98842da0..d23fa2697b 100644 --- a/src/Core/Cache/RedisCacheDriver.php +++ b/src/Core/Cache/RedisCacheDriver.php @@ -11,7 +11,7 @@ use Friendica\Core\Cache; * @author Hypolite Petovan * @author Roland Haeder */ -class RedisCacheDriver extends BaseObject implements ICacheDriver +class RedisCacheDriver extends BaseObject implements IMemoryCacheDriver { /** * @var Redis @@ -55,14 +55,21 @@ class RedisCacheDriver extends BaseObject implements ICacheDriver return $return; } - public function set($key, $value, $duration = Cache::MONTH) + public function set($key, $value, $ttl = Cache::FIVE_MINUTES) { // We store with the hostname as key to avoid problems with other applications - return $this->redis->set( - self::getApp()->get_hostname() . ":" . $key, - serialize($value), - time() + $duration - ); + if ($ttl > 0) { + return $this->redis->setex( + self::getApp()->get_hostname() . ":" . $key, + time() + $ttl, + serialize($value) + ); + } else { + return $this->redis->set( + self::getApp()->get_hostname() . ":" . $key, + serialize($value) + ); + } } public function delete($key) @@ -74,4 +81,75 @@ class RedisCacheDriver extends BaseObject implements ICacheDriver { return true; } + + + /** + * @brief Sets a value if it's not already stored + * + * @param string $key The cache key + * @param mixed $value The old value we know from the cache + * @param int $ttl The cache lifespan, must be one of the Cache constants + * @return bool + */ + public function add($key, $value, $ttl = Cache::FIVE_MINUTES) + { + if (!is_int($value)) { + $value = serialize($value); + } + + return $this->redis->setnx(self::getApp()->get_hostname() . ":" . $key, $value); + } + + /** + * @brief Compares if the old value is set and sets the new value + * + * @param string $key The cache key + * @param mixed $oldValue The old value we know + * @param mixed $newValue The new value we want to set + * @param int $ttl The cache lifespan, must be one of the Cache constants + * @return bool + */ + public function compareSet($key, $oldValue, $newValue, $ttl = Cache::FIVE_MINUTES) + { + if (!is_int($newValue)) { + $newValue = serialize($newValue); + } + + $this->redis->watch(self::getApp()->get_hostname() . ":" . $key); + // If the old value isn't what we expected, somebody else changed the key meanwhile + if ($this->get($key) === $oldValue) { + if ($ttl > 0) { + $result = $this->redis->multi() + ->setex(self::getApp()->get_hostname() . ":" . $ttl, $key, $newValue) + ->exec(); + } else { + $result = $this->redis->multi() + ->set(self::getApp()->get_hostname() . ":" . $key, $newValue) + ->exec(); + } + return $result !== false; + } + $this->redis->unwatch(); + return false; + } + /** + * @brief Compares if the old value is set and removes it + * + * @param string $key The cache key + * @param mixed $value The old value we know and want to delete + * @return bool + */ + public function compareDelete($key, $value) + { + $this->redis->watch(self::getApp()->get_hostname() . ":" . $key); + // If the old value isn't what we expected, somebody else changed the key meanwhile + if ($this->get($key) === $value) { + $result = $this->redis->multi() + ->del(self::getApp()->get_hostname() . ":" . $key) + ->exec(); + return $result !== false; + } + $this->redis->unwatch(); + return false; + } } diff --git a/src/Core/Cache/TraitCompareDelete.php b/src/Core/Cache/TraitCompareDelete.php new file mode 100644 index 0000000000..898e39aecc --- /dev/null +++ b/src/Core/Cache/TraitCompareDelete.php @@ -0,0 +1,45 @@ +add($key . "_lock", true)) { + if ($this->get($key) === $value) { + $this->delete($key); + $this->delete($key . "_lock"); + return true; + } else { + $this->delete($key . "_lock"); + return false; + } + } else { + return false; + } + } +} \ No newline at end of file diff --git a/src/Core/Cache/TraitCompareSet.php b/src/Core/Cache/TraitCompareSet.php new file mode 100644 index 0000000000..55193b7567 --- /dev/null +++ b/src/Core/Cache/TraitCompareSet.php @@ -0,0 +1,48 @@ +add($key . "_lock", true)) { + if ($this->get($key) === $oldValue) { + $this->set($key, $newValue, $ttl); + $this->delete($key . "_lock"); + return true; + } else { + $this->delete($key . "_lock"); + return false; + } + } else { + return false; + } + } +} \ No newline at end of file diff --git a/src/Core/Lock.php b/src/Core/Lock.php index 7db0ea0db6..7235c64a98 100644 --- a/src/Core/Lock.php +++ b/src/Core/Lock.php @@ -10,6 +10,7 @@ namespace Friendica\Core; */ use Friendica\Core\Cache\CacheDriverFactory; +use Friendica\Core\Cache\IMemoryCacheDriver; /** * @brief This class contain Functions for preventing parallel execution of functions @@ -29,17 +30,23 @@ class Lock switch ($lock_driver) { case 'memcache': $cache_driver = CacheDriverFactory::create('memcache'); - self::$driver = new Lock\CacheLockDriver($cache_driver); + if ($cache_driver instanceof IMemoryCacheDriver) { + self::$driver = new Lock\CacheLockDriver($cache_driver); + } break; case 'memcached': $cache_driver = CacheDriverFactory::create('memcached'); - self::$driver = new Lock\CacheLockDriver($cache_driver); + if ($cache_driver instanceof IMemoryCacheDriver) { + self::$driver = new Lock\CacheLockDriver($cache_driver); + } break; case 'redis': $cache_driver = CacheDriverFactory::create('redis'); - self::$driver = new Lock\CacheLockDriver($cache_driver); + if ($cache_driver instanceof IMemoryCacheDriver) { + self::$driver = new Lock\CacheLockDriver($cache_driver); + } break; case 'database': @@ -85,7 +92,9 @@ class Lock if ($cache_driver != 'database') { try { $lock_driver = CacheDriverFactory::create($cache_driver); - self::$driver = new Lock\CacheLockDriver($lock_driver); + if ($lock_driver instanceof IMemoryCacheDriver) { + self::$driver = new Lock\CacheLockDriver($lock_driver); + } return; } catch (\Exception $exception) { logger('Using Cache driver for locking failed: ' . $exception->getMessage()); diff --git a/src/Core/Lock/AbstractLockDriver.php b/src/Core/Lock/AbstractLockDriver.php index bcce26129c..09549c50bf 100644 --- a/src/Core/Lock/AbstractLockDriver.php +++ b/src/Core/Lock/AbstractLockDriver.php @@ -1,6 +1,7 @@ acquireLock[$key]); + return isset($this->acquireLock[$key]) && $this->acquiredLocks[$key] === true; } /** @@ -50,7 +51,7 @@ abstract class AbstractLockDriver implements ILockDriver * @return void */ public function releaseAll() { - foreach ($this->acquiredLocks as $acquiredLock) { + foreach ($this->acquiredLocks as $acquiredLock => $hasLock) { $this->releaseLock($acquiredLock); } } diff --git a/src/Core/Lock/CacheLockDriver.php b/src/Core/Lock/CacheLockDriver.php index 1bb768bd0f..13d912c1e2 100644 --- a/src/Core/Lock/CacheLockDriver.php +++ b/src/Core/Lock/CacheLockDriver.php @@ -2,7 +2,7 @@ namespace Friendica\Core\Lock; -use Friendica\Core\Cache\ICacheDriver; +use Friendica\Core\Cache\IMemoryCacheDriver; class CacheLockDriver extends AbstractLockDriver { @@ -14,9 +14,9 @@ class CacheLockDriver extends AbstractLockDriver /** * CacheLockDriver constructor. * - * @param ICacheDriver $cache The CacheDriver for this type of lock + * @param IMemoryCacheDriver $cache The CacheDriver for this type of lock */ - public function __construct(ICacheDriver $cache) + public function __construct(IMemoryCacheDriver $cache) { $this->cache = $cache; } @@ -35,32 +35,31 @@ class CacheLockDriver extends AbstractLockDriver $got_lock = false; $start = time(); - $cachekey = get_app()->get_hostname() . ";lock:" . $key; + $cachekey = self::getCacheKey($key); do { $lock = $this->cache->get($cachekey); + // When we do want to lock something that was already locked by us. + if ((int)$lock == getmypid()) { + $got_lock = true; + } - if (!is_bool($lock)) { - $pid = (int)$lock; - - // When the process id isn't used anymore, we can safely claim the lock for us. - // Or we do want to lock something that was already locked by us. - if (!posix_kill($pid, 0) || ($pid == getmypid())) { - $lock = false; + // When we do want to lock something new + if (is_null($lock)) { + // At first initialize it with "0" + $this->cache->add($cachekey, 0); + // Now the value has to be "0" because otherwise the key was used by another process meanwhile + if ($this->cache->compareSet($cachekey, 0, getmypid(), 300)) { + $got_lock = true; + $this->markAcquire($key); } } - if (is_bool($lock)) { - $this->cache->set($cachekey, getmypid(), 300); - $got_lock = true; - } if (!$got_lock && ($timeout > 0)) { usleep(rand(10000, 200000)); } } while (!$got_lock && ((time() - $start) < $timeout)); - $this->markAcquire($key); - return $got_lock; } @@ -68,22 +67,33 @@ class CacheLockDriver extends AbstractLockDriver * @brief Removes a lock if it was set by us * * @param string $key Name of the lock - * - * @return mixed */ public function releaseLock($key) { - $cachekey = get_app()->get_hostname() . ";lock:" . $key; - $lock = $this->cache->get($cachekey); - - if (!is_bool($lock)) { - if ((int)$lock == getmypid()) { - $this->cache->delete($cachekey); - } - } + $cachekey = self::getCacheKey($key); + $this->cache->compareDelete($cachekey, getmypid()); $this->markRelease($key); + } - return; + /** + * @brief Checks, if a key is currently locked to a process + * + * @param string $key The name of the lock + * @return bool + */ + public function isLocked($key) + { + $cachekey = self::getCacheKey($key); + $lock = $this->cache->get($cachekey); + return isset($lock) && ($lock !== false); + } + + /** + * @param string $key The original key + * @return string The cache key used for the cache + */ + private static function getCacheKey($key) { + return self::getApp()->get_hostname() . ";lock:" . $key; } } diff --git a/src/Core/Lock/DatabaseLockDriver.php b/src/Core/Lock/DatabaseLockDriver.php index 9b415753fc..6f4b942a4d 100644 --- a/src/Core/Lock/DatabaseLockDriver.php +++ b/src/Core/Lock/DatabaseLockDriver.php @@ -4,6 +4,7 @@ namespace Friendica\Core\Lock; use dba; use Friendica\Database\DBM; +use Friendica\Util\DateTimeFormat; /** * Locking driver that stores the locks in the database @@ -11,12 +12,7 @@ use Friendica\Database\DBM; class DatabaseLockDriver extends AbstractLockDriver { /** - * @brief Sets a lock for a given name - * - * @param string $key The Name of the lock - * @param integer $timeout Seconds until we give up - * - * @return boolean Was the lock successful? + * (@inheritdoc) */ public function acquireLock($key, $timeout = 120) { @@ -25,26 +21,25 @@ class DatabaseLockDriver extends AbstractLockDriver do { dba::lock('locks'); - $lock = dba::selectFirst('locks', ['locked', 'pid'], ['name' => $key]); + $lock = dba::selectFirst('locks', ['locked', 'pid'], ['`name` = ? AND `expires` >= ?', $key, DateTimeFormat::utcNow()]); if (DBM::is_result($lock)) { if ($lock['locked']) { - // When the process id isn't used anymore, we can safely claim the lock for us. - if (!posix_kill($lock['pid'], 0)) { - $lock['locked'] = false; - } // We want to lock something that was already locked by us? So we got the lock. if ($lock['pid'] == getmypid()) { $got_lock = true; + $this->markAcquire($key); } } if (!$lock['locked']) { - dba::update('locks', ['locked' => true, 'pid' => getmypid()], ['name' => $key]); + dba::update('locks', ['locked' => true, 'pid' => getmypid(), 'expires' => DateTimeFormat::utc('now + 300seconds')], ['name' => $key]); $got_lock = true; + $this->markAcquire($key); } } else { - dba::insert('locks', ['name' => $key, 'locked' => true, 'pid' => getmypid()]); + dba::insert('locks', ['name' => $key, 'locked' => true, 'pid' => getmypid(), 'expires' => DateTimeFormat::utc('now + 300seconds')]); $got_lock = true; + $this->markAcquire($key); } dba::unlock(); @@ -54,36 +49,42 @@ class DatabaseLockDriver extends AbstractLockDriver } } while (!$got_lock && ((time() - $start) < $timeout)); - $this->markAcquire($key); - return $got_lock; } /** - * @brief Removes a lock if it was set by us - * - * @param string $key Name of the lock - * - * @return mixed + * (@inheritdoc) */ public function releaseLock($key) { - dba::delete('locks', ['locked' => false, 'pid' => 0], ['name' => $key, 'pid' => getmypid()]); + dba::delete('locks', ['name' => $key, 'pid' => getmypid()]); - $this->releaseLock($key); + $this->markRelease($key); return; } /** - * @brief Removes all lock that were set by us - * - * @return void + * (@inheritdoc) */ public function releaseAll() { - dba::delete('locks', ['locked' => false, 'pid' => 0], ['pid' => getmypid()]); + dba::delete('locks', ['pid' => getmypid()]); $this->acquiredLocks = []; } + + /** + * (@inheritdoc) + */ + public function isLocked($key) + { + $lock = dba::selectFirst('locks', ['locked'], ['`name` = ? AND `expires` >= ?', $key, DateTimeFormat::utcNow()]); + + if (DBM::is_result($lock)) { + return $lock['locked'] !== false; + } else { + return false; + } + } } diff --git a/src/Core/Lock/ILockDriver.php b/src/Core/Lock/ILockDriver.php index 8744d757f1..3fbe049d37 100644 --- a/src/Core/Lock/ILockDriver.php +++ b/src/Core/Lock/ILockDriver.php @@ -9,6 +9,14 @@ namespace Friendica\Core\Lock; */ interface ILockDriver { + /** + * @brief Checks, if a key is currently locked to a or my process + * + * @param string $key The name of the lock + * @return bool + */ + public function isLocked($key); + /** * * @brief Acquires a lock for a given name diff --git a/src/Core/Lock/SemaphoreLockDriver.php b/src/Core/Lock/SemaphoreLockDriver.php index 39e3e1d32c..b4439743c8 100644 --- a/src/Core/Lock/SemaphoreLockDriver.php +++ b/src/Core/Lock/SemaphoreLockDriver.php @@ -4,6 +4,8 @@ namespace Friendica\Core\Lock; class SemaphoreLockDriver extends AbstractLockDriver { + private static $semaphore = []; + public function __construct() { if (!function_exists('sem_get')) { @@ -42,10 +44,15 @@ class SemaphoreLockDriver extends AbstractLockDriver */ public function acquireLock($key, $timeout = 120) { - $this->acquiredLocks[$key] = sem_get(self::semaphoreKey($key)); - if ($this->acquiredLocks[$key]) { - return sem_acquire($this->acquiredLocks[$key], ($timeout == 0)); + self::$semaphore[$key] = sem_get(self::semaphoreKey($key)); + if (self::$semaphore[$key]) { + if (sem_acquire(self::$semaphore[$key], ($timeout == 0))) { + $this->markAcquire($key); + return true; + } } + + return false; } /** @@ -57,12 +64,24 @@ class SemaphoreLockDriver extends AbstractLockDriver */ public function releaseLock($key) { - if (empty($this->acquiredLocks[$key])) { + if (empty(self::$semaphore[$key])) { return false; } else { - $success = @sem_release($this->acquiredLocks[$key]); - unset($this->acquiredLocks[$key]); + $success = @sem_release(self::$semaphore[$key]); + unset(self::$semaphore[$key]); + $this->markRelease($key); return $success; } } + + /** + * @brief Checks, if a key is currently locked to a process + * + * @param string $key The name of the lock + * @return bool + */ + public function isLocked($key) + { + return @sem_get(self::$semaphore[$key]) !== false; + } } diff --git a/src/Database/DBStructure.php b/src/Database/DBStructure.php index 8fb2afddb2..b14f31b11f 100644 --- a/src/Database/DBStructure.php +++ b/src/Database/DBStructure.php @@ -4,10 +4,9 @@ */ namespace Friendica\Database; +use dba; use Friendica\Core\Config; use Friendica\Core\L10n; -use Friendica\Database\DBM; -use dba; require_once 'boot.php'; require_once 'include/dba.php'; @@ -1285,9 +1284,11 @@ class DBStructure "name" => ["type" => "varchar(128)", "not null" => "1", "default" => "", "comment" => ""], "locked" => ["type" => "boolean", "not null" => "1", "default" => "0", "comment" => ""], "pid" => ["type" => "int unsigned", "not null" => "1", "default" => "0", "comment" => "Process ID"], - ], + "expires" => ["type" => "datetime", "not null" => "1", "default" => NULL_DATE, "comment" => "datetime of cache expiration"], + ], "indexes" => [ "PRIMARY" => ["id"], + "name_expires" => ["name", "expires"] ] ]; $database["mail"] = [ diff --git a/tests/datasets/api.yml b/tests/datasets/api.yml index 9ba5ec387e..14053c3e86 100644 --- a/tests/datasets/api.yml +++ b/tests/datasets/api.yml @@ -14,7 +14,7 @@ user: uid: 42 username: Test user nickname: selfcontact - verified: true + verified: 1 password: $2y$10$DLRNTRmJgKe1cSrFJ5Jb0edCqvXlA9sh/RHdSnfxjbR.04yZRm4Qm theme: frio @@ -24,12 +24,12 @@ contact: uid: 42 name: Self contact nick: selfcontact - self: true + self: 1 nurl: http://localhost/profile/selfcontact url: http://localhost/profile/selfcontact about: User used in tests - pending: false - blocked: false + pending: 0 + blocked: 0 rel: 1 network: dfrn - @@ -39,11 +39,11 @@ contact: # the fallback to api_get_nick() in api_get_user() name: othercontact nick: othercontact - self: false + self: 0 nurl: http://localhost/profile/othercontact url: http://localhost/profile/othercontact - pending: false - blocked: false + pending: 0 + blocked: 0 rel: 0 network: dfrn - @@ -51,150 +51,150 @@ contact: uid: 0 name: Friend contact nick: friendcontact - self: false + self: 0 nurl: http://localhost/profile/friendcontact url: http://localhost/profile/friendcontact - pending: false - blocked: false + pending: 0 + blocked: 0 rel: 2 network: dfrn item: - id: 1 - visible: true + visible: 1 contact-id: 42 author-id: 42 owner-id: 45 uid: 42 verb: http://activitystrea.ms/schema/1.0/post - unseen: true + unseen: 1 body: Parent status parent: 1 author-link: http://localhost/profile/selfcontact - wall: true - starred: true - origin: true + wall: 1 + starred: 1 + origin: 1 allow_cid: '' allow_gid: '' deny_cid: '' deny_gid: '' - id: 2 - visible: true + visible: 1 contact-id: 42 author-id: 42 owner-id: 45 uid: 42 verb: http://activitystrea.ms/schema/1.0/post - unseen: false + unseen: 0 body: Reply parent: 1 author-link: http://localhost/profile/selfcontact - wall: true - starred: false - origin: true + wall: 1 + starred: 0 + origin: 1 - id: 3 - visible: true + visible: 1 contact-id: 43 author-id: 43 owner-id: 42 uid: 42 verb: http://activitystrea.ms/schema/1.0/post - unseen: false + unseen: 0 body: Other user status parent: 3 author-link: http://localhost/profile/othercontact - wall: true - starred: false - origin: true + wall: 1 + starred: 0 + origin: 1 - id: 4 - visible: true + visible: 1 contact-id: 44 author-id: 44 owner-id: 42 uid: 42 verb: http://activitystrea.ms/schema/1.0/post - unseen: false + unseen: 0 body: Friend user reply parent: 1 author-link: http://localhost/profile/othercontact - wall: true - starred: false - origin: true + wall: 1 + starred: 0 + origin: 1 - id: 5 - visible: true + visible: 1 contact-id: 42 author-id: 42 owner-id: 42 uid: 42 verb: http://activitystrea.ms/schema/1.0/post - unseen: false + unseen: 0 body: '[share]Shared status[/share]' parent: 1 author-link: http://localhost/profile/othercontact - wall: true - starred: false - origin: true + wall: 1 + starred: 0 + origin: 1 allow_cid: '' allow_gid: '' deny_cid: '' deny_gid: '' - id: 6 - visible: true + visible: 1 contact-id: 44 author-id: 44 owner-id: 42 uid: 42 verb: http://activitystrea.ms/schema/1.0/post - unseen: false + unseen: 0 body: Friend user status parent: 6 author-link: http://localhost/profile/othercontact - wall: true - starred: false - origin: true + wall: 1 + starred: 0 + origin: 1 thread: - iid: 1 - visible: true + visible: 1 contact-id: 42 author-id: 42 owner-id: 42 uid: 42 - wall: true + wall: 1 - iid: 3 - visible: true + visible: 1 contact-id: 43 author-id: 43 owner-id: 43 uid: 0 - wall: true + wall: 1 - iid: 6 - visible: true + visible: 1 contact-id: 44 author-id: 44 owner-id: 44 uid: 0 - wall: true + wall: 1 group: - id: 1 uid: 42 - visible: true + visible: 1 name: Visible list - id: 2 uid: 42 - visible: false + visible: 0 name: Private list search: diff --git a/tests/src/Core/Lock/CacheLockDriverTest.php b/tests/src/Core/Lock/CacheLockDriverTest.php new file mode 100644 index 0000000000..a089059725 --- /dev/null +++ b/tests/src/Core/Lock/CacheLockDriverTest.php @@ -0,0 +1,27 @@ +cache = new ArrayCache(); + return new CacheLockDriver($this->cache); + } + + public function tearDown() + { + $this->cache->clear(); + parent::tearDown(); + } +} \ No newline at end of file diff --git a/tests/src/Core/Lock/DatabaseLockDriverTest.php b/tests/src/Core/Lock/DatabaseLockDriverTest.php new file mode 100644 index 0000000000..a80ff4c37c --- /dev/null +++ b/tests/src/Core/Lock/DatabaseLockDriverTest.php @@ -0,0 +1,67 @@ +module = 'install'; + + // Create database structure + DBStructure::update(false, true, true); + } else { + $this->markTestSkipped('Could not connect to the database.'); + } + } + + return $this->createDefaultDBConnection(dba::get_db(), getenv('DB')); + } + + /** + * Get dataset to populate the database with. + * @return YamlDataSet + * @see https://phpunit.de/manual/5.7/en/database.html + */ + protected function getDataSet() + { + return new YamlDataSet(__DIR__ . '/../../../datasets/api.yml'); + } + + protected function getInstance() + { + return new DatabaseLockDriver(); + } + + public function tearDown() + { + dba::delete('locks', [ 'id > 0']); + parent::tearDown(); + } +} \ No newline at end of file diff --git a/tests/src/Core/Lock/LockTest.php b/tests/src/Core/Lock/LockTest.php new file mode 100644 index 0000000000..ec7b97a9c7 --- /dev/null +++ b/tests/src/Core/Lock/LockTest.php @@ -0,0 +1,80 @@ +instance = $this->getInstance(); + + // Reusable App object + $this->app = new App(__DIR__.'/../'); + $a = $this->app; + + // Default config + Config::set('config', 'hostname', 'localhost'); + Config::set('system', 'throttle_limit_day', 100); + Config::set('system', 'throttle_limit_week', 100); + Config::set('system', 'throttle_limit_month', 100); + Config::set('system', 'theme', 'system_theme'); + } + + public function testLock() { + $this->instance->acquireLock('foo', 1); + $this->assertTrue($this->instance->isLocked('foo')); + $this->assertFalse($this->instance->isLocked('bar')); + } + + public function testDoubleLock() { + $this->instance->acquireLock('foo', 1); + $this->assertTrue($this->instance->isLocked('foo')); + // We already locked it + $this->assertTrue($this->instance->acquireLock('foo', 1)); + } + + public function testReleaseLock() { + $this->instance->acquireLock('foo', 1); + $this->assertTrue($this->instance->isLocked('foo')); + $this->instance->releaseLock('foo'); + $this->assertFalse($this->instance->isLocked('foo')); + } + + public function testReleaseAll() { + $this->instance->acquireLock('foo', 1); + $this->instance->acquireLock('bar', 1); + $this->instance->acquireLock('#/$%§', 1); + + $this->instance->releaseAll(); + + $this->assertFalse($this->instance->isLocked('foo')); + $this->assertFalse($this->instance->isLocked('bar')); + $this->assertFalse($this->instance->isLocked('#/$%§')); + } + + public function testReleaseAfterUnlock() { + $this->instance->acquireLock('foo', 1); + $this->instance->acquireLock('bar', 1); + $this->instance->acquireLock('#/$%§', 1); + + $this->instance->releaseLock('foo'); + + $this->instance->releaseAll(); + + $this->assertFalse($this->instance->isLocked('bar')); + $this->assertFalse($this->instance->isLocked('#/$%§')); + } +} \ No newline at end of file diff --git a/tests/src/Core/Lock/SemaphoreLockDriverTest.php b/tests/src/Core/Lock/SemaphoreLockDriverTest.php new file mode 100644 index 0000000000..fb7efd6584 --- /dev/null +++ b/tests/src/Core/Lock/SemaphoreLockDriverTest.php @@ -0,0 +1,14 @@ +