diff --git a/src/Core/Cache.php b/src/Core/Cache.php index 4202db325..cc77d9bfd 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/ArrayCache.php b/src/Core/Cache/ArrayCache.php new file mode 100644 index 000000000..71610c39d --- /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; + } + } +} diff --git a/src/Core/Cache/CacheDriverFactory.php b/src/Core/Cache/CacheDriverFactory.php new file mode 100644 index 000000000..8fbdc1549 --- /dev/null +++ b/src/Core/Cache/CacheDriverFactory.php @@ -0,0 +1,48 @@ + 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 be896edf7..ced7b4e21 100644 --- a/src/Core/Cache/ICacheDriver.php +++ b/src/Core/Cache/ICacheDriver.php @@ -25,17 +25,16 @@ interface ICacheDriver * * @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 * - * @param string $key + * @param string $key The cache key * * @return bool */ diff --git a/src/Core/Cache/IMemoryCacheDriver.php b/src/Core/Cache/IMemoryCacheDriver.php new file mode 100644 index 000000000..a50e2d1d4 --- /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,24 +60,49 @@ 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); } + /** + * (@inheritdoc) + */ 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 78a4a6bdf..dda36ba19 100644 --- a/src/Core/Cache/MemcachedCacheDriver.php +++ b/src/Core/Cache/MemcachedCacheDriver.php @@ -10,10 +10,13 @@ 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 + * @var \Memcached */ private $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 fa98842da..735418b2d 100644 --- a/src/Core/Cache/RedisCacheDriver.php +++ b/src/Core/Cache/RedisCacheDriver.php @@ -11,10 +11,10 @@ use Friendica\Core\Cache; * @author Hypolite Petovan * @author Roland Haeder */ -class RedisCacheDriver extends BaseObject implements ICacheDriver +class RedisCacheDriver extends BaseObject implements IMemoryCacheDriver { /** - * @var Redis + * @var \Redis */ private $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,60 @@ class RedisCacheDriver extends BaseObject implements ICacheDriver { return true; } + + + /** + * (@inheritdoc) + */ + 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); + } + + /** + * (@inheritdoc) + */ + 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; + } + /** + * (@inheritdoc) + */ + 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 000000000..ef59f69cd --- /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; + } + } +} diff --git a/src/Core/Cache/TraitCompareSet.php b/src/Core/Cache/TraitCompareSet.php new file mode 100644 index 000000000..77a602835 --- /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; + } + } +} diff --git a/src/Core/Lock.php b/src/Core/Lock.php new file mode 100644 index 000000000..9892f1f4e --- /dev/null +++ b/src/Core/Lock.php @@ -0,0 +1,140 @@ +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); + 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()); + } + } + + // 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 acquire($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 void + */ + public static function release($key) + { + 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 000000000..033d6f356 --- /dev/null +++ b/src/Core/Lock/AbstractLockDriver.php @@ -0,0 +1,58 @@ +acquireLock[$key]) && $this->acquiredLocks[$key] === true; + } + + /** + * Mark a locally acquired lock + * + * @param string $key The Name of the lock + */ + protected function markAcquire($key) { + $this->acquiredLocks[$key] = true; + } + + /** + * Mark a release of a locally acquired lock + * + * @param string $key The Name of the lock + */ + protected function markRelease($key) { + unset($this->acquiredLocks[$key]); + } + + /** + * Releases all lock that were set by us + * + * @return void + */ + public function releaseAll() { + foreach ($this->acquiredLocks as $acquiredLock => $hasLock) { + $this->releaseLock($acquiredLock); + } + } +} diff --git a/src/Core/Lock/CacheLockDriver.php b/src/Core/Lock/CacheLockDriver.php new file mode 100644 index 000000000..2dd6f3fad --- /dev/null +++ b/src/Core/Lock/CacheLockDriver.php @@ -0,0 +1,88 @@ +cache = $cache; + } + + /** + * (@inheritdoc) + */ + public function acquireLock($key, $timeout = 120) + { + $got_lock = false; + $start = time(); + + $cachekey = self::getCacheKey($key); + + do { + $lock = $this->cache->get($cachekey); + // When we do want to lock something that was already locked by us. + if ((int)$lock == getmypid()) { + $got_lock = true; + } + + // 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)); + + return $got_lock; + } + + /** + * (@inheritdoc) + */ + public function releaseLock($key) + { + $cachekey = self::getCacheKey($key); + + $this->cache->compareDelete($cachekey, getmypid()); + $this->markRelease($key); + } + + /** + * (@inheritdoc) + */ + 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 new file mode 100644 index 000000000..6f4b942a4 --- /dev/null +++ b/src/Core/Lock/DatabaseLockDriver.php @@ -0,0 +1,90 @@ += ?', $key, DateTimeFormat::utcNow()]); + + if (DBM::is_result($lock)) { + if ($lock['locked']) { + // 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(), 'expires' => DateTimeFormat::utc('now + 300seconds')], ['name' => $key]); + $got_lock = true; + $this->markAcquire($key); + } + } else { + dba::insert('locks', ['name' => $key, 'locked' => true, 'pid' => getmypid(), 'expires' => DateTimeFormat::utc('now + 300seconds')]); + $got_lock = true; + $this->markAcquire($key); + } + + dba::unlock(); + + if (!$got_lock && ($timeout > 0)) { + usleep(rand(100000, 2000000)); + } + } while (!$got_lock && ((time() - $start) < $timeout)); + + return $got_lock; + } + + /** + * (@inheritdoc) + */ + public function releaseLock($key) + { + dba::delete('locks', ['name' => $key, 'pid' => getmypid()]); + + $this->markRelease($key); + + return; + } + + /** + * (@inheritdoc) + */ + public function releaseAll() + { + 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 new file mode 100644 index 000000000..cd54cc03c --- /dev/null +++ b/src/Core/Lock/ILockDriver.php @@ -0,0 +1,46 @@ + + */ +interface ILockDriver +{ + /** + * 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); + + /** + * + * 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); + + /** + * Releases a lock if it was set by us + * + * @param string $key The Name of the lock + * + * @return void + */ + public function releaseLock($key); + + /** + * Releases all lock that were set by us + * + * @return void + */ + public function releaseAll(); +} diff --git a/src/Core/Lock/SemaphoreLockDriver.php b/src/Core/Lock/SemaphoreLockDriver.php new file mode 100644 index 000000000..d032d46cc --- /dev/null +++ b/src/Core/Lock/SemaphoreLockDriver.php @@ -0,0 +1,71 @@ +markAcquire($key); + return true; + } + } + + return false; + } + + /** + * (@inheritdoc) + */ + public function releaseLock($key) + { + if (empty(self::$semaphore[$key])) { + return false; + } else { + $success = @sem_release(self::$semaphore[$key]); + unset(self::$semaphore[$key]); + $this->markRelease($key); + return $success; + } + } + + /** + * (@inheritdoc) + */ + public function isLocked($key) + { + return isset(self::$semaphore[$key]); + } +} diff --git a/src/Core/Worker.php b/src/Core/Worker.php index c965e0583..c4c91bbf8 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 dba; use Friendica\Database\DBM; use Friendica\Model\Process; use Friendica\Util\DateTimeFormat; -use Friendica\Util\Lock; 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::set('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::remove('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::set('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::remove('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::set('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::remove('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::set('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::remove('worker'); + Lock::release('worker'); if ($quit) { return true; diff --git a/src/Database/DBStructure.php b/src/Database/DBStructure.php index db6997ec6..d3ce350a7 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'; @@ -1283,9 +1282,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/src/Model/Item.php b/src/Model/Item.php index ac53bb022..d31d7c132 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 5a30bfd2c..dadc19dcc 100644 --- a/src/Protocol/OStatus.php +++ b/src/Protocol/OStatus.php @@ -4,11 +4,15 @@ */ namespace Friendica\Protocol; +use dba; +use DOMDocument; +use DOMXPath; use Friendica\Content\Text\BBCode; 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,12 +23,8 @@ 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; -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::set('ostatus_process_item_insert')) { + if (Lock::acquire('ostatus_process_item_insert')) { $ret = Item::insert($item); - Lock::remove('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/src/Util/Lock.php b/src/Util/Lock.php deleted file mode 100644 index eba264ad9..000000000 --- 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; - } -} diff --git a/tests/datasets/api.yml b/tests/datasets/api.yml index 9ba5ec387..14053c3e8 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 000000000..a08905972 --- /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 000000000..a80ff4c37 --- /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 000000000..c8c0c32ae --- /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('nice', 1); + + $this->instance->releaseAll(); + + $this->assertFalse($this->instance->isLocked('foo')); + $this->assertFalse($this->instance->isLocked('bar')); + $this->assertFalse($this->instance->isLocked('nice')); + } + + public function testReleaseAfterUnlock() { + $this->instance->acquireLock('foo', 1); + $this->instance->acquireLock('bar', 1); + $this->instance->acquireLock('nice', 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 000000000..56c96458f --- /dev/null +++ b/tests/src/Core/Lock/SemaphoreLockDriverTest.php @@ -0,0 +1,26 @@ +semaphoreLockDriver = new SemaphoreLockDriver(); + return $this->semaphoreLockDriver; + } + + public function tearDown() + { + $this->semaphoreLockDriver->releaseAll(); + parent::tearDown(); + } +} \ No newline at end of file