From 34cea93a8be03690d8515039c61affc9f328a02b Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Thu, 5 Jul 2018 21:47:52 +0200 Subject: [PATCH] Bugfixings for Cache-Lock - used wrong cachekey in set - therefore added an abstraction to avoid wrong key concatenation - forgot to increase the db-version to 1275 --- boot.php | 2 +- src/Core/Cache/AbstractCacheDriver.php | 23 ++++++++++++++++++ src/Core/Cache/ArrayCache.php | 2 +- src/Core/Cache/MemcacheCacheDriver.php | 18 ++++++++------ src/Core/Cache/MemcachedCacheDriver.php | 20 +++++++++------- src/Core/Cache/RedisCacheDriver.php | 32 +++++++++++++++---------- src/Core/Lock/CacheLockDriver.php | 10 ++++---- 7 files changed, 72 insertions(+), 35 deletions(-) create mode 100644 src/Core/Cache/AbstractCacheDriver.php diff --git a/boot.php b/boot.php index a4588b3f7..249fd9a89 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', 1274); +define('DB_UPDATE_VERSION', 1275); define('NEW_UPDATE_ROUTINE_VERSION', 1170); /** diff --git a/src/Core/Cache/AbstractCacheDriver.php b/src/Core/Cache/AbstractCacheDriver.php new file mode 100644 index 000000000..6974dff1b --- /dev/null +++ b/src/Core/Cache/AbstractCacheDriver.php @@ -0,0 +1,23 @@ +get_hostname() . ":" . $key; + } +} \ No newline at end of file diff --git a/src/Core/Cache/ArrayCache.php b/src/Core/Cache/ArrayCache.php index 71610c39d..714021c72 100644 --- a/src/Core/Cache/ArrayCache.php +++ b/src/Core/Cache/ArrayCache.php @@ -12,7 +12,7 @@ use Friendica\Core\Cache; * * @package Friendica\Core\Cache */ -class ArrayCache implements IMemoryCacheDriver +class ArrayCache extends AbstractCacheDriver { use TraitCompareDelete; diff --git a/src/Core/Cache/MemcacheCacheDriver.php b/src/Core/Cache/MemcacheCacheDriver.php index 8eb45d907..9c16d5531 100644 --- a/src/Core/Cache/MemcacheCacheDriver.php +++ b/src/Core/Cache/MemcacheCacheDriver.php @@ -2,7 +2,6 @@ namespace Friendica\Core\Cache; -use Friendica\BaseObject; use Friendica\Core\Cache; /** @@ -10,7 +9,7 @@ use Friendica\Core\Cache; * * @author Hypolite Petovan */ -class MemcacheCacheDriver extends BaseObject implements IMemoryCacheDriver +class MemcacheCacheDriver extends AbstractCacheDriver { use TraitCompareSet; use TraitCompareDelete; @@ -39,9 +38,10 @@ class MemcacheCacheDriver extends BaseObject implements IMemoryCacheDriver public function get($key) { $return = null; + $cachekey = $this->getCacheKey($key); // We fetch with the hostname as key to avoid problems with other applications - $cached = $this->memcache->get(self::getApp()->get_hostname() . ':' . $key); + $cached = $this->memcache->get($cachekey); // @see http://php.net/manual/en/memcache.get.php#84275 if (is_bool($cached) || is_double($cached) || is_long($cached)) { @@ -65,17 +65,19 @@ class MemcacheCacheDriver extends BaseObject implements IMemoryCacheDriver */ public function set($key, $value, $ttl = Cache::FIVE_MINUTES) { + $cachekey = $this->getCacheKey($key); + // We store with the hostname as key to avoid problems with other applications if ($ttl > 0) { return $this->memcache->set( - self::getApp()->get_hostname() . ":" . $key, + $cachekey, serialize($value), MEMCACHE_COMPRESSED, time() + $ttl ); } else { return $this->memcache->set( - self::getApp()->get_hostname() . ":" . $key, + $cachekey, serialize($value), MEMCACHE_COMPRESSED ); @@ -87,7 +89,8 @@ class MemcacheCacheDriver extends BaseObject implements IMemoryCacheDriver */ public function delete($key) { - return $this->memcache->delete($key); + $cachekey = $this->getCacheKey($key); + return $this->memcache->delete($cachekey); } /** @@ -103,6 +106,7 @@ class MemcacheCacheDriver extends BaseObject implements IMemoryCacheDriver */ public function add($key, $value, $ttl = Cache::FIVE_MINUTES) { - return $this->memcache->add(self::getApp()->get_hostname() . ":" . $key, $value, $ttl); + $cachekey = $this->getCacheKey($key); + return $this->memcache->add($cachekey, $value, $ttl); } } diff --git a/src/Core/Cache/MemcachedCacheDriver.php b/src/Core/Cache/MemcachedCacheDriver.php index dda36ba19..52b6a08cf 100644 --- a/src/Core/Cache/MemcachedCacheDriver.php +++ b/src/Core/Cache/MemcachedCacheDriver.php @@ -2,7 +2,6 @@ namespace Friendica\Core\Cache; -use Friendica\BaseObject; use Friendica\Core\Cache; /** @@ -10,7 +9,7 @@ use Friendica\Core\Cache; * * @author Hypolite Petovan */ -class MemcachedCacheDriver extends BaseObject implements IMemoryCacheDriver +class MemcachedCacheDriver extends AbstractCacheDriver { use TraitCompareSet; use TraitCompareDelete; @@ -38,9 +37,10 @@ class MemcachedCacheDriver extends BaseObject implements IMemoryCacheDriver public function get($key) { $return = null; + $cachekey = $this->getCacheKey($key); // We fetch with the hostname as key to avoid problems with other applications - $value = $this->memcached->get(self::getApp()->get_hostname() . ':' . $key); + $value = $this->memcached->get($cachekey); if ($this->memcached->getResultCode() === \Memcached::RES_SUCCESS) { $return = $value; @@ -51,16 +51,18 @@ class MemcachedCacheDriver extends BaseObject implements IMemoryCacheDriver public function set($key, $value, $ttl = Cache::FIVE_MINUTES) { + $cachekey = $this->getCacheKey($key); + // We store with the hostname as key to avoid problems with other applications if ($ttl > 0) { return $this->memcached->set( - self::getApp()->get_hostname() . ':' . $key, + $cachekey, $value, time() + $ttl ); } else { return $this->memcached->set( - self::getApp()->get_hostname() . ':' . $key, + $cachekey, $value ); } @@ -69,9 +71,8 @@ class MemcachedCacheDriver extends BaseObject implements IMemoryCacheDriver public function delete($key) { - $return = $this->memcached->delete(self::getApp()->get_hostname() . ':' . $key); - - return $return; + $cachekey = $this->getCacheKey($key); + return $this->memcached->delete($cachekey); } public function clear() @@ -89,6 +90,7 @@ class MemcachedCacheDriver extends BaseObject implements IMemoryCacheDriver */ public function add($key, $value, $ttl = Cache::FIVE_MINUTES) { - return $this->memcached->add(self::getApp()->get_hostname() . ":" . $key, $value, $ttl); + $cachekey = $this->getCacheKey($key); + return $this->memcached->add($cachekey, $value, $ttl); } } diff --git a/src/Core/Cache/RedisCacheDriver.php b/src/Core/Cache/RedisCacheDriver.php index 735418b2d..8f85e67d7 100644 --- a/src/Core/Cache/RedisCacheDriver.php +++ b/src/Core/Cache/RedisCacheDriver.php @@ -2,7 +2,6 @@ namespace Friendica\Core\Cache; -use Friendica\BaseObject; use Friendica\Core\Cache; /** @@ -11,7 +10,7 @@ use Friendica\Core\Cache; * @author Hypolite Petovan * @author Roland Haeder */ -class RedisCacheDriver extends BaseObject implements IMemoryCacheDriver +class RedisCacheDriver extends AbstractCacheDriver { /** * @var \Redis @@ -34,9 +33,10 @@ class RedisCacheDriver extends BaseObject implements IMemoryCacheDriver public function get($key) { $return = null; + $cachekey = $this->getCacheKey($key); // We fetch with the hostname as key to avoid problems with other applications - $cached = $this->redis->get(self::getApp()->get_hostname() . ':' . $key); + $cached = $this->redis->get($cachekey); // @see http://php.net/manual/en/redis.get.php#84275 if (is_bool($cached) || is_double($cached) || is_long($cached)) { @@ -57,16 +57,18 @@ class RedisCacheDriver extends BaseObject implements IMemoryCacheDriver public function set($key, $value, $ttl = Cache::FIVE_MINUTES) { + $cachekey = $this->getCacheKey($key); + // We store with the hostname as key to avoid problems with other applications if ($ttl > 0) { return $this->redis->setex( - self::getApp()->get_hostname() . ":" . $key, + $cachekey, time() + $ttl, serialize($value) ); } else { return $this->redis->set( - self::getApp()->get_hostname() . ":" . $key, + $cachekey, serialize($value) ); } @@ -88,11 +90,13 @@ class RedisCacheDriver extends BaseObject implements IMemoryCacheDriver */ public function add($key, $value, $ttl = Cache::FIVE_MINUTES) { + $cachekey = $this->getCacheKey($key); + if (!is_int($value)) { $value = serialize($value); } - return $this->redis->setnx(self::getApp()->get_hostname() . ":" . $key, $value); + return $this->redis->setnx($cachekey, $value); } /** @@ -100,20 +104,22 @@ class RedisCacheDriver extends BaseObject implements IMemoryCacheDriver */ public function compareSet($key, $oldValue, $newValue, $ttl = Cache::FIVE_MINUTES) { + $cachekey = $this->getCacheKey($key); + if (!is_int($newValue)) { $newValue = serialize($newValue); } - $this->redis->watch(self::getApp()->get_hostname() . ":" . $key); + $this->redis->watch($cachekey); // If the old value isn't what we expected, somebody else changed the key meanwhile - if ($this->get($key) === $oldValue) { + if ($this->get($cachekey) === $oldValue) { if ($ttl > 0) { $result = $this->redis->multi() - ->setex(self::getApp()->get_hostname() . ":" . $ttl, $key, $newValue) + ->setex($cachekey, $ttl, $newValue) ->exec(); } else { $result = $this->redis->multi() - ->set(self::getApp()->get_hostname() . ":" . $key, $newValue) + ->set($cachekey, $newValue) ->exec(); } return $result !== false; @@ -126,11 +132,13 @@ class RedisCacheDriver extends BaseObject implements IMemoryCacheDriver */ public function compareDelete($key, $value) { - $this->redis->watch(self::getApp()->get_hostname() . ":" . $key); + $cachekey = $this->getCacheKey($key); + + $this->redis->watch($cachekey); // 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) + ->del($cachekey) ->exec(); return $result !== false; } diff --git a/src/Core/Lock/CacheLockDriver.php b/src/Core/Lock/CacheLockDriver.php index 2dd6f3fad..ccbbb8a35 100644 --- a/src/Core/Lock/CacheLockDriver.php +++ b/src/Core/Lock/CacheLockDriver.php @@ -29,7 +29,7 @@ class CacheLockDriver extends AbstractLockDriver $got_lock = false; $start = time(); - $cachekey = self::getCacheKey($key); + $cachekey = self::getLockKey($key); do { $lock = $this->cache->get($cachekey); @@ -62,7 +62,7 @@ class CacheLockDriver extends AbstractLockDriver */ public function releaseLock($key) { - $cachekey = self::getCacheKey($key); + $cachekey = self::getLockKey($key); $this->cache->compareDelete($cachekey, getmypid()); $this->markRelease($key); @@ -73,7 +73,7 @@ class CacheLockDriver extends AbstractLockDriver */ public function isLocked($key) { - $cachekey = self::getCacheKey($key); + $cachekey = self::getLockKey($key); $lock = $this->cache->get($cachekey); return isset($lock) && ($lock !== false); } @@ -82,7 +82,7 @@ class CacheLockDriver extends AbstractLockDriver * @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; + private static function getLockKey($key) { + return "lock:" . $key; } }