Merge pull request #8071 from nupplaphil/task/Lock_to_lock

Replace Core\Lock wrapper with DI::lock() method
This commit is contained in:
Hypolite Petovan 2020-01-08 14:25:46 -05:00 committed by GitHub
commit f67f398fe1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 75 additions and 130 deletions

View file

@ -133,7 +133,7 @@ HELP;
if (count($this->args) >= 2) {
$lock = $this->getArgument(1);
if ($this->lock->releaseLock($lock, true)) {
if ($this->lock->release($lock, true)) {
$this->out(sprintf('Lock \'%s\' released.', $lock));
} else {
$this->out(sprintf('Couldn\'t release Lock \'%s\'', $lock));
@ -156,11 +156,11 @@ HELP;
}
if (!empty($ttl) && !empty($timeout)) {
$result = $this->lock->acquireLock($lock, $timeout, $ttl);
$result = $this->lock->acquire($lock, $timeout, $ttl);
} elseif (!empty($timeout)) {
$result = $this->lock->acquireLock($lock, $timeout);
$result = $this->lock->acquire($lock, $timeout);
} else {
$result = $this->lock->acquireLock($lock);
$result = $this->lock->acquire($lock);
}
if ($result) {

View file

@ -1,56 +0,0 @@
<?php
/**
* @file src/Core/Lock.php
* @brief Functions for preventing parallel execution of functions
*/
namespace Friendica\Core;
use Friendica\Core\Cache\Cache;
use Friendica\DI;
/**
* This class contain Functions for preventing parallel execution of functions
*/
class Lock
{
/**
* @brief Acquires a lock for a given name
*
* @param string $key Name of the lock
* @param integer $timeout Seconds until we give up
* @param integer $ttl The Lock lifespan, must be one of the Cache constants
*
* @return boolean Was the lock successful?
* @throws \Exception
*/
public static function acquire($key, $timeout = 120, $ttl = Cache::FIVE_MINUTES)
{
return DI::lock()->acquireLock($key, $timeout, $ttl);
}
/**
* @brief Releases a lock if it was set by us
*
* @param string $key Name of the lock
* @param bool $override Overrides the lock to get releases
*
* @return bool
* @throws \Exception
*/
public static function release($key, $override = false)
{
return DI::lock()->releaseLock($key, $override);
}
/**
* @brief Releases all lock that were set by us
* @return void
* @throws \Exception
*/
public static function releaseAll()
{
DI::lock()->releaseAll();
}
}

View file

@ -30,7 +30,7 @@ class CacheLock extends Lock
/**
* (@inheritdoc)
*/
public function acquireLock($key, $timeout = 120, $ttl = Cache\Cache::FIVE_MINUTES)
public function acquire($key, $timeout = 120, $ttl = Cache\Cache::FIVE_MINUTES)
{
$got_lock = false;
$start = time();
@ -66,7 +66,7 @@ class CacheLock extends Lock
/**
* (@inheritdoc)
*/
public function releaseLock($key, $override = false)
public function release($key, $override = false)
{
$cachekey = self::getLockKey($key);
@ -122,7 +122,7 @@ class CacheLock extends Lock
$locks = $this->getLocks();
foreach ($locks as $lock) {
if (!$this->releaseLock($lock, $override)) {
if (!$this->release($lock, $override)) {
$success = false;
}
}

View file

@ -35,7 +35,7 @@ class DatabaseLock extends Lock
/**
* (@inheritdoc)
*/
public function acquireLock($key, $timeout = 120, $ttl = Cache::FIVE_MINUTES)
public function acquire($key, $timeout = 120, $ttl = Cache::FIVE_MINUTES)
{
$got_lock = false;
$start = time();
@ -74,7 +74,7 @@ class DatabaseLock extends Lock
/**
* (@inheritdoc)
*/
public function releaseLock($key, $override = false)
public function release($key, $override = false)
{
if ($override) {
$where = ['name' => $key];

View file

@ -30,7 +30,7 @@ interface ILock
*
* @return boolean Was the lock successful?
*/
public function acquireLock($key, $timeout = 120, $ttl = Cache\Cache::FIVE_MINUTES);
public function acquire($key, $timeout = 120, $ttl = Cache\Cache::FIVE_MINUTES);
/**
* Releases a lock if it was set by us
@ -40,7 +40,7 @@ interface ILock
*
* @return boolean Was the unlock successful?
*/
public function releaseLock($key, $override = false);
public function release($key, $override = false);
/**
* Releases all lock that were set by us

View file

@ -61,7 +61,7 @@ abstract class Lock implements ILock
$return = true;
foreach ($this->acquiredLocks as $acquiredLock => $hasLock) {
if (!$this->releaseLock($acquiredLock, $override)) {
if (!$this->release($acquiredLock, $override)) {
$return = false;
}
}

View file

@ -36,7 +36,7 @@ class SemaphoreLock extends Lock
/**
* (@inheritdoc)
*/
public function acquireLock($key, $timeout = 120, $ttl = Cache\Cache::FIVE_MINUTES)
public function acquire($key, $timeout = 120, $ttl = Cache\Cache::FIVE_MINUTES)
{
self::$semaphore[$key] = sem_get(self::semaphoreKey($key));
if (!empty(self::$semaphore[$key])) {
@ -55,7 +55,7 @@ class SemaphoreLock extends Lock
* @param bool $override not necessary parameter for semaphore locks since the lock lives as long as the execution
* of the using function
*/
public function releaseLock($key, $override = false)
public function release($key, $override = false)
{
$success = false;

View file

@ -5,6 +5,7 @@ namespace Friendica\Core;
use Friendica\App;
use Friendica\Database\DBA;
use Friendica\Database\DBStructure;
use Friendica\DI;
use Friendica\Util\Strings;
class Update
@ -73,7 +74,7 @@ class Update
// In force mode, we release the dbupdate lock first
// Necessary in case of an stuck update
if ($override) {
Lock::release('dbupdate', true);
DI::lock()->release('dbupdate', true);
}
$build = Config::get('system', 'build', null, true);
@ -95,13 +96,13 @@ class Update
// Compare the current structure with the defined structure
// If the Lock is acquired, never release it automatically to avoid double updates
if (Lock::acquire('dbupdate', 120, Cache::INFINITE)) {
if (DI::lock()->acquire('dbupdate', 120, Cache::INFINITE)) {
// Checks if the build changed during Lock acquiring (so no double update occurs)
$retryBuild = Config::get('system', 'build', null, true);
if ($retryBuild !== $build) {
Logger::info('Update already done.', ['from' => $stored, 'to' => $current]);
Lock::release('dbupdate');
DI::lock()->release('dbupdate');
return '';
}
@ -110,7 +111,7 @@ class Update
$r = self::runUpdateFunction($x, 'pre_update');
if (!$r) {
Config::set('system', 'update', Update::FAILED);
Lock::release('dbupdate');
DI::lock()->release('dbupdate');
return $r;
}
}
@ -126,7 +127,7 @@ class Update
}
Logger::error('Update ERROR.', ['from' => $stored, 'to' => $current, 'retval' => $retval]);
Config::set('system', 'update', Update::FAILED);
Lock::release('dbupdate');
DI::lock()->release('dbupdate');
return $retval;
} else {
Config::set('database', 'last_successful_update', $current);
@ -139,7 +140,7 @@ class Update
$r = self::runUpdateFunction($x, 'update');
if (!$r) {
Config::set('system', 'update', Update::FAILED);
Lock::release('dbupdate');
DI::lock()->release('dbupdate');
return $r;
}
}
@ -150,7 +151,7 @@ class Update
}
Config::set('system', 'update', Update::SUCCESS);
Lock::release('dbupdate');
DI::lock()->release('dbupdate');
}
}
}
@ -181,7 +182,7 @@ class Update
// If the update fails or times-out completely you may need to
// delete the config entry to try again.
if (Lock::acquire('dbupdate_function', 120,Cache::INFINITE)) {
if (DI::lock()->acquire('dbupdate_function', 120,Cache::INFINITE)) {
// call the specific update
$retval = $funcname();
@ -193,7 +194,7 @@ class Update
L10n::t('Update %s failed. See error logs.', $x)
);
Logger::error('Update function ERROR.', ['function' => $funcname, 'retval' => $retval]);
Lock::release('dbupdate_function');
DI::lock()->release('dbupdate_function');
return false;
} else {
Config::set('database', 'last_successful_update_function', $funcname);
@ -203,7 +204,7 @@ class Update
Config::set('system', 'build', $x);
}
Lock::release('dbupdate_function');
DI::lock()->release('dbupdate_function');
Logger::info('Update function finished.', ['function' => $funcname]);
return true;
}

View file

@ -115,9 +115,9 @@ class Worker
}
// Trying to fetch new processes - but only once when successful
if (!$refetched && Lock::acquire('worker_process', 0)) {
if (!$refetched && DI::lock()->acquire('worker_process', 0)) {
self::findWorkerProcesses();
Lock::release('worker_process');
DI::lock()->release('worker_process');
self::$state = self::STATE_REFETCH;
$refetched = true;
} else {
@ -129,21 +129,21 @@ class Worker
if (!self::getWaitingJobForPID()) {
self::$state = self::STATE_LONG_LOOP;
if (Lock::acquire('worker', 0)) {
if (DI::lock()->acquire('worker', 0)) {
// Count active workers and compare them with a maximum value that depends on the load
if (self::tooMuchWorkers()) {
Logger::log('Active worker limit reached, quitting.', Logger::DEBUG);
Lock::release('worker');
DI::lock()->release('worker');
return;
}
// Check free memory
if (DI::process()->isMinMemoryReached()) {
Logger::log('Memory limit reached, quitting.', Logger::DEBUG);
Lock::release('worker');
DI::lock()->release('worker');
return;
}
Lock::release('worker');
DI::lock()->release('worker');
}
}
@ -933,14 +933,14 @@ class Worker
}
$stamp = (float)microtime(true);
if (!Lock::acquire('worker_process')) {
if (!DI::lock()->acquire('worker_process')) {
return false;
}
self::$lock_duration += (microtime(true) - $stamp);
$found = self::findWorkerProcesses();
Lock::release('worker_process');
DI::lock()->release('worker_process');
if ($found) {
$stamp = (float)microtime(true);
@ -1172,13 +1172,13 @@ class Worker
}
// If there is a lock then we don't have to check for too much worker
if (!Lock::acquire('worker', 0)) {
if (!DI::lock()->acquire('worker', 0)) {
return $added;
}
// If there are already enough workers running, don't fork another one
$quit = self::tooMuchWorkers();
Lock::release('worker');
DI::lock()->release('worker');
if ($quit) {
return $added;

View file

@ -2060,7 +2060,7 @@ class Item
}
// To avoid timing problems, we are using locks.
$locked = Lock::acquire('item_insert_activity');
$locked = DI::lock()->acquire('item_insert_activity');
if (!$locked) {
Logger::log("Couldn't acquire lock for URI " . $item['uri'] . " - proceeding anyway.");
}
@ -2076,11 +2076,11 @@ class Item
} else {
// This shouldn't happen.
Logger::log('Could not insert activity for URI ' . $item['uri'] . ' - should not happen');
Lock::release('item_insert_activity');
DI::lock()->release('item_insert_activity');
return false;
}
if ($locked) {
Lock::release('item_insert_activity');
DI::lock()->release('item_insert_activity');
}
return true;
}
@ -2103,7 +2103,7 @@ class Item
}
// To avoid timing problems, we are using locks.
$locked = Lock::acquire('item_insert_content');
$locked = DI::lock()->acquire('item_insert_content');
if (!$locked) {
Logger::log("Couldn't acquire lock for URI " . $item['uri'] . " - proceeding anyway.");
}
@ -2121,7 +2121,7 @@ class Item
Logger::log('Could not insert content for URI ' . $item['uri'] . ' - should not happen');
}
if ($locked) {
Lock::release('item_insert_content');
DI::lock()->release('item_insert_content');
}
}

View file

@ -539,9 +539,9 @@ class OStatus
Logger::log("Item with uri ".$item["uri"]." is from a blocked contact.", Logger::DEBUG);
} else {
// We are having duplicated entries. Hopefully this solves it.
if (Lock::acquire('ostatus_process_item_insert')) {
if (DI::lock()->acquire('ostatus_process_item_insert')) {
$ret = Item::insert($item);
Lock::release('ostatus_process_item_insert');
DI::lock()->release('ostatus_process_item_insert');
Logger::log("Item with uri ".$item["uri"]." for user ".$importer["uid"].' stored. Return value: '.$ret);
} else {
$ret = Item::insert($item);

View file

@ -22,7 +22,7 @@ trait DbaLockMockTrait
* @param null $time The current timestamp
* @param null|int $times How often the method will get used
*
*@see DatabaseLock::acquireLock()
*@see DatabaseLock::acquire()
*
*/
public function mockAcquireLock($key, $ttl = Cache::FIVE_MINUTES, $locked = false, $pid = null, $rowExists = true, $time = null, $times = null)
@ -103,7 +103,7 @@ trait DbaLockMockTrait
* @param null|int $pid The PID which was set
* @param null|int $times How often the method will get used
*
*@see DatabaseLock::releaseLock()
*@see DatabaseLock::release()
*
*/
public function mockReleaseLock($key, $pid = null, $times = null)

View file

@ -68,7 +68,7 @@ class LockConsoleTest extends ConsoleTest
public function testDelLock()
{
$this->lockMock
->shouldReceive('releaseLock')
->shouldReceive('release')
->with('test', true)
->andReturn(true)
->once();
@ -83,7 +83,7 @@ class LockConsoleTest extends ConsoleTest
public function testDelUnknownLock()
{
$this->lockMock
->shouldReceive('releaseLock')
->shouldReceive('release')
->with('test', true)
->andReturn(false)
->once();
@ -103,7 +103,7 @@ class LockConsoleTest extends ConsoleTest
->andReturn(false)
->once();
$this->lockMock
->shouldReceive('acquireLock')
->shouldReceive('acquire')
->with('test')
->andReturn(true)
->once();
@ -138,7 +138,7 @@ class LockConsoleTest extends ConsoleTest
->andReturn(false)
->once();
$this->lockMock
->shouldReceive('acquireLock')
->shouldReceive('acquire')
->with('test')
->andReturn(false)
->once();

View file

@ -38,7 +38,7 @@ abstract class LockTest extends MockedTest
public function testLock()
{
$this->assertFalse($this->instance->isLocked('foo'));
$this->assertTrue($this->instance->acquireLock('foo', 1));
$this->assertTrue($this->instance->acquire('foo', 1));
$this->assertTrue($this->instance->isLocked('foo'));
$this->assertFalse($this->instance->isLocked('bar'));
}
@ -49,10 +49,10 @@ abstract class LockTest extends MockedTest
public function testDoubleLock()
{
$this->assertFalse($this->instance->isLocked('foo'));
$this->assertTrue($this->instance->acquireLock('foo', 1));
$this->assertTrue($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));
}
/**
@ -61,9 +61,9 @@ abstract class LockTest extends MockedTest
public function testReleaseLock()
{
$this->assertFalse($this->instance->isLocked('foo'));
$this->assertTrue($this->instance->acquireLock('foo', 1));
$this->assertTrue($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'));
}
@ -72,9 +72,9 @@ abstract class LockTest extends MockedTest
*/
public function testReleaseAll()
{
$this->assertTrue($this->instance->acquireLock('foo', 1));
$this->assertTrue($this->instance->acquireLock('bar', 1));
$this->assertTrue($this->instance->acquireLock('nice', 1));
$this->assertTrue($this->instance->acquire('foo', 1));
$this->assertTrue($this->instance->acquire('bar', 1));
$this->assertTrue($this->instance->acquire('nice', 1));
$this->assertTrue($this->instance->isLocked('foo'));
$this->assertTrue($this->instance->isLocked('bar'));
@ -95,11 +95,11 @@ abstract class LockTest extends MockedTest
$this->assertFalse($this->instance->isLocked('foo'));
$this->assertFalse($this->instance->isLocked('bar'));
$this->assertFalse($this->instance->isLocked('nice'));
$this->assertTrue($this->instance->acquireLock('foo', 1));
$this->assertTrue($this->instance->acquireLock('bar', 1));
$this->assertTrue($this->instance->acquireLock('nice', 1));
$this->assertTrue($this->instance->acquire('foo', 1));
$this->assertTrue($this->instance->acquire('bar', 1));
$this->assertTrue($this->instance->acquire('nice', 1));
$this->assertTrue($this->instance->releaseLock('foo'));
$this->assertTrue($this->instance->release('foo'));
$this->assertFalse($this->instance->isLocked('foo'));
$this->assertTrue($this->instance->isLocked('bar'));
@ -117,9 +117,9 @@ abstract class LockTest extends MockedTest
public function testReleaseWitTTL()
{
$this->assertFalse($this->instance->isLocked('test'));
$this->assertTrue($this->instance->acquireLock('test', 1, 10));
$this->assertTrue($this->instance->acquire('test', 1, 10));
$this->assertTrue($this->instance->isLocked('test'));
$this->assertTrue($this->instance->releaseLock('test'));
$this->assertTrue($this->instance->release('test'));
$this->assertFalse($this->instance->isLocked('test'));
}
@ -128,9 +128,9 @@ abstract class LockTest extends MockedTest
*/
public function testGetLocks()
{
$this->assertTrue($this->instance->acquireLock('foo', 1));
$this->assertTrue($this->instance->acquireLock('bar', 1));
$this->assertTrue($this->instance->acquireLock('nice', 1));
$this->assertTrue($this->instance->acquire('foo', 1));
$this->assertTrue($this->instance->acquire('bar', 1));
$this->assertTrue($this->instance->acquire('nice', 1));
$this->assertTrue($this->instance->isLocked('foo'));
$this->assertTrue($this->instance->isLocked('bar'));
@ -148,9 +148,9 @@ abstract class LockTest extends MockedTest
*/
public function testGetLocksWithPrefix()
{
$this->assertTrue($this->instance->acquireLock('foo', 1));
$this->assertTrue($this->instance->acquireLock('test1', 1));
$this->assertTrue($this->instance->acquireLock('test2', 1));
$this->assertTrue($this->instance->acquire('foo', 1));
$this->assertTrue($this->instance->acquire('test1', 1));
$this->assertTrue($this->instance->acquire('test2', 1));
$this->assertTrue($this->instance->isLocked('foo'));
$this->assertTrue($this->instance->isLocked('test1'));
@ -174,8 +174,8 @@ abstract class LockTest extends MockedTest
$this->assertFalse($this->instance->isLocked('bar'));
// TODO [nupplaphil] - Because of the Datetime-Utils for the database, we have to wait a FULL second between the checks to invalidate the db-locks/cache
$this->assertTrue($this->instance->acquireLock('foo', 2, 1));
$this->assertTrue($this->instance->acquireLock('bar', 2, 3));
$this->assertTrue($this->instance->acquire('foo', 2, 1));
$this->assertTrue($this->instance->acquire('bar', 2, 3));
$this->assertTrue($this->instance->isLocked('foo'));
$this->assertTrue($this->instance->isLocked('bar'));
@ -197,6 +197,6 @@ abstract class LockTest extends MockedTest
public function testReleaseLockWithoutLock()
{
$this->assertFalse($this->instance->isLocked('wrongLock'));
$this->assertFalse($this->instance->releaseLock('wrongLock'));
$this->assertFalse($this->instance->release('wrongLock'));
}
}

View file

@ -55,7 +55,7 @@ class SemaphoreLockTest extends LockTest
touch($file);
$this->assertTrue(file_exists($file));
$this->assertFalse($this->instance->releaseLock('test', false));
$this->assertFalse($this->instance->release('test', false));
$this->assertTrue(file_exists($file));
}
@ -72,7 +72,7 @@ class SemaphoreLockTest extends LockTest
touch($file);
$this->assertTrue(file_exists($file));
$this->assertFalse($this->instance->releaseLock('test', true));
$this->assertFalse($this->instance->release('test', true));
$this->assertTrue(file_exists($file));
}
@ -85,8 +85,8 @@ class SemaphoreLockTest extends LockTest
touch($file);
$this->assertTrue(file_exists($file));
$this->assertTrue($this->instance->acquireLock('test'));
$this->assertTrue($this->instance->acquire('test'));
$this->assertTrue($this->instance->isLocked('test'));
$this->assertTrue($this->instance->releaseLock('test'));
$this->assertTrue($this->instance->release('test'));
}
}