From 0218d16335cd4b873c3edd59fbcc110306d87e71 Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Tue, 26 Jun 2018 22:31:04 +0200 Subject: [PATCH 01/12] Lock abstraction (like the Cache) - adding interface - adding seperate drivers - moving Lock to the Core package --- src/Core/Lock.php | 97 ++++++++++++ src/Core/Lock/DatabaseLockDriver.php | 83 ++++++++++ src/Core/Lock/ILockDriver.php | 38 +++++ src/Core/Lock/MemcacheLockDriver.php | 86 +++++++++++ src/Core/Lock/SemaphoreLockDriver.php | 82 ++++++++++ src/Core/Worker.php | 17 +-- src/Protocol/OStatus.php | 5 +- src/Util/Lock.php | 211 -------------------------- 8 files changed, 396 insertions(+), 223 deletions(-) create mode 100644 src/Core/Lock.php create mode 100644 src/Core/Lock/DatabaseLockDriver.php create mode 100644 src/Core/Lock/ILockDriver.php create mode 100644 src/Core/Lock/MemcacheLockDriver.php create mode 100644 src/Core/Lock/SemaphoreLockDriver.php delete mode 100644 src/Util/Lock.php diff --git a/src/Core/Lock.php b/src/Core/Lock.php new file mode 100644 index 0000000000..9e02d14fcb --- /dev/null +++ b/src/Core/Lock.php @@ -0,0 +1,97 @@ +=')) { + self::$driver = new Lock\SemaphoreLockDriver(); + } elseif (Config::get('system', 'cache_driver', 'database') == 'memcache') { + self::$driver = new Lock\MemcacheLockDriver(); + } else { + self::$driver = new Lock\DatabaseLockDriver(); + } + } + } + + /** + * Returns the current cache driver + * + * @return Lock\ILockDriver; + */ + private static function getDriver() + { + if (self::$driver === null) { + self::init(); + } + + return self::$driver; + } + + /** + * @brief Acquires a lock for a given name + * + * @param string $key Name of the lock + * @param integer $timeout Seconds until we give up + * + * @return boolean Was the lock successful? + */ + public static function acquireLock($key, $timeout = 120) + { + return self::getDriver()->acquireLock($key, $timeout); + } + + /** + * @brief Releases a lock if it was set by us + * + * @param string $key Name of the lock + * @return mixed + */ + public static function releaseLock($key) + { + return self::getDriver()->releaseLock($key); + } + + /** + * @brief Releases all lock that were set by us + * @return void + */ + public static function releaseAll() + { + self::getDriver()->releaseAll(); + } +} diff --git a/src/Core/Lock/DatabaseLockDriver.php b/src/Core/Lock/DatabaseLockDriver.php new file mode 100644 index 0000000000..8761a1479e --- /dev/null +++ b/src/Core/Lock/DatabaseLockDriver.php @@ -0,0 +1,83 @@ + $key]); + + 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; + } + } + if (!$lock['locked']) { + dba::update('locks', ['locked' => true, 'pid' => getmypid()], ['name' => $key]); + $got_lock = true; + } + } elseif (!DBM::is_result($lock)) { + dba::insert('locks', ['name' => $key, 'locked' => true, 'pid' => getmypid()]); + $got_lock = true; + } + + dba::unlock(); + + if (!$got_lock && ($timeout > 0)) { + usleep(rand(100000, 2000000)); + } + } while (!$got_lock && ((time() - $start) < $timeout)); + + return $got_lock; + } + + /** + * @brief Removes a lock if it was set by us + * + * @param string $key Name of the lock + * + * @return mixed + */ + public function releaseLock($key) + { + dba::update('locks', ['locked' => false, 'pid' => 0], ['name' => $key, 'pid' => getmypid()]); + + return; + } + + /** + * @brief Removes all lock that were set by us + * + * @return void + */ + public function releaseAll() + { + dba::update('locks', ['locked' => false, 'pid' => 0], ['pid' => getmypid()]); + } +} diff --git a/src/Core/Lock/ILockDriver.php b/src/Core/Lock/ILockDriver.php new file mode 100644 index 0000000000..b066361be6 --- /dev/null +++ b/src/Core/Lock/ILockDriver.php @@ -0,0 +1,38 @@ + + */ +interface ILockDriver +{ + /** + * + * @brief Acquires 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? + */ + public function acquireLock($key, $timeout = 120); + + /** + * @brief Releases a lock if it was set by us + * + * @param string $key Name of the lock + * + * @return mixed + */ + public function releaseLock($key); + + /** + * @brief Releases all lock that were set by us + * + * @return void + */ + public function releaseAll(); +} \ No newline at end of file diff --git a/src/Core/Lock/MemcacheLockDriver.php b/src/Core/Lock/MemcacheLockDriver.php new file mode 100644 index 0000000000..1796d81366 --- /dev/null +++ b/src/Core/Lock/MemcacheLockDriver.php @@ -0,0 +1,86 @@ +get_hostname() . ";lock:" . $key; + + do { + // We only lock to be sure that nothing happens at exactly the same time + dba::lock('locks'); + $lock = Cache::get($cachekey); + + 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; + } + } + if (is_bool($lock)) { + Cache::set($cachekey, getmypid(), 300); + $got_lock = true; + } + + dba::unlock(); + + if (!$got_lock && ($timeout > 0)) { + usleep(rand(10000, 200000)); + } + } while (!$got_lock && ((time() - $start) < $timeout)); + + return $got_lock; + } + + /** + * @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 = Cache::get($cachekey); + + if (!is_bool($lock)) { + if ((int)$lock == getmypid()) { + Cache::delete($cachekey); + } + } + + return; + } + + /** + * @brief Removes all lock that were set by us + * + * @return void + */ + public function releaseAll() + { + // We cannot delete all cache entries, but this doesn't matter with memcache + return; + } +} \ No newline at end of file diff --git a/src/Core/Lock/SemaphoreLockDriver.php b/src/Core/Lock/SemaphoreLockDriver.php new file mode 100644 index 0000000000..93911a3d10 --- /dev/null +++ b/src/Core/Lock/SemaphoreLockDriver.php @@ -0,0 +1,82 @@ +=')) { + self::$semaphore[$key] = sem_get(self::semaphoreKey($key)); + if (self::$semaphore[$key]) { + return sem_acquire(self::$semaphore[$key], ($timeout == 0)); + } + } + } + + /** + * @brief Removes a lock if it was set by us + * + * @param string $key Name of the lock + * + * @return mixed + */ + public function releaseLock($key) + { + if (function_exists('sem_get') && version_compare(PHP_VERSION, '5.6.1', '>=')) { + if (empty(self::$semaphore[$key])) { + return false; + } else { + $success = @sem_release(self::$semaphore[$key]); + unset(self::$semaphore[$key]); + return $success; + } + } + } + + /** + * @brief Removes all lock that were set by us + * + * @return void + */ + public function releaseAll() + { + // not needed/supported + return; + } +} \ No newline at end of file diff --git a/src/Core/Worker.php b/src/Core/Worker.php index c965e05830..b8a9021d0f 100644 --- a/src/Core/Worker.php +++ b/src/Core/Worker.php @@ -10,7 +10,6 @@ use Friendica\Core\System; use Friendica\Database\DBM; use Friendica\Model\Process; use Friendica\Util\DateTimeFormat; -use Friendica\Util\Lock; use Friendica\Util\Network; use dba; @@ -108,16 +107,16 @@ class Worker } // If possible we will fetch new jobs for this worker - if (!$refetched && Lock::set('worker_process', 0)) { + if (!$refetched && Lock::acquireLock('worker_process', 0)) { $stamp = (float)microtime(true); $refetched = self::findWorkerProcesses($passing_slow); self::$db_duration += (microtime(true) - $stamp); - Lock::remove('worker_process'); + Lock::releaseLock('worker_process'); } } // To avoid the quitting of multiple workers only one worker at a time will execute the check - if (Lock::set('worker', 0)) { + if (Lock::acquireLock('worker', 0)) { $stamp = (float)microtime(true); // Count active workers and compare them with a maximum value that depends on the load if (self::tooMuchWorkers()) { @@ -130,7 +129,7 @@ class Worker logger('Memory limit reached, quitting.', LOGGER_DEBUG); return; } - Lock::remove('worker'); + Lock::releaseLock('worker'); self::$db_duration += (microtime(true) - $stamp); } @@ -883,7 +882,7 @@ class Worker dba::close($r); $stamp = (float)microtime(true); - if (!Lock::set('worker_process')) { + if (!Lock::acquireLock('worker_process')) { return false; } self::$lock_duration = (microtime(true) - $stamp); @@ -892,7 +891,7 @@ class Worker $found = self::findWorkerProcesses($passing_slow); self::$db_duration += (microtime(true) - $stamp); - Lock::remove('worker_process'); + Lock::releaseLock('worker_process'); if ($found) { $r = dba::select('workerqueue', [], ['pid' => getmypid(), 'done' => false]); @@ -1097,13 +1096,13 @@ class Worker } // If there is a lock then we don't have to check for too much worker - if (!Lock::set('worker', 0)) { + if (!Lock::acquireLock('worker', 0)) { return true; } // If there are already enough workers running, don't fork another one $quit = self::tooMuchWorkers(); - Lock::remove('worker'); + Lock::releaseLock('worker'); if ($quit) { return true; diff --git a/src/Protocol/OStatus.php b/src/Protocol/OStatus.php index 2c826221e7..69975c5753 100644 --- a/src/Protocol/OStatus.php +++ b/src/Protocol/OStatus.php @@ -19,7 +19,6 @@ use Friendica\Model\User; use Friendica\Network\Probe; use Friendica\Object\Image; use Friendica\Util\DateTimeFormat; -use Friendica\Util\Lock; use Friendica\Util\Network; use Friendica\Util\XML; use dba; @@ -513,9 +512,9 @@ class OStatus logger("Item with uri ".$item["uri"]." is from a blocked contact.", LOGGER_DEBUG); } else { // We are having duplicated entries. Hopefully this solves it. - if (Lock::set('ostatus_process_item_insert')) { + if (Lock::acquireLock('ostatus_process_item_insert')) { $ret = Item::insert($item); - Lock::remove('ostatus_process_item_insert'); + Lock::releaseLock('ostatus_process_item_insert'); logger("Item with uri ".$item["uri"]." for user ".$importer["uid"].' stored. Return value: '.$ret); } else { $ret = Item::insert($item); diff --git a/src/Util/Lock.php b/src/Util/Lock.php deleted file mode 100644 index eba264ad93..0000000000 --- a/src/Util/Lock.php +++ /dev/null @@ -1,211 +0,0 @@ -connect($memcache_host, $memcache_port)) { - return false; - } - - return $memcache; - } - - /** - * @brief Creates a semaphore key - * - * @param string $fn_name Name of the lock - * - * @return ressource the semaphore key - */ - private static function semaphoreKey($fn_name) - { - $temp = get_temppath(); - - $file = $temp.'/'.$fn_name.'.sem'; - - if (!file_exists($file)) { - file_put_contents($file, $fn_name); - } - - return ftok($file, 'f'); - } - - /** - * @brief Sets a lock for a given name - * - * @param string $fn_name Name of the lock - * @param integer $timeout Seconds until we give up - * - * @return boolean Was the lock successful? - */ - public static function set($fn_name, $timeout = 120) - { - $got_lock = false; - $start = time(); - - // The second parameter for "sem_acquire" doesn't exist before 5.6.1 - if (function_exists('sem_get') && version_compare(PHP_VERSION, '5.6.1', '>=')) { - self::$semaphore[$fn_name] = sem_get(self::semaphoreKey($fn_name)); - if (self::$semaphore[$fn_name]) { - return sem_acquire(self::$semaphore[$fn_name], ($timeout == 0)); - } - } - - $memcache = self::connectMemcache(); - if (is_object($memcache)) { - $cachekey = get_app()->get_hostname().";lock:".$fn_name; - - do { - // We only lock to be sure that nothing happens at exactly the same time - dba::lock('locks'); - $lock = $memcache->get($cachekey); - - 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; - } - } - if (is_bool($lock)) { - $memcache->set($cachekey, getmypid(), MEMCACHE_COMPRESSED, 300); - $got_lock = true; - } - - dba::unlock(); - - if (!$got_lock && ($timeout > 0)) { - usleep(rand(10000, 200000)); - } - } while (!$got_lock && ((time() - $start) < $timeout)); - - return $got_lock; - } - - do { - dba::lock('locks'); - $lock = dba::selectFirst('locks', ['locked', 'pid'], ['name' => $fn_name]); - - 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; - } - } - if (!$lock['locked']) { - dba::update('locks', ['locked' => true, 'pid' => getmypid()], ['name' => $fn_name]); - $got_lock = true; - } - } elseif (!DBM::is_result($lock)) { - dba::insert('locks', ['name' => $fn_name, 'locked' => true, 'pid' => getmypid()]); - $got_lock = true; - } - - dba::unlock(); - - if (!$got_lock && ($timeout > 0)) { - usleep(rand(100000, 2000000)); - } - } while (!$got_lock && ((time() - $start) < $timeout)); - - return $got_lock; - } - - /** - * @brief Removes a lock if it was set by us - * - * @param string $fn_name Name of the lock - * @return mixed - */ - public static function remove($fn_name) - { - if (function_exists('sem_get') && version_compare(PHP_VERSION, '5.6.1', '>=')) { - if (empty(self::$semaphore[$fn_name])) { - return false; - } else { - $success = @sem_release(self::$semaphore[$fn_name]); - unset(self::$semaphore[$fn_name]); - return $success; - } - } - - $memcache = self::connectMemcache(); - if (is_object($memcache)) { - $cachekey = get_app()->get_hostname().";lock:".$fn_name; - $lock = $memcache->get($cachekey); - - if (!is_bool($lock)) { - if ((int)$lock == getmypid()) { - $memcache->delete($cachekey); - } - } - return; - } - - dba::update('locks', ['locked' => false, 'pid' => 0], ['name' => $fn_name, 'pid' => getmypid()]); - return; - } - - /** - * @brief Removes all lock that were set by us - * @return void - */ - public static function removeAll() - { - $memcache = self::connectMemcache(); - if (is_object($memcache)) { - // We cannot delete all cache entries, but this doesn't matter with memcache - return; - } - - dba::update('locks', ['locked' => false, 'pid' => 0], ['pid' => getmypid()]); - return; - } -} From a57e6cfa1b6c1e7c75698b2e804b917edff671e9 Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Tue, 26 Jun 2018 23:28:07 +0200 Subject: [PATCH 02/12] Moved Lock.php back to Utils --- src/Core/Worker.php | 1 + src/Protocol/OStatus.php | 1 + src/{Core => Util}/Lock.php | 7 ++++++- src/{Core => Util}/Lock/DatabaseLockDriver.php | 2 +- src/{Core => Util}/Lock/ILockDriver.php | 5 ++++- src/{Core => Util}/Lock/MemcacheLockDriver.php | 2 +- src/{Core => Util}/Lock/SemaphoreLockDriver.php | 2 +- 7 files changed, 15 insertions(+), 5 deletions(-) rename src/{Core => Util}/Lock.php (96%) rename src/{Core => Util}/Lock/DatabaseLockDriver.php (98%) rename src/{Core => Util}/Lock/ILockDriver.php (91%) rename src/{Core => Util}/Lock/MemcacheLockDriver.php (98%) rename src/{Core => Util}/Lock/SemaphoreLockDriver.php (98%) diff --git a/src/Core/Worker.php b/src/Core/Worker.php index b8a9021d0f..0e8835a156 100644 --- a/src/Core/Worker.php +++ b/src/Core/Worker.php @@ -11,6 +11,7 @@ use Friendica\Database\DBM; use Friendica\Model\Process; use Friendica\Util\DateTimeFormat; use Friendica\Util\Network; +use Friendica\Util\Lock; use dba; require_once 'include/dba.php'; diff --git a/src/Protocol/OStatus.php b/src/Protocol/OStatus.php index 69975c5753..a1223b7b68 100644 --- a/src/Protocol/OStatus.php +++ b/src/Protocol/OStatus.php @@ -19,6 +19,7 @@ use Friendica\Model\User; use Friendica\Network\Probe; use Friendica\Object\Image; use Friendica\Util\DateTimeFormat; +use Friendica\Util\Lock; use Friendica\Util\Network; use Friendica\Util\XML; use dba; diff --git a/src/Core/Lock.php b/src/Util/Lock.php similarity index 96% rename from src/Core/Lock.php rename to src/Util/Lock.php index 9e02d14fcb..95341b4b66 100644 --- a/src/Core/Lock.php +++ b/src/Util/Lock.php @@ -10,7 +10,7 @@ namespace Friendica\Util; */ use Friendica\Core\Config; -use Friendica\Core\Lock; +use Friendica\Util\Lock; require_once 'include/dba.php'; @@ -94,4 +94,9 @@ class Lock { self::getDriver()->releaseAll(); } + + public static function isLocked($key) + { + + } } diff --git a/src/Core/Lock/DatabaseLockDriver.php b/src/Util/Lock/DatabaseLockDriver.php similarity index 98% rename from src/Core/Lock/DatabaseLockDriver.php rename to src/Util/Lock/DatabaseLockDriver.php index 8761a1479e..b2e8f50279 100644 --- a/src/Core/Lock/DatabaseLockDriver.php +++ b/src/Util/Lock/DatabaseLockDriver.php @@ -1,6 +1,6 @@ Date: Tue, 26 Jun 2018 23:33:02 +0200 Subject: [PATCH 03/12] Bugfixing ILockDriver (forgot isLocked) --- src/Util/Lock/ILockDriver.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Util/Lock/ILockDriver.php b/src/Util/Lock/ILockDriver.php index 6b8a08cb04..97052b35af 100644 --- a/src/Util/Lock/ILockDriver.php +++ b/src/Util/Lock/ILockDriver.php @@ -35,7 +35,4 @@ interface ILockDriver * @return void */ public function releaseAll(); - - - public function isLocked(); } \ No newline at end of file From dd085ae59226d4d25fa7b2ab0ec18edaab54d286 Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Tue, 26 Jun 2018 23:42:26 +0200 Subject: [PATCH 04/12] minor changes --- src/Core/Worker.php | 2 +- src/Util/Lock.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Core/Worker.php b/src/Core/Worker.php index 0e8835a156..ef7873fcd4 100644 --- a/src/Core/Worker.php +++ b/src/Core/Worker.php @@ -10,8 +10,8 @@ use Friendica\Core\System; use Friendica\Database\DBM; use Friendica\Model\Process; use Friendica\Util\DateTimeFormat; -use Friendica\Util\Network; use Friendica\Util\Lock; +use Friendica\Util\Network; use dba; require_once 'include/dba.php'; diff --git a/src/Util/Lock.php b/src/Util/Lock.php index 95341b4b66..76e29826cb 100644 --- a/src/Util/Lock.php +++ b/src/Util/Lock.php @@ -1,11 +1,11 @@ Date: Tue, 26 Jun 2018 23:43:43 +0200 Subject: [PATCH 05/12] minor changes --- src/Util/Lock.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/Util/Lock.php b/src/Util/Lock.php index 76e29826cb..b6a4a97f18 100644 --- a/src/Util/Lock.php +++ b/src/Util/Lock.php @@ -94,9 +94,4 @@ class Lock { self::getDriver()->releaseAll(); } - - public static function isLocked($key) - { - - } } From acf6a5cb9ede84dabedda3e0925776c83efd9337 Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Tue, 26 Jun 2018 23:44:30 +0200 Subject: [PATCH 06/12] minor changes --- src/Util/Lock/ILockDriver.php | 2 +- src/Util/Lock/MemcacheLockDriver.php | 2 +- src/Util/Lock/SemaphoreLockDriver.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Util/Lock/ILockDriver.php b/src/Util/Lock/ILockDriver.php index 97052b35af..7f5a359946 100644 --- a/src/Util/Lock/ILockDriver.php +++ b/src/Util/Lock/ILockDriver.php @@ -35,4 +35,4 @@ interface ILockDriver * @return void */ public function releaseAll(); -} \ No newline at end of file +} diff --git a/src/Util/Lock/MemcacheLockDriver.php b/src/Util/Lock/MemcacheLockDriver.php index 9a3ceb4645..d0789a95c4 100644 --- a/src/Util/Lock/MemcacheLockDriver.php +++ b/src/Util/Lock/MemcacheLockDriver.php @@ -83,4 +83,4 @@ class MemcacheLockDriver implements ILockDriver // We cannot delete all cache entries, but this doesn't matter with memcache return; } -} \ No newline at end of file +} diff --git a/src/Util/Lock/SemaphoreLockDriver.php b/src/Util/Lock/SemaphoreLockDriver.php index 3adf1ff37c..1cd3af1411 100644 --- a/src/Util/Lock/SemaphoreLockDriver.php +++ b/src/Util/Lock/SemaphoreLockDriver.php @@ -79,4 +79,4 @@ class SemaphoreLockDriver implements ILockDriver // not needed/supported return; } -} \ No newline at end of file +} From 3f7e4f5bb67c67586423d9e1105ce274b83767c3 Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Thu, 28 Jun 2018 22:57:17 +0200 Subject: [PATCH 07/12] redesign of locking & caching - New Factory "CacheDriverFactory" for Cache and Locks - Adding Redis/Memcached Locking - Moved Lock to Core - other improvements --- src/Core/Cache.php | 27 +--- src/Core/Cache/CacheDriverFactory.php | 48 ++++++ src/Core/Lock.php | 145 ++++++++++++++++++ src/Core/Lock/AbstractLockDriver.php | 57 +++++++ .../Lock/CacheLockDriver.php} | 51 +++--- .../Lock/DatabaseLockDriver.php | 20 ++- src/{Util => Core}/Lock/ILockDriver.php | 10 +- src/Core/Lock/SemaphoreLockDriver.php | 68 ++++++++ src/Core/Worker.php | 2 +- src/Protocol/OStatus.php | 2 +- src/Util/Lock.php | 97 ------------ src/Util/Lock/SemaphoreLockDriver.php | 82 ---------- 12 files changed, 369 insertions(+), 240 deletions(-) create mode 100644 src/Core/Cache/CacheDriverFactory.php create mode 100644 src/Core/Lock.php create mode 100644 src/Core/Lock/AbstractLockDriver.php rename src/{Util/Lock/MemcacheLockDriver.php => Core/Lock/CacheLockDriver.php} (61%) rename src/{Util => Core}/Lock/DatabaseLockDriver.php (79%) rename src/{Util => Core}/Lock/ILockDriver.php (72%) create mode 100644 src/Core/Lock/SemaphoreLockDriver.php delete mode 100644 src/Util/Lock.php delete mode 100644 src/Util/Lock/SemaphoreLockDriver.php diff --git a/src/Core/Cache.php b/src/Core/Cache.php index 4202db325c..cc77d9bfd0 100644 --- a/src/Core/Cache.php +++ b/src/Core/Cache.php @@ -4,8 +4,7 @@ */ namespace Friendica\Core; -use Friendica\Core\Cache; -use Friendica\Core\Config; +use Friendica\Core\Cache\CacheDriverFactory; /** * @brief Class for storing data for a short time @@ -24,31 +23,13 @@ class Cache extends \Friendica\BaseObject /** * @var Cache\ICacheDriver */ - static $driver = null; + private static $driver = null; public static function init() { - switch(Config::get('system', 'cache_driver', 'database')) { - case 'memcache': - $memcache_host = Config::get('system', 'memcache_host', '127.0.0.1'); - $memcache_port = Config::get('system', 'memcache_port', 11211); + $driver_name = Config::get('system', 'cache_driver', 'database'); - self::$driver = new Cache\MemcacheCacheDriver($memcache_host, $memcache_port); - break; - case 'memcached': - $memcached_hosts = Config::get('system', 'memcached_hosts', [['127.0.0.1', 11211]]); - - self::$driver = new Cache\MemcachedCacheDriver($memcached_hosts); - break; - case 'redis': - $redis_host = Config::get('system', 'redis_host', '127.0.0.1'); - $redis_port = Config::get('system', 'redis_port', 6379); - - self::$driver = new Cache\RedisCacheDriver($redis_host, $redis_port); - break; - default: - self::$driver = new Cache\DatabaseCacheDriver(); - } + self::$driver = CacheDriverFactory::create($driver_name); } /** diff --git a/src/Core/Cache/CacheDriverFactory.php b/src/Core/Cache/CacheDriverFactory.php new file mode 100644 index 0000000000..d15b8e9858 --- /dev/null +++ b/src/Core/Cache/CacheDriverFactory.php @@ -0,0 +1,48 @@ +getMessage()); + } + } + + // 2. Try to use Cache Locking (don't use the DB-Cache Locking because it works different!) + $cache_driver = Config::get('system', 'cache_driver', 'database'); + if ($cache_driver != 'database') { + try { + $lock_driver = CacheDriverFactory::create($cache_driver); + self::$driver = new Lock\CacheLockDriver($lock_driver); + return; + } catch (\Exception $exception) { + logger('Using Cache driver for locking failed: ' . $exception->getMessage()); + } + } + + // 3. Use Database Locking as a Fallback + self::$driver = new Lock\DatabaseLockDriver(); + } + + /** + * Returns the current cache driver + * + * @return Lock\ILockDriver; + */ + private static function getDriver() + { + if (self::$driver === null) { + self::init(); + } + + return self::$driver; + } + + /** + * @brief Acquires a lock for a given name + * + * @param string $key Name of the lock + * @param integer $timeout Seconds until we give up + * + * @return boolean Was the lock successful? + */ + public static function acquireLock($key, $timeout = 120) + { + return self::getDriver()->acquireLock($key, $timeout); + } + + /** + * @brief Releases a lock if it was set by us + * + * @param string $key Name of the lock + * @return mixed + */ + public static function releaseLock($key) + { + return self::getDriver()->releaseLock($key); + } + + /** + * @brief Releases all lock that were set by us + * @return void + */ + public static function releaseAll() + { + self::getDriver()->releaseAll(); + } +} diff --git a/src/Core/Lock/AbstractLockDriver.php b/src/Core/Lock/AbstractLockDriver.php new file mode 100644 index 0000000000..15820c3782 --- /dev/null +++ b/src/Core/Lock/AbstractLockDriver.php @@ -0,0 +1,57 @@ +acquireLock[$key]); + } + + /** + * @brief Mark a locally acquired lock + * + * @param string $key The Name of the lock + */ + protected function markAcquire(string $key) { + $this->acquiredLocks[$key] = true; + } + + /** + * @brief Mark a release of a locally acquired lock + * + * @param string $key The Name of the lock + */ + protected function markRelease(string $key) { + unset($this->acquiredLocks[$key]); + } + + /** + * @brief Releases all lock that were set by us + * + * @return void + */ + public function releaseAll() { + foreach ($this->acquiredLocks as $acquiredLock) { + $this->releaseLock($acquiredLock); + } + } +} diff --git a/src/Util/Lock/MemcacheLockDriver.php b/src/Core/Lock/CacheLockDriver.php similarity index 61% rename from src/Util/Lock/MemcacheLockDriver.php rename to src/Core/Lock/CacheLockDriver.php index d0789a95c4..0adca140d1 100644 --- a/src/Util/Lock/MemcacheLockDriver.php +++ b/src/Core/Lock/CacheLockDriver.php @@ -1,12 +1,26 @@ cache = $cache; + } + /** * * @brief Sets a lock for a given name @@ -16,7 +30,7 @@ class MemcacheLockDriver implements ILockDriver * * @return boolean Was the lock successful? */ - public function acquireLock($key, $timeout = 120) + public function acquireLock(string $key, int $timeout = 120) { $got_lock = false; $start = time(); @@ -24,9 +38,7 @@ class MemcacheLockDriver implements ILockDriver $cachekey = get_app()->get_hostname() . ";lock:" . $key; do { - // We only lock to be sure that nothing happens at exactly the same time - dba::lock('locks'); - $lock = Cache::get($cachekey); + $lock = $this->cache->get($cachekey); if (!is_bool($lock)) { $pid = (int)$lock; @@ -38,17 +50,17 @@ class MemcacheLockDriver implements ILockDriver } } if (is_bool($lock)) { - Cache::set($cachekey, getmypid(), 300); + $this->cache->set($cachekey, getmypid(), 300); $got_lock = true; } - dba::unlock(); - if (!$got_lock && ($timeout > 0)) { usleep(rand(10000, 200000)); } } while (!$got_lock && ((time() - $start) < $timeout)); + $this->markAcquire($key); + return $got_lock; } @@ -59,28 +71,19 @@ class MemcacheLockDriver implements ILockDriver * * @return mixed */ - public function releaseLock($key) + public function releaseLock(string $key) { $cachekey = get_app()->get_hostname() . ";lock:" . $key; - $lock = Cache::get($cachekey); + $lock = $this->cache->get($cachekey); if (!is_bool($lock)) { if ((int)$lock == getmypid()) { - Cache::delete($cachekey); + $this->cache->delete($cachekey); } } - return; - } + $this->markRelease($key); - /** - * @brief Removes all lock that were set by us - * - * @return void - */ - public function releaseAll() - { - // We cannot delete all cache entries, but this doesn't matter with memcache return; } } diff --git a/src/Util/Lock/DatabaseLockDriver.php b/src/Core/Lock/DatabaseLockDriver.php similarity index 79% rename from src/Util/Lock/DatabaseLockDriver.php rename to src/Core/Lock/DatabaseLockDriver.php index b2e8f50279..f9878340cf 100644 --- a/src/Util/Lock/DatabaseLockDriver.php +++ b/src/Core/Lock/DatabaseLockDriver.php @@ -1,6 +1,6 @@ true, 'pid' => getmypid()], ['name' => $key]); $got_lock = true; } - } elseif (!DBM::is_result($lock)) { + } else { dba::insert('locks', ['name' => $key, 'locked' => true, 'pid' => getmypid()]); $got_lock = true; } @@ -54,6 +54,8 @@ class DatabaseLockDriver implements ILockDriver } } while (!$got_lock && ((time() - $start) < $timeout)); + $this->markAcquire($key); + return $got_lock; } @@ -64,9 +66,11 @@ class DatabaseLockDriver implements ILockDriver * * @return mixed */ - public function releaseLock($key) + public function releaseLock(string $key) { - dba::update('locks', ['locked' => false, 'pid' => 0], ['name' => $key, 'pid' => getmypid()]); + dba::delete('locks', ['locked' => false, 'pid' => 0], ['name' => $key, 'pid' => getmypid()]); + + $this->releaseLock($key); return; } @@ -78,6 +82,8 @@ class DatabaseLockDriver implements ILockDriver */ public function releaseAll() { - dba::update('locks', ['locked' => false, 'pid' => 0], ['pid' => getmypid()]); + dba::delete('locks', ['locked' => false, 'pid' => 0], ['pid' => getmypid()]); + + $this->acquiredLocks = []; } } diff --git a/src/Util/Lock/ILockDriver.php b/src/Core/Lock/ILockDriver.php similarity index 72% rename from src/Util/Lock/ILockDriver.php rename to src/Core/Lock/ILockDriver.php index 7f5a359946..39e4ba8e89 100644 --- a/src/Util/Lock/ILockDriver.php +++ b/src/Core/Lock/ILockDriver.php @@ -1,6 +1,6 @@ acquiredLocks[$key] = sem_get(self::semaphoreKey($key)); + if ($this->acquiredLocks[$key]) { + return sem_acquire($this->acquiredLocks[$key], ($timeout == 0)); + } + } + + /** + * @brief Removes a lock if it was set by us + * + * @param string $key Name of the lock + * + * @return mixed + */ + public function releaseLock(string $key) + { + if (empty($this->acquiredLocks[$key])) { + return false; + } else { + $success = @sem_release($this->acquiredLocks[$key]); + unset($this->acquiredLocks[$key]); + return $success; + } + } +} diff --git a/src/Core/Worker.php b/src/Core/Worker.php index ef7873fcd4..cbf2ae8bd9 100644 --- a/src/Core/Worker.php +++ b/src/Core/Worker.php @@ -7,10 +7,10 @@ namespace Friendica\Core; use Friendica\Core\Addon; use Friendica\Core\Config; use Friendica\Core\System; +use Friendica\Core\Lock; use Friendica\Database\DBM; use Friendica\Model\Process; use Friendica\Util\DateTimeFormat; -use Friendica\Util\Lock; use Friendica\Util\Network; use dba; diff --git a/src/Protocol/OStatus.php b/src/Protocol/OStatus.php index a1223b7b68..288cbfed1b 100644 --- a/src/Protocol/OStatus.php +++ b/src/Protocol/OStatus.php @@ -9,6 +9,7 @@ use Friendica\Content\Text\HTML; use Friendica\Core\Cache; use Friendica\Core\Config; use Friendica\Core\L10n; +use Friendica\Core\Lock; use Friendica\Core\System; use Friendica\Database\DBM; use Friendica\Model\Contact; @@ -19,7 +20,6 @@ use Friendica\Model\User; use Friendica\Network\Probe; use Friendica\Object\Image; use Friendica\Util\DateTimeFormat; -use Friendica\Util\Lock; use Friendica\Util\Network; use Friendica\Util\XML; use dba; diff --git a/src/Util/Lock.php b/src/Util/Lock.php deleted file mode 100644 index b6a4a97f18..0000000000 --- a/src/Util/Lock.php +++ /dev/null @@ -1,97 +0,0 @@ -=')) { - self::$driver = new Lock\SemaphoreLockDriver(); - } elseif (Config::get('system', 'cache_driver', 'database') == 'memcache') { - self::$driver = new Lock\MemcacheLockDriver(); - } else { - self::$driver = new Lock\DatabaseLockDriver(); - } - } - } - - /** - * Returns the current cache driver - * - * @return Lock\ILockDriver; - */ - private static function getDriver() - { - if (self::$driver === null) { - self::init(); - } - - return self::$driver; - } - - /** - * @brief Acquires a lock for a given name - * - * @param string $key Name of the lock - * @param integer $timeout Seconds until we give up - * - * @return boolean Was the lock successful? - */ - public static function acquireLock($key, $timeout = 120) - { - return self::getDriver()->acquireLock($key, $timeout); - } - - /** - * @brief Releases a lock if it was set by us - * - * @param string $key Name of the lock - * @return mixed - */ - public static function releaseLock($key) - { - return self::getDriver()->releaseLock($key); - } - - /** - * @brief Releases all lock that were set by us - * @return void - */ - public static function releaseAll() - { - self::getDriver()->releaseAll(); - } -} diff --git a/src/Util/Lock/SemaphoreLockDriver.php b/src/Util/Lock/SemaphoreLockDriver.php deleted file mode 100644 index 1cd3af1411..0000000000 --- a/src/Util/Lock/SemaphoreLockDriver.php +++ /dev/null @@ -1,82 +0,0 @@ -=')) { - self::$semaphore[$key] = sem_get(self::semaphoreKey($key)); - if (self::$semaphore[$key]) { - return sem_acquire(self::$semaphore[$key], ($timeout == 0)); - } - } - } - - /** - * @brief Removes a lock if it was set by us - * - * @param string $key Name of the lock - * - * @return mixed - */ - public function releaseLock($key) - { - if (function_exists('sem_get') && version_compare(PHP_VERSION, '5.6.1', '>=')) { - if (empty(self::$semaphore[$key])) { - return false; - } else { - $success = @sem_release(self::$semaphore[$key]); - unset(self::$semaphore[$key]); - return $success; - } - } - } - - /** - * @brief Removes all lock that were set by us - * - * @return void - */ - public function releaseAll() - { - // not needed/supported - return; - } -} From 4b7be15560e3e9623d806a86bd999a513e281d4f Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Thu, 28 Jun 2018 23:02:00 +0200 Subject: [PATCH 08/12] Deleting return-types of methods --- src/Core/Cache/CacheDriverFactory.php | 2 +- src/Core/Lock/AbstractLockDriver.php | 2 +- src/Core/Lock/SemaphoreLockDriver.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Core/Cache/CacheDriverFactory.php b/src/Core/Cache/CacheDriverFactory.php index d15b8e9858..e9ff4331d7 100644 --- a/src/Core/Cache/CacheDriverFactory.php +++ b/src/Core/Cache/CacheDriverFactory.php @@ -20,7 +20,7 @@ class CacheDriverFactory * @return ICacheDriver The instance of the CacheDriver * @throws \Exception The exception if something went wrong during the CacheDriver creation */ - public static function create(string $driver) : ICacheDriver { + public static function create(string $driver) { switch ($driver) { case 'memcache': diff --git a/src/Core/Lock/AbstractLockDriver.php b/src/Core/Lock/AbstractLockDriver.php index 15820c3782..4c2bfaec95 100644 --- a/src/Core/Lock/AbstractLockDriver.php +++ b/src/Core/Lock/AbstractLockDriver.php @@ -22,7 +22,7 @@ abstract class AbstractLockDriver implements ILockDriver * @param string key The Name of the lock * @return bool Returns true if the lock is set */ - protected function hasAcquiredLock(string $key): bool { + protected function hasAcquiredLock(string $key) { return isset($this->acquireLock[$key]); } diff --git a/src/Core/Lock/SemaphoreLockDriver.php b/src/Core/Lock/SemaphoreLockDriver.php index 84965a1643..4eb30b9d02 100644 --- a/src/Core/Lock/SemaphoreLockDriver.php +++ b/src/Core/Lock/SemaphoreLockDriver.php @@ -18,7 +18,7 @@ class SemaphoreLockDriver extends AbstractLockDriver * * @return integer the semaphore key */ - private static function semaphoreKey(string $key): int + private static function semaphoreKey(string $key) { $temp = get_temppath(); From ad5ee75159f86010ad0acdf3475eb49f3b9f1192 Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Thu, 28 Jun 2018 23:06:14 +0200 Subject: [PATCH 09/12] Deleting parameter-types of methods (lack of support in PHP 5.6) --- src/Core/Cache/CacheDriverFactory.php | 2 +- src/Core/Lock/AbstractLockDriver.php | 6 +++--- src/Core/Lock/CacheLockDriver.php | 4 ++-- src/Core/Lock/DatabaseLockDriver.php | 4 ++-- src/Core/Lock/ILockDriver.php | 4 ++-- src/Core/Lock/SemaphoreLockDriver.php | 6 +++--- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/Core/Cache/CacheDriverFactory.php b/src/Core/Cache/CacheDriverFactory.php index e9ff4331d7..45cc17a52f 100644 --- a/src/Core/Cache/CacheDriverFactory.php +++ b/src/Core/Cache/CacheDriverFactory.php @@ -20,7 +20,7 @@ class CacheDriverFactory * @return ICacheDriver The instance of the CacheDriver * @throws \Exception The exception if something went wrong during the CacheDriver creation */ - public static function create(string $driver) { + public static function create($driver) { switch ($driver) { case 'memcache': diff --git a/src/Core/Lock/AbstractLockDriver.php b/src/Core/Lock/AbstractLockDriver.php index 4c2bfaec95..bcce26129c 100644 --- a/src/Core/Lock/AbstractLockDriver.php +++ b/src/Core/Lock/AbstractLockDriver.php @@ -22,7 +22,7 @@ abstract class AbstractLockDriver implements ILockDriver * @param string key The Name of the lock * @return bool Returns true if the lock is set */ - protected function hasAcquiredLock(string $key) { + protected function hasAcquiredLock($key) { return isset($this->acquireLock[$key]); } @@ -31,7 +31,7 @@ abstract class AbstractLockDriver implements ILockDriver * * @param string $key The Name of the lock */ - protected function markAcquire(string $key) { + protected function markAcquire($key) { $this->acquiredLocks[$key] = true; } @@ -40,7 +40,7 @@ abstract class AbstractLockDriver implements ILockDriver * * @param string $key The Name of the lock */ - protected function markRelease(string $key) { + protected function markRelease($key) { unset($this->acquiredLocks[$key]); } diff --git a/src/Core/Lock/CacheLockDriver.php b/src/Core/Lock/CacheLockDriver.php index 0adca140d1..1bb768bd0f 100644 --- a/src/Core/Lock/CacheLockDriver.php +++ b/src/Core/Lock/CacheLockDriver.php @@ -30,7 +30,7 @@ class CacheLockDriver extends AbstractLockDriver * * @return boolean Was the lock successful? */ - public function acquireLock(string $key, int $timeout = 120) + public function acquireLock($key, $timeout = 120) { $got_lock = false; $start = time(); @@ -71,7 +71,7 @@ class CacheLockDriver extends AbstractLockDriver * * @return mixed */ - public function releaseLock(string $key) + public function releaseLock($key) { $cachekey = get_app()->get_hostname() . ";lock:" . $key; $lock = $this->cache->get($cachekey); diff --git a/src/Core/Lock/DatabaseLockDriver.php b/src/Core/Lock/DatabaseLockDriver.php index f9878340cf..9b415753fc 100644 --- a/src/Core/Lock/DatabaseLockDriver.php +++ b/src/Core/Lock/DatabaseLockDriver.php @@ -18,7 +18,7 @@ class DatabaseLockDriver extends AbstractLockDriver * * @return boolean Was the lock successful? */ - public function acquireLock(string $key, int $timeout = 120) + public function acquireLock($key, $timeout = 120) { $got_lock = false; $start = time(); @@ -66,7 +66,7 @@ class DatabaseLockDriver extends AbstractLockDriver * * @return mixed */ - public function releaseLock(string $key) + public function releaseLock($key) { dba::delete('locks', ['locked' => false, 'pid' => 0], ['name' => $key, 'pid' => getmypid()]); diff --git a/src/Core/Lock/ILockDriver.php b/src/Core/Lock/ILockDriver.php index 39e4ba8e89..8744d757f1 100644 --- a/src/Core/Lock/ILockDriver.php +++ b/src/Core/Lock/ILockDriver.php @@ -18,7 +18,7 @@ interface ILockDriver * * @return boolean Was the lock successful? */ - public function acquireLock(string $key, int $timeout = 120); + public function acquireLock($key, $timeout = 120); /** * @brief Releases a lock if it was set by us @@ -27,7 +27,7 @@ interface ILockDriver * * @return void */ - public function releaseLock(string $key); + public function releaseLock($key); /** * @brief Releases all lock that were set by us diff --git a/src/Core/Lock/SemaphoreLockDriver.php b/src/Core/Lock/SemaphoreLockDriver.php index 4eb30b9d02..39e3e1d32c 100644 --- a/src/Core/Lock/SemaphoreLockDriver.php +++ b/src/Core/Lock/SemaphoreLockDriver.php @@ -18,7 +18,7 @@ class SemaphoreLockDriver extends AbstractLockDriver * * @return integer the semaphore key */ - private static function semaphoreKey(string $key) + private static function semaphoreKey($key) { $temp = get_temppath(); @@ -40,7 +40,7 @@ class SemaphoreLockDriver extends AbstractLockDriver * * @return boolean Was the lock successful? */ - public function acquireLock(string $key, int $timeout = 120) + public function acquireLock($key, $timeout = 120) { $this->acquiredLocks[$key] = sem_get(self::semaphoreKey($key)); if ($this->acquiredLocks[$key]) { @@ -55,7 +55,7 @@ class SemaphoreLockDriver extends AbstractLockDriver * * @return mixed */ - public function releaseLock(string $key) + public function releaseLock($key) { if (empty($this->acquiredLocks[$key])) { return false; From aac94d1d7445f2287f89a3559f4b3988e39edbdb Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Wed, 4 Jul 2018 23:37:22 +0200 Subject: [PATCH 10/12] 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 | 68 +++++++------ 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, 741 insertions(+), 154 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); - - 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; - } - } - if (is_bool($lock)) { - $this->cache->set($cachekey, getmypid(), 300); + // When we do want to lock something that was already locked by us. + if ((int)$lock == getmypid()) { $got_lock = true; } + // 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 (!$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 @@ + Date: Thu, 5 Jul 2018 07:59:56 +0200 Subject: [PATCH 11/12] code standards / simplifications --- src/Core/Cache/ArrayCache.php | 4 +-- src/Core/Cache/CacheDriverFactory.php | 4 +-- src/Core/Cache/ICacheDriver.php | 10 +++---- src/Core/Cache/IMemoryCacheDriver.php | 10 +++---- src/Core/Cache/MemcacheCacheDriver.php | 3 ++ src/Core/Cache/RedisCacheDriver.php | 21 ++------------ src/Core/Cache/TraitCompareDelete.php | 6 ++-- src/Core/Cache/TraitCompareSet.php | 6 ++-- src/Core/Lock.php | 18 ++---------- src/Core/Lock/AbstractLockDriver.php | 12 ++++---- src/Core/Lock/CacheLockDriver.php | 21 ++++---------- src/Core/Lock/DatabaseLockDriver.php | 4 +-- src/Core/Lock/ILockDriver.php | 12 ++++---- src/Core/Lock/SemaphoreLockDriver.php | 28 ++++--------------- tests/src/Core/Lock/CacheLockDriverTest.php | 2 +- .../src/Core/Lock/DatabaseLockDriverTest.php | 2 +- tests/src/Core/Lock/LockTest.php | 26 ++++++++--------- .../src/Core/Lock/SemaphoreLockDriverTest.php | 2 +- 18 files changed, 70 insertions(+), 121 deletions(-) diff --git a/src/Core/Cache/ArrayCache.php b/src/Core/Cache/ArrayCache.php index d4fe8bc7f7..71610c39df 100644 --- a/src/Core/Cache/ArrayCache.php +++ b/src/Core/Cache/ArrayCache.php @@ -6,7 +6,7 @@ namespace Friendica\Core\Cache; use Friendica\Core\Cache; /** - * @brief Implementation of the IMemoryCacheDriver mainly for testing purpose + * Implementation of the IMemoryCacheDriver mainly for testing purpose * * Class ArrayCache * @@ -80,4 +80,4 @@ class ArrayCache implements IMemoryCacheDriver return false; } } -} \ No newline at end of file +} diff --git a/src/Core/Cache/CacheDriverFactory.php b/src/Core/Cache/CacheDriverFactory.php index 45cc17a52f..8fbdc1549e 100644 --- a/src/Core/Cache/CacheDriverFactory.php +++ b/src/Core/Cache/CacheDriverFactory.php @@ -9,12 +9,12 @@ use Friendica\Core\Config; * * @package Friendica\Core\Cache * - * @brief A basic class to generate a CacheDriver + * A basic class to generate a CacheDriver */ class CacheDriverFactory { /** - * @brief This method creates a CacheDriver for the given cache driver name + * This method creates a CacheDriver for the given cache driver name * * @param string $driver The name of the cache driver * @return ICacheDriver The instance of the CacheDriver diff --git a/src/Core/Cache/ICacheDriver.php b/src/Core/Cache/ICacheDriver.php index ff329f34eb..ced7b4e216 100644 --- a/src/Core/Cache/ICacheDriver.php +++ b/src/Core/Cache/ICacheDriver.php @@ -12,7 +12,7 @@ use Friendica\Core\Cache; interface ICacheDriver { /** - * @brief Fetches cached data according to the key + * Fetches cached data according to the key * * @param string $key The key to the cached data * @@ -21,18 +21,18 @@ interface ICacheDriver public function get($key); /** - * @brief Stores data in the cache identified by the key. The input $value can have multiple formats. + * 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 $ttl 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, $ttl = Cache::FIVE_MINUTES); /** - * @brief Delete a key from the cache + * Delete a key from the cache * * @param string $key The cache key * @@ -41,7 +41,7 @@ interface ICacheDriver public function delete($key); /** - * @brief Remove outdated data from the cache + * Remove outdated data from the cache * * @return bool */ diff --git a/src/Core/Cache/IMemoryCacheDriver.php b/src/Core/Cache/IMemoryCacheDriver.php index 7843ca7b5f..a50e2d1d48 100644 --- a/src/Core/Cache/IMemoryCacheDriver.php +++ b/src/Core/Cache/IMemoryCacheDriver.php @@ -4,7 +4,7 @@ namespace Friendica\Core\Cache; use Friendica\Core\Cache; /** - * @brief This interface defines methods for Memory-Caches only + * This interface defines methods for Memory-Caches only * * Interface IMemoryCacheDriver * @@ -13,7 +13,7 @@ use Friendica\Core\Cache; interface IMemoryCacheDriver extends ICacheDriver { /** - * @brief Sets a value if it's not already stored + * 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 @@ -23,7 +23,7 @@ interface IMemoryCacheDriver extends ICacheDriver public function add($key, $value, $ttl = Cache::FIVE_MINUTES); /** - * @brief Compares if the old value is set and sets the new value + * 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 from the cache @@ -35,11 +35,11 @@ interface IMemoryCacheDriver extends ICacheDriver public function compareSet($key, $oldValue, $newValue, $ttl = Cache::FIVE_MINUTES); /** - * @brief Compares if the old value is set and removes it + * 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); -} \ No newline at end of file +} diff --git a/src/Core/Cache/MemcacheCacheDriver.php b/src/Core/Cache/MemcacheCacheDriver.php index 0b1ca3cec9..8eb45d907e 100644 --- a/src/Core/Cache/MemcacheCacheDriver.php +++ b/src/Core/Cache/MemcacheCacheDriver.php @@ -90,6 +90,9 @@ class MemcacheCacheDriver extends BaseObject implements IMemoryCacheDriver return $this->memcache->delete($key); } + /** + * (@inheritdoc) + */ public function clear() { return $this->memcache->flush(); diff --git a/src/Core/Cache/RedisCacheDriver.php b/src/Core/Cache/RedisCacheDriver.php index d23fa2697b..25c18aa6b6 100644 --- a/src/Core/Cache/RedisCacheDriver.php +++ b/src/Core/Cache/RedisCacheDriver.php @@ -84,12 +84,7 @@ class RedisCacheDriver extends BaseObject implements IMemoryCacheDriver /** - * @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 + * (@inheritdoc) */ public function add($key, $value, $ttl = Cache::FIVE_MINUTES) { @@ -101,13 +96,7 @@ class RedisCacheDriver extends BaseObject implements IMemoryCacheDriver } /** - * @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 + * (@inheritdoc) */ public function compareSet($key, $oldValue, $newValue, $ttl = Cache::FIVE_MINUTES) { @@ -133,11 +122,7 @@ class RedisCacheDriver extends BaseObject implements IMemoryCacheDriver 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 + * (@inheritdoc) */ public function compareDelete($key, $value) { diff --git a/src/Core/Cache/TraitCompareDelete.php b/src/Core/Cache/TraitCompareDelete.php index 898e39aecc..ef59f69cd1 100644 --- a/src/Core/Cache/TraitCompareDelete.php +++ b/src/Core/Cache/TraitCompareDelete.php @@ -7,7 +7,7 @@ use Friendica\Core\Cache; /** * Trait TraitCompareSetDelete * - * @brief This Trait is to compensate non native "exclusive" sets/deletes in caches + * This Trait is to compensate non native "exclusive" sets/deletes in caches * * @package Friendica\Core\Cache */ @@ -22,7 +22,7 @@ trait TraitCompareDelete abstract public function add($key, $value, $ttl = Cache::FIVE_MINUTES); /** - * @brief NonNative - Compares if the old value is set and removes it + * NonNative - 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 @@ -42,4 +42,4 @@ trait TraitCompareDelete return false; } } -} \ No newline at end of file +} diff --git a/src/Core/Cache/TraitCompareSet.php b/src/Core/Cache/TraitCompareSet.php index 55193b7567..77a6028355 100644 --- a/src/Core/Cache/TraitCompareSet.php +++ b/src/Core/Cache/TraitCompareSet.php @@ -7,7 +7,7 @@ use Friendica\Core\Cache; /** * Trait TraitCompareSetDelete * - * @brief This Trait is to compensate non native "exclusive" sets/deletes in caches + * This Trait is to compensate non native "exclusive" sets/deletes in caches * * @package Friendica\Core\Cache */ @@ -22,7 +22,7 @@ trait TraitCompareSet abstract public function add($key, $value, $ttl = Cache::FIVE_MINUTES); /** - * @brief NonNative - Compares if the old value is set and sets the new value + * NonNative - 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 from the cache @@ -45,4 +45,4 @@ trait TraitCompareSet return false; } } -} \ No newline at end of file +} diff --git a/src/Core/Lock.php b/src/Core/Lock.php index 7235c64a98..47a1f9b4fe 100644 --- a/src/Core/Lock.php +++ b/src/Core/Lock.php @@ -29,21 +29,9 @@ class Lock try { switch ($lock_driver) { case 'memcache': - $cache_driver = CacheDriverFactory::create('memcache'); - if ($cache_driver instanceof IMemoryCacheDriver) { - self::$driver = new Lock\CacheLockDriver($cache_driver); - } - break; - case 'memcached': - $cache_driver = CacheDriverFactory::create('memcached'); - if ($cache_driver instanceof IMemoryCacheDriver) { - self::$driver = new Lock\CacheLockDriver($cache_driver); - } - break; - case 'redis': - $cache_driver = CacheDriverFactory::create('redis'); + $cache_driver = CacheDriverFactory::create($lock_driver); if ($cache_driver instanceof IMemoryCacheDriver) { self::$driver = new Lock\CacheLockDriver($cache_driver); } @@ -129,7 +117,7 @@ class Lock */ public static function acquireLock($key, $timeout = 120) { - return self::getDriver()->acquireLock($key, $timeout); + return self::getDriver()->acquire($key, $timeout); } /** @@ -140,7 +128,7 @@ class Lock */ public static function releaseLock($key) { - return self::getDriver()->releaseLock($key); + return self::getDriver()->release($key); } /** diff --git a/src/Core/Lock/AbstractLockDriver.php b/src/Core/Lock/AbstractLockDriver.php index 09549c50bf..53597d45fc 100644 --- a/src/Core/Lock/AbstractLockDriver.php +++ b/src/Core/Lock/AbstractLockDriver.php @@ -8,7 +8,7 @@ use Friendica\BaseObject; * * @package Friendica\Core\Lock * - * @brief Basic class for Locking with common functions (local acquired locks, releaseAll, ..) + * Basic class for Locking with common functions (local acquired locks, releaseAll, ..) */ abstract class AbstractLockDriver extends BaseObject implements ILockDriver { @@ -18,7 +18,7 @@ abstract class AbstractLockDriver extends BaseObject implements ILockDriver protected $acquiredLocks = []; /** - * @brief Check if we've locally acquired a lock + * Check if we've locally acquired a lock * * @param string key The Name of the lock * @return bool Returns true if the lock is set @@ -28,7 +28,7 @@ abstract class AbstractLockDriver extends BaseObject implements ILockDriver } /** - * @brief Mark a locally acquired lock + * Mark a locally acquired lock * * @param string $key The Name of the lock */ @@ -37,7 +37,7 @@ abstract class AbstractLockDriver extends BaseObject implements ILockDriver } /** - * @brief Mark a release of a locally acquired lock + * Mark a release of a locally acquired lock * * @param string $key The Name of the lock */ @@ -46,13 +46,13 @@ abstract class AbstractLockDriver extends BaseObject implements ILockDriver } /** - * @brief Releases all lock that were set by us + * Releases all lock that were set by us * * @return void */ public function releaseAll() { foreach ($this->acquiredLocks as $acquiredLock => $hasLock) { - $this->releaseLock($acquiredLock); + $this->release($acquiredLock); } } } diff --git a/src/Core/Lock/CacheLockDriver.php b/src/Core/Lock/CacheLockDriver.php index 13d912c1e2..57627acecf 100644 --- a/src/Core/Lock/CacheLockDriver.php +++ b/src/Core/Lock/CacheLockDriver.php @@ -22,15 +22,9 @@ class CacheLockDriver 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) + public function acquire($key, $timeout = 120) { $got_lock = false; $start = time(); @@ -64,11 +58,9 @@ class CacheLockDriver extends AbstractLockDriver } /** - * @brief Removes a lock if it was set by us - * - * @param string $key Name of the lock + * (@inheritdoc) */ - public function releaseLock($key) + public function release($key) { $cachekey = self::getCacheKey($key); @@ -77,10 +69,7 @@ class CacheLockDriver extends AbstractLockDriver } /** - * @brief Checks, if a key is currently locked to a process - * - * @param string $key The name of the lock - * @return bool + * (@inheritdoc) */ public function isLocked($key) { diff --git a/src/Core/Lock/DatabaseLockDriver.php b/src/Core/Lock/DatabaseLockDriver.php index 6f4b942a4d..8f8e174214 100644 --- a/src/Core/Lock/DatabaseLockDriver.php +++ b/src/Core/Lock/DatabaseLockDriver.php @@ -14,7 +14,7 @@ class DatabaseLockDriver extends AbstractLockDriver /** * (@inheritdoc) */ - public function acquireLock($key, $timeout = 120) + public function acquire($key, $timeout = 120) { $got_lock = false; $start = time(); @@ -55,7 +55,7 @@ class DatabaseLockDriver extends AbstractLockDriver /** * (@inheritdoc) */ - public function releaseLock($key) + public function release($key) { dba::delete('locks', ['name' => $key, 'pid' => getmypid()]); diff --git a/src/Core/Lock/ILockDriver.php b/src/Core/Lock/ILockDriver.php index 3fbe049d37..af8a1d56ae 100644 --- a/src/Core/Lock/ILockDriver.php +++ b/src/Core/Lock/ILockDriver.php @@ -10,7 +10,7 @@ namespace Friendica\Core\Lock; interface ILockDriver { /** - * @brief Checks, if a key is currently locked to a or my process + * Checks, if a key is currently locked to a or my process * * @param string $key The name of the lock * @return bool @@ -19,26 +19,26 @@ interface ILockDriver /** * - * @brief Acquires a lock for a given name + * Acquires 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? */ - public function acquireLock($key, $timeout = 120); + public function acquire($key, $timeout = 120); /** - * @brief Releases a lock if it was set by us + * Releases a lock if it was set by us * * @param string $key The Name of the lock * * @return void */ - public function releaseLock($key); + public function release($key); /** - * @brief Releases all lock that were set by us + * Releases all lock that were set by us * * @return void */ diff --git a/src/Core/Lock/SemaphoreLockDriver.php b/src/Core/Lock/SemaphoreLockDriver.php index b4439743c8..250a75fbfe 100644 --- a/src/Core/Lock/SemaphoreLockDriver.php +++ b/src/Core/Lock/SemaphoreLockDriver.php @@ -14,11 +14,7 @@ class SemaphoreLockDriver extends AbstractLockDriver } /** - * @brief Creates a semaphore key - * - * @param string $key Name of the lock - * - * @return integer the semaphore key + * (@inheritdoc) */ private static function semaphoreKey($key) { @@ -35,14 +31,9 @@ class SemaphoreLockDriver 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) + public function acquire($key, $timeout = 120) { self::$semaphore[$key] = sem_get(self::semaphoreKey($key)); if (self::$semaphore[$key]) { @@ -56,13 +47,9 @@ class SemaphoreLockDriver extends AbstractLockDriver } /** - * @brief Removes a lock if it was set by us - * - * @param string $key Name of the lock - * - * @return mixed + * (@inheritdoc) */ - public function releaseLock($key) + public function release($key) { if (empty(self::$semaphore[$key])) { return false; @@ -75,10 +62,7 @@ class SemaphoreLockDriver extends AbstractLockDriver } /** - * @brief Checks, if a key is currently locked to a process - * - * @param string $key The name of the lock - * @return bool + * (@inheritdoc) */ public function isLocked($key) { diff --git a/tests/src/Core/Lock/CacheLockDriverTest.php b/tests/src/Core/Lock/CacheLockDriverTest.php index a089059725..b39000e119 100644 --- a/tests/src/Core/Lock/CacheLockDriverTest.php +++ b/tests/src/Core/Lock/CacheLockDriverTest.php @@ -1,6 +1,6 @@ instance->acquireLock('foo', 1); + $this->instance->acquire('foo', 1); $this->assertTrue($this->instance->isLocked('foo')); $this->assertFalse($this->instance->isLocked('bar')); } public function testDoubleLock() { - $this->instance->acquireLock('foo', 1); + $this->instance->acquire('foo', 1); $this->assertTrue($this->instance->isLocked('foo')); // We already locked it - $this->assertTrue($this->instance->acquireLock('foo', 1)); + $this->assertTrue($this->instance->acquire('foo', 1)); } public function testReleaseLock() { - $this->instance->acquireLock('foo', 1); + $this->instance->acquire('foo', 1); $this->assertTrue($this->instance->isLocked('foo')); - $this->instance->releaseLock('foo'); + $this->instance->release('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->acquire('foo', 1); + $this->instance->acquire('bar', 1); + $this->instance->acquire('#/$%§', 1); $this->instance->releaseAll(); @@ -66,11 +66,11 @@ abstract class LockTest extends TestCase } public function testReleaseAfterUnlock() { - $this->instance->acquireLock('foo', 1); - $this->instance->acquireLock('bar', 1); - $this->instance->acquireLock('#/$%§', 1); + $this->instance->acquire('foo', 1); + $this->instance->acquire('bar', 1); + $this->instance->acquire('#/$%§', 1); - $this->instance->releaseLock('foo'); + $this->instance->release('foo'); $this->instance->releaseAll(); diff --git a/tests/src/Core/Lock/SemaphoreLockDriverTest.php b/tests/src/Core/Lock/SemaphoreLockDriverTest.php index fb7efd6584..0fcf789e61 100644 --- a/tests/src/Core/Lock/SemaphoreLockDriverTest.php +++ b/tests/src/Core/Lock/SemaphoreLockDriverTest.php @@ -1,6 +1,6 @@ Date: Thu, 5 Jul 2018 20:57:31 +0200 Subject: [PATCH 12/12] Fixings - fixed test for semaphore - fixed some issues - changed namespace in Tests back to "src/" - changed namings --- src/Core/Cache/MemcachedCacheDriver.php | 2 +- src/Core/Cache/RedisCacheDriver.php | 2 +- src/Core/Lock.php | 14 ++++------ src/Core/Lock/AbstractLockDriver.php | 2 +- src/Core/Lock/CacheLockDriver.php | 4 +-- src/Core/Lock/DatabaseLockDriver.php | 4 +-- src/Core/Lock/ILockDriver.php | 4 +-- src/Core/Lock/SemaphoreLockDriver.php | 8 +++--- src/Core/Worker.php | 22 ++++++--------- src/Model/Item.php | 12 +++----- src/Protocol/OStatus.php | 10 +++---- tests/src/Core/Lock/CacheLockDriverTest.php | 2 +- .../src/Core/Lock/DatabaseLockDriverTest.php | 2 +- tests/src/Core/Lock/LockTest.php | 28 +++++++++---------- .../src/Core/Lock/SemaphoreLockDriverTest.php | 16 +++++++++-- 15 files changed, 67 insertions(+), 65 deletions(-) diff --git a/src/Core/Cache/MemcachedCacheDriver.php b/src/Core/Cache/MemcachedCacheDriver.php index 9c860711f8..dda36ba19c 100644 --- a/src/Core/Cache/MemcachedCacheDriver.php +++ b/src/Core/Cache/MemcachedCacheDriver.php @@ -16,7 +16,7 @@ class MemcachedCacheDriver extends BaseObject implements IMemoryCacheDriver use TraitCompareDelete; /** - * @var Memcached + * @var \Memcached */ private $memcached; diff --git a/src/Core/Cache/RedisCacheDriver.php b/src/Core/Cache/RedisCacheDriver.php index 25c18aa6b6..735418b2d2 100644 --- a/src/Core/Cache/RedisCacheDriver.php +++ b/src/Core/Cache/RedisCacheDriver.php @@ -14,7 +14,7 @@ use Friendica\Core\Cache; class RedisCacheDriver extends BaseObject implements IMemoryCacheDriver { /** - * @var Redis + * @var \Redis */ private $redis; diff --git a/src/Core/Lock.php b/src/Core/Lock.php index 47a1f9b4fe..9892f1f4e4 100644 --- a/src/Core/Lock.php +++ b/src/Core/Lock.php @@ -1,7 +1,5 @@ acquire($key, $timeout); + return self::getDriver()->acquireLock($key, $timeout); } /** * @brief Releases a lock if it was set by us * * @param string $key Name of the lock - * @return mixed + * @return void */ - public static function releaseLock($key) + public static function release($key) { - return self::getDriver()->release($key); + self::getDriver()->releaseLock($key); } /** diff --git a/src/Core/Lock/AbstractLockDriver.php b/src/Core/Lock/AbstractLockDriver.php index 53597d45fc..033d6f356e 100644 --- a/src/Core/Lock/AbstractLockDriver.php +++ b/src/Core/Lock/AbstractLockDriver.php @@ -52,7 +52,7 @@ abstract class AbstractLockDriver extends BaseObject implements ILockDriver */ public function releaseAll() { foreach ($this->acquiredLocks as $acquiredLock => $hasLock) { - $this->release($acquiredLock); + $this->releaseLock($acquiredLock); } } } diff --git a/src/Core/Lock/CacheLockDriver.php b/src/Core/Lock/CacheLockDriver.php index 57627acecf..2dd6f3fad9 100644 --- a/src/Core/Lock/CacheLockDriver.php +++ b/src/Core/Lock/CacheLockDriver.php @@ -24,7 +24,7 @@ class CacheLockDriver extends AbstractLockDriver /** * (@inheritdoc) */ - public function acquire($key, $timeout = 120) + public function acquireLock($key, $timeout = 120) { $got_lock = false; $start = time(); @@ -60,7 +60,7 @@ class CacheLockDriver extends AbstractLockDriver /** * (@inheritdoc) */ - public function release($key) + public function releaseLock($key) { $cachekey = self::getCacheKey($key); diff --git a/src/Core/Lock/DatabaseLockDriver.php b/src/Core/Lock/DatabaseLockDriver.php index 8f8e174214..6f4b942a4d 100644 --- a/src/Core/Lock/DatabaseLockDriver.php +++ b/src/Core/Lock/DatabaseLockDriver.php @@ -14,7 +14,7 @@ class DatabaseLockDriver extends AbstractLockDriver /** * (@inheritdoc) */ - public function acquire($key, $timeout = 120) + public function acquireLock($key, $timeout = 120) { $got_lock = false; $start = time(); @@ -55,7 +55,7 @@ class DatabaseLockDriver extends AbstractLockDriver /** * (@inheritdoc) */ - public function release($key) + public function releaseLock($key) { dba::delete('locks', ['name' => $key, 'pid' => getmypid()]); diff --git a/src/Core/Lock/ILockDriver.php b/src/Core/Lock/ILockDriver.php index af8a1d56ae..cd54cc03c3 100644 --- a/src/Core/Lock/ILockDriver.php +++ b/src/Core/Lock/ILockDriver.php @@ -26,7 +26,7 @@ interface ILockDriver * * @return boolean Was the lock successful? */ - public function acquire($key, $timeout = 120); + public function acquireLock($key, $timeout = 120); /** * Releases a lock if it was set by us @@ -35,7 +35,7 @@ interface ILockDriver * * @return void */ - public function release($key); + public function releaseLock($key); /** * Releases all lock that were set by us diff --git a/src/Core/Lock/SemaphoreLockDriver.php b/src/Core/Lock/SemaphoreLockDriver.php index 250a75fbfe..d032d46cc6 100644 --- a/src/Core/Lock/SemaphoreLockDriver.php +++ b/src/Core/Lock/SemaphoreLockDriver.php @@ -20,7 +20,7 @@ class SemaphoreLockDriver extends AbstractLockDriver { $temp = get_temppath(); - $file = $temp.'/'.$key.'.sem'; + $file = $temp . '/' . $key . '.sem'; if (!file_exists($file)) { file_put_contents($file, $key); @@ -33,7 +33,7 @@ class SemaphoreLockDriver extends AbstractLockDriver * * (@inheritdoc) */ - public function acquire($key, $timeout = 120) + public function acquireLock($key, $timeout = 120) { self::$semaphore[$key] = sem_get(self::semaphoreKey($key)); if (self::$semaphore[$key]) { @@ -49,7 +49,7 @@ class SemaphoreLockDriver extends AbstractLockDriver /** * (@inheritdoc) */ - public function release($key) + public function releaseLock($key) { if (empty(self::$semaphore[$key])) { return false; @@ -66,6 +66,6 @@ class SemaphoreLockDriver extends AbstractLockDriver */ public function isLocked($key) { - return @sem_get(self::$semaphore[$key]) !== false; + return isset(self::$semaphore[$key]); } } diff --git a/src/Core/Worker.php b/src/Core/Worker.php index cbf2ae8bd9..c4c91bbf8b 100644 --- a/src/Core/Worker.php +++ b/src/Core/Worker.php @@ -4,15 +4,11 @@ */ namespace Friendica\Core; -use Friendica\Core\Addon; -use Friendica\Core\Config; -use Friendica\Core\System; -use Friendica\Core\Lock; +use dba; use Friendica\Database\DBM; use Friendica\Model\Process; use Friendica\Util\DateTimeFormat; use Friendica\Util\Network; -use dba; require_once 'include/dba.php'; @@ -108,16 +104,16 @@ class Worker } // If possible we will fetch new jobs for this worker - if (!$refetched && Lock::acquireLock('worker_process', 0)) { + if (!$refetched && Lock::acquire('worker_process', 0)) { $stamp = (float)microtime(true); $refetched = self::findWorkerProcesses($passing_slow); self::$db_duration += (microtime(true) - $stamp); - Lock::releaseLock('worker_process'); + Lock::release('worker_process'); } } // To avoid the quitting of multiple workers only one worker at a time will execute the check - if (Lock::acquireLock('worker', 0)) { + if (Lock::acquire('worker', 0)) { $stamp = (float)microtime(true); // Count active workers and compare them with a maximum value that depends on the load if (self::tooMuchWorkers()) { @@ -130,7 +126,7 @@ class Worker logger('Memory limit reached, quitting.', LOGGER_DEBUG); return; } - Lock::releaseLock('worker'); + Lock::release('worker'); self::$db_duration += (microtime(true) - $stamp); } @@ -883,7 +879,7 @@ class Worker dba::close($r); $stamp = (float)microtime(true); - if (!Lock::acquireLock('worker_process')) { + if (!Lock::acquire('worker_process')) { return false; } self::$lock_duration = (microtime(true) - $stamp); @@ -892,7 +888,7 @@ class Worker $found = self::findWorkerProcesses($passing_slow); self::$db_duration += (microtime(true) - $stamp); - Lock::releaseLock('worker_process'); + Lock::release('worker_process'); if ($found) { $r = dba::select('workerqueue', [], ['pid' => getmypid(), 'done' => false]); @@ -1097,13 +1093,13 @@ class Worker } // If there is a lock then we don't have to check for too much worker - if (!Lock::acquireLock('worker', 0)) { + if (!Lock::acquire('worker', 0)) { return true; } // If there are already enough workers running, don't fork another one $quit = self::tooMuchWorkers(); - Lock::releaseLock('worker'); + Lock::release('worker'); if ($quit) { return true; diff --git a/src/Model/Item.php b/src/Model/Item.php index ac53bb0220..d31d7c1323 100644 --- a/src/Model/Item.php +++ b/src/Model/Item.php @@ -6,26 +6,22 @@ namespace Friendica\Model; +use dba; use Friendica\BaseObject; use Friendica\Content\Text; use Friendica\Core\Addon; use Friendica\Core\Config; use Friendica\Core\L10n; +use Friendica\Core\Lock; use Friendica\Core\PConfig; use Friendica\Core\System; use Friendica\Core\Worker; use Friendica\Database\DBM; -use Friendica\Model\Contact; -use Friendica\Model\Conversation; -use Friendica\Model\Group; -use Friendica\Model\Term; use Friendica\Object\Image; use Friendica\Protocol\Diaspora; use Friendica\Protocol\OStatus; use Friendica\Util\DateTimeFormat; use Friendica\Util\XML; -use Friendica\Util\Lock; -use dba; use Text_LanguageDetect; require_once 'boot.php'; @@ -1595,7 +1591,7 @@ class Item extends BaseObject } // To avoid timing problems, we are using locks. - $locked = Lock::set('item_insert_content'); + $locked = Lock::acquire('item_insert_content'); if (!$locked) { logger("Couldn't acquire lock for URI " . $item['uri'] . " - proceeding anyway."); } @@ -1615,7 +1611,7 @@ class Item extends BaseObject logger('Could not insert content for URI ' . $item['uri'] . ' - trying asynchronously'); } if ($locked) { - Lock::remove('item_insert_content'); + Lock::release('item_insert_content'); } } diff --git a/src/Protocol/OStatus.php b/src/Protocol/OStatus.php index d94f44814f..dadc19dcca 100644 --- a/src/Protocol/OStatus.php +++ b/src/Protocol/OStatus.php @@ -4,6 +4,9 @@ */ namespace Friendica\Protocol; +use dba; +use DOMDocument; +use DOMXPath; use Friendica\Content\Text\BBCode; use Friendica\Content\Text\HTML; use Friendica\Core\Cache; @@ -22,9 +25,6 @@ use Friendica\Object\Image; use Friendica\Util\DateTimeFormat; use Friendica\Util\Network; use Friendica\Util\XML; -use dba; -use DOMDocument; -use DOMXPath; require_once 'include/dba.php'; require_once 'include/items.php'; @@ -513,9 +513,9 @@ class OStatus logger("Item with uri ".$item["uri"]." is from a blocked contact.", LOGGER_DEBUG); } else { // We are having duplicated entries. Hopefully this solves it. - if (Lock::acquireLock('ostatus_process_item_insert')) { + if (Lock::acquire('ostatus_process_item_insert')) { $ret = Item::insert($item); - Lock::releaseLock('ostatus_process_item_insert'); + Lock::release('ostatus_process_item_insert'); logger("Item with uri ".$item["uri"]." for user ".$importer["uid"].' stored. Return value: '.$ret); } else { $ret = Item::insert($item); diff --git a/tests/src/Core/Lock/CacheLockDriverTest.php b/tests/src/Core/Lock/CacheLockDriverTest.php index b39000e119..a089059725 100644 --- a/tests/src/Core/Lock/CacheLockDriverTest.php +++ b/tests/src/Core/Lock/CacheLockDriverTest.php @@ -1,6 +1,6 @@ instance->acquire('foo', 1); + $this->instance->acquireLock('foo', 1); $this->assertTrue($this->instance->isLocked('foo')); $this->assertFalse($this->instance->isLocked('bar')); } public function testDoubleLock() { - $this->instance->acquire('foo', 1); + $this->instance->acquireLock('foo', 1); $this->assertTrue($this->instance->isLocked('foo')); // We already locked it - $this->assertTrue($this->instance->acquire('foo', 1)); + $this->assertTrue($this->instance->acquireLock('foo', 1)); } public function testReleaseLock() { - $this->instance->acquire('foo', 1); + $this->instance->acquireLock('foo', 1); $this->assertTrue($this->instance->isLocked('foo')); - $this->instance->release('foo'); + $this->instance->releaseLock('foo'); $this->assertFalse($this->instance->isLocked('foo')); } public function testReleaseAll() { - $this->instance->acquire('foo', 1); - $this->instance->acquire('bar', 1); - $this->instance->acquire('#/$%§', 1); + $this->instance->acquireLock('foo', 1); + $this->instance->acquireLock('bar', 1); + $this->instance->acquireLock('nice', 1); $this->instance->releaseAll(); $this->assertFalse($this->instance->isLocked('foo')); $this->assertFalse($this->instance->isLocked('bar')); - $this->assertFalse($this->instance->isLocked('#/$%§')); + $this->assertFalse($this->instance->isLocked('nice')); } public function testReleaseAfterUnlock() { - $this->instance->acquire('foo', 1); - $this->instance->acquire('bar', 1); - $this->instance->acquire('#/$%§', 1); + $this->instance->acquireLock('foo', 1); + $this->instance->acquireLock('bar', 1); + $this->instance->acquireLock('nice', 1); - $this->instance->release('foo'); + $this->instance->releaseLock('foo'); $this->instance->releaseAll(); diff --git a/tests/src/Core/Lock/SemaphoreLockDriverTest.php b/tests/src/Core/Lock/SemaphoreLockDriverTest.php index 0fcf789e61..56c96458f2 100644 --- a/tests/src/Core/Lock/SemaphoreLockDriverTest.php +++ b/tests/src/Core/Lock/SemaphoreLockDriverTest.php @@ -1,14 +1,26 @@ semaphoreLockDriver = new SemaphoreLockDriver(); + return $this->semaphoreLockDriver; + } + + public function tearDown() + { + $this->semaphoreLockDriver->releaseAll(); + parent::tearDown(); } } \ No newline at end of file