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

@@ -4,8 +4,7 @@
  */
 namespace Friendica\Core;
 
-use Friendica\Core\Cache;
+use Friendica\Core\Cache\CacheDriverFactory;
-use Friendica\Core\Config;
 
 /**
  * @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')) {
+		$driver_name = 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);
 
-				self::$driver = new Cache\MemcacheCacheDriver($memcache_host, $memcache_port);
+		self::$driver = CacheDriverFactory::create($driver_name);
-				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();
-		}
 	}
 
 	/**

src/Core/Cache/CacheDriverFactory.php

@@ -0,0 +1,48 @@
+<?php
+
+namespace Friendica\Core\Cache;
+
+use Friendica\Core\Config;
+
+/**
+ * Class CacheDriverFactory
+ *
+ * @package Friendica\Core\Cache
+ *
+ * @brief A basic class to generate a CacheDriver
+ */
+class CacheDriverFactory
+{
+	/**
+	 * @brief 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
+	 * @throws \Exception    The exception if something went wrong during the CacheDriver creation
+	 */
+	public static function create(string $driver) : ICacheDriver {
+
+		switch ($driver) {
+			case 'memcache':
+				$memcache_host = Config::get('system', 'memcache_host', '127.0.0.1');
+				$memcache_port = Config::get('system', 'memcache_port', 11211);
+
+				return new MemcacheCacheDriver($memcache_host, $memcache_port);
+				break;
+
+			case 'memcached':
+				$memcached_hosts = Config::get('system', 'memcached_hosts', [['127.0.0.1', 11211]]);
+
+				return new 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);
+
+				return new RedisCacheDriver($redis_host, $redis_port);
+				break;
+			default:
+				return new DatabaseCacheDriver();
+		}
+	}
+}

src/Core/Lock.php

