From e41e7d2edd286483b2001b9164ca3039997e7634 Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Thu, 5 Jul 2018 20:57:31 +0200 Subject: [PATCH] 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 -- 2.39.5