@@ -0,0 +1,145 @@
+<?php
+/**
+ * @file src/Util/Lock.php
+ */
+namespace Friendica\Core;
+
+/**
+ * @file src/Core/Lock.php
+ * @brief Functions for preventing parallel execution of functions
+ */
+
+use Friendica\Core\Cache\CacheDriverFactory;
+
+/**
+ * @brief This class contain Functions for preventing parallel execution of functions
+ */
+class Lock
+{
+	/**
+	 * @var Lock\ILockDriver;
+	 */
+	static $driver = null;
+
+	public static function init()
+	{
+		$lock_driver = Config::get('system', 'lock_driver', 'default');
+
+		try {
+			switch ($lock_driver) {
+				case 'memcache':
+					$cache_driver = CacheDriverFactory::create('memcache');
+					self::$driver = new Lock\CacheLockDriver($cache_driver);
+					break;
+
+				case 'memcached':
+					$cache_driver = CacheDriverFactory::create('memcached');
+					self::$driver = new Lock\CacheLockDriver($cache_driver);
+					break;
+
+				case 'redis':
+					$cache_driver = CacheDriverFactory::create('redis');
+					self::$driver = new Lock\CacheLockDriver($cache_driver);
+					break;
+
+				case 'database':
+					self::$driver = new Lock\DatabaseLockDriver();
+					break;
+
+				case 'semaphore':
+					self::$driver = new Lock\SemaphoreLockDriver();
+					break;
+
+				default:
+					self::useAutoDriver();
+			}
+		} catch (\Exception $exception) {
+			logger ('Driver \'' . $lock_driver . '\' failed - Fallback to \'useAutoDriver()\'');
+			self::useAutoDriver();
+		}
+	}
+
+	/**
+	 * @brief This method tries to find the best - local - locking method for Friendica
+	 *
+	 * The following sequence will be tried:
+	 * 1. Semaphore Locking
+	 * 2. Cache Locking
+	 * 3. Database Locking
+	 *
+	 */
+	private static function useAutoDriver() {
+
+		// 1. Try to use Semaphores for - local - locking
+		if (function_exists('sem_get')) {
+			try {
+				self::$driver = new Lock\SemaphoreLockDriver();
+				return;
+			} catch (\Exception $exception) {
+				logger ('Using Semaphore driver for locking failed: ' . $exception->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();
+	}
+}

src/Core/Lock/AbstractLockDriver.php

@@ -0,0 +1,57 @@
+<?php
+
+namespace Friendica\Core\Lock;
+
+/**
+ * Class AbstractLockDriver
+ *
+ * @package Friendica\Core\Lock
+ *
+ * @brief Basic class for Locking with common functions (local acquired locks, releaseAll, ..)
+ */
+abstract class AbstractLockDriver implements ILockDriver
+{
+	/**
+	 * @var array The local acquired locks
+	 */
+	protected $acquiredLocks = [];
+
+	/**
+	 * @brief 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
+	 */
+	protected function hasAcquiredLock(string $key): bool {
+		return isset($this->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);
+		}
+	}
+}

src/Core/Lock/CacheLockDriver.php

@@ -1,12 +1,26 @@
 <?php
 
-namespace Friendica\Util\Lock;
+namespace Friendica\Core\Lock;
 
-use Friendica\Core\Cache;
+use Friendica\Core\Cache\ICacheDriver;
-use dba;
 
-class MemcacheLockDriver implements ILockDriver
+class CacheLockDriver extends AbstractLockDriver
 {
+	/**
+	 * @var \Friendica\Core\Cache\ICacheDriver;
+	 */
+	private $cache;
+
+	/**
+	 * CacheLockDriver constructor.
+	 *
+	 * @param ICacheDriver $cache The CacheDriver for this type of lock
+	 */
+	public function __construct(ICacheDriver $cache)
+	{
+		$this->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
+			$lock = $this->cache->get($cachekey);
-			dba::lock('locks');
-			$lock = 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;
 	}
 }

src/Core/Lock/DatabaseLockDriver.php

@@ -1,6 +1,6 @@
 <?php
 
-namespace Friendica\Util\Lock;
+namespace Friendica\Core\Lock;
 
 use dba;
 use Friendica\Database\DBM;
@@ -8,7 +8,7 @@ use Friendica\Database\DBM;
 /**
  * Locking driver that stores the locks in the database
  */
-class DatabaseLockDriver implements ILockDriver
+class DatabaseLockDriver extends AbstractLockDriver
 {
 	/**
 	 * @brief Sets a lock for a given name
@@ -18,7 +18,7 @@ class DatabaseLockDriver 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();
@@ -42,7 +42,7 @@ class DatabaseLockDriver implements ILockDriver
 					dba::update('locks', ['locked' => 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 = [];
 	}
 }

src/Core/Lock/ILockDriver.php

@@ -1,6 +1,6 @@
 <?php
 
-namespace Friendica\Util\Lock;
+namespace Friendica\Core\Lock;
 
 /**
  * Lock Driver Interface
@@ -18,16 +18,16 @@ interface ILockDriver
 	 *
 	 * @return boolean Was the lock successful?
 	 */
-	public function acquireLock($key, $timeout = 120);
+	public function acquireLock(string $key, int $timeout = 120);
 
 	/**
 	 * @brief Releases a lock if it was set by us
 	 *
-	 * @param string $key Name of the lock
+	 * @param string $key The Name of the lock
 	 *
-	 * @return mixed
+	 * @return void
 	 */
-	public function releaseLock($key);
+	public function releaseLock(string $key);
 
 	/**
 	 * @brief Releases all lock that were set by us

src/Core/Lock/SemaphoreLockDriver.php

@@ -0,0 +1,68 @@
+<?php
+
+namespace Friendica\Core\Lock;
+
+class SemaphoreLockDriver extends AbstractLockDriver
+{
+	public function __construct()
+	{
+		if (!function_exists('sem_get')) {
+			throw new \Exception('Semaphore lock not supported');
+		}
+	}
+
+	/**
+	 * @brief Creates a semaphore key
+	 *
+	 * @param string $key Name of the lock
+	 *
+	 * @return integer the semaphore key
+	 */
+	private static function semaphoreKey(string $key): int
+	{
+		$temp = get_temppath();
+
+		$file = $temp.'/'.$key.'.sem';
+
+		if (!file_exists($file)) {
+			file_put_contents($file, $key);
+		}
+
+		return ftok($file, 'f');
+	}
+
+	/**
+	 *
+	 * @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?
+	 */
+	public function acquireLock(string $key, int $timeout = 120)
+	{
+		$this->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;
+		}
+	}
+}

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;
 

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;

src/Util/Lock.php

@@ -1,97 +0,0 @@
-<?php
-/**
- * @file src/Util/Lock.php
- */
-namespace Friendica\Util;
-
-/**
- * @file src/Util/Lock.php
- * @brief Functions for preventing parallel execution of functions
- */
-
-use Friendica\Core\Config;
-use Friendica\Util\Lock;
-
-require_once 'include/dba.php';
-
-/**
- * @brief This class contain Functions for preventing parallel execution of functions
- */
-class Lock
-{
-	/**
-	 * @var Lock\ILockDriver;
-	 */
-	static $driver = null;
-
-	public static function init()
-	{
-		switch(Config::get('system', 'lock_driver', 'default')) {
-			case 'memcache':
-				self::$driver = new Lock\MemcacheLockDriver();
-				break;
-			case 'database':
-				self::$driver = new Lock\DatabaseLockDriver();
-				break;
-			case 'semaphore':
-				self::$driver = new Lock\SemaphoreLockDriver();
-				break;
-			default:
-				// Determine what's the best/fastest locking driver (default behavior in Friendica)
-				if (function_exists('sem_get') && version_compare(PHP_VERSION, '5.6.1', '>=')) {
-					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();
-	}
-}

src/Util/Lock/SemaphoreLockDriver.php

@@ -1,82 +0,0 @@
-<?php
-
-namespace Friendica\Util\Lock;
-
-class SemaphoreLockDriver implements ILockDriver
-{
-	/**
-	 * @var array stored semaphores
-	 */
-	private static $semaphore = [];
-
-	/**
-	 * @brief Creates a semaphore key
-	 *
-	 * @param string $key Name of the lock
-	 *
-	 * @return integer the semaphore key
-	 */
-	private static function semaphoreKey($key)
-	{
-		$temp = get_temppath();
-
-		$file = $temp.'/'.$key.'.sem';
-
-		if (!file_exists($file)) {
-			file_put_contents($file, $key);
-		}
-
-		return ftok($file, 'f');
-	}
-
-	/**
-	 *
-	 * @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?
-	 */
-	public function acquireLock($key, $timeout = 120)
-	{
-		// 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[$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;
-	}
-}