From 29213b6136a4b2f71e5f981e9bc08e3e76128d4e Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 28 Apr 2015 15:39:38 +0200 Subject: [PATCH 1/6] extends memcache with add, inc and dec --- lib/private/memcache/apc.php | 39 ++++++++++++++++- lib/private/memcache/arraycache.php | 56 +++++++++++++++++++++++- lib/private/memcache/memcached.php | 39 ++++++++++++++++- lib/private/memcache/redis.php | 54 ++++++++++++++++++++--- lib/private/memcache/xcache.php | 66 ++++++++++++++++++++++------ lib/public/imemcache.php | 68 +++++++++++++++++++++++++++++ tests/lib/memcache/cache.php | 36 +++++++++++++++ 7 files changed, 336 insertions(+), 22 deletions(-) create mode 100644 lib/public/imemcache.php diff --git a/lib/private/memcache/apc.php b/lib/private/memcache/apc.php index b8b2b608124..ba16972e6d6 100644 --- a/lib/private/memcache/apc.php +++ b/lib/private/memcache/apc.php @@ -24,7 +24,9 @@ namespace OC\Memcache; -class APC extends Cache { +use OCP\IMemcache; + +class APC extends Cache implements IMemcache { public function get($key) { $result = apc_fetch($this->getPrefix() . $key, $success); if (!$success) { @@ -52,6 +54,41 @@ class APC extends Cache { return apc_delete($iter); } + /** + * Set a value in the cache if it's not already stored + * + * @param string $key + * @param mixed $value + * @param int $ttl Time To Live in seconds. Defaults to 60*60*24 + * @return bool + */ + public function add($key, $value, $ttl = 0) { + return apc_add($this->getPrefix() . $key, $value, $ttl); + } + + /** + * Increase a stored number + * + * @param string $key + * @param int $step + * @return int | bool + */ + public function inc($key, $step = 1) { + $this->add($key, 0); + return apc_inc($this->getPrefix() . $key, $step); + } + + /** + * Decrease a stored number + * + * @param string $key + * @param int $step + * @return int | bool + */ + public function dec($key, $step = 1) { + return apc_dec($this->getPrefix() . $key, $step); + } + static public function isAvailable() { if (!extension_loaded('apc')) { return false; diff --git a/lib/private/memcache/arraycache.php b/lib/private/memcache/arraycache.php index 939472dc518..6db920a69a8 100644 --- a/lib/private/memcache/arraycache.php +++ b/lib/private/memcache/arraycache.php @@ -22,7 +22,9 @@ namespace OC\Memcache; -class ArrayCache extends Cache { +use OCP\IMemcache; + +class ArrayCache extends Cache implements IMemcache { /** @var array Array with the cached data */ protected $cachedData = array(); @@ -76,6 +78,58 @@ class ArrayCache extends Cache { return true; } + /** + * Set a value in the cache if it's not already stored + * + * @param string $key + * @param mixed $value + * @param int $ttl Time To Live in seconds. Defaults to 60*60*24 + * @return bool + */ + public function add($key, $value, $ttl = 0) { + // since this cache is not shared race conditions aren't an issue + if ($this->hasKey($key)) { + return false; + } else { + return $this->set($key, $value, $ttl); + } + } + + /** + * Increase a stored number + * + * @param string $key + * @param int $step + * @return int | bool + */ + public function inc($key, $step = 1) { + $oldValue = $this->get($key); + if (is_int($oldValue)) { + $this->set($key, $oldValue + $step); + return $oldValue + $step; + } else { + $success = $this->add($key, $step); + return ($success) ? $step : false; + } + } + + /** + * Decrease a stored number + * + * @param string $key + * @param int $step + * @return int | bool + */ + public function dec($key, $step = 1) { + $oldValue = $this->get($key); + if (is_int($oldValue)) { + $this->set($key, $oldValue - $step); + return $oldValue - $step; + } else { + return false; + } + } + /** * {@inheritDoc} */ diff --git a/lib/private/memcache/memcached.php b/lib/private/memcache/memcached.php index a2b3440317f..9566e54c42b 100644 --- a/lib/private/memcache/memcached.php +++ b/lib/private/memcache/memcached.php @@ -24,7 +24,9 @@ namespace OC\Memcache; -class Memcached extends Cache { +use OCP\IMemcache; + +class Memcached extends Cache implements IMemcache { /** * @var \Memcached $cache */ @@ -100,6 +102,41 @@ class Memcached extends Cache { return true; } + /** + * Set a value in the cache if it's not already stored + * + * @param string $key + * @param mixed $value + * @param int $ttl Time To Live in seconds. Defaults to 60*60*24 + * @return bool + */ + public function add($key, $value, $ttl = 0) { + return self::$cache->add($this->getPrefix() . $key, $value, $ttl); + } + + /** + * Increase a stored number + * + * @param string $key + * @param int $step + * @return int | bool + */ + public function inc($key, $step = 1) { + $this->add($key, 0); + return self::$cache->increment($this->getPrefix() . $key, $step); + } + + /** + * Decrease a stored number + * + * @param string $key + * @param int $step + * @return int | bool + */ + public function dec($key, $step = 1) { + return self::$cache->decrement($this->getPrefix() . $key, $step); + } + static public function isAvailable() { return extension_loaded('memcached'); } diff --git a/lib/private/memcache/redis.php b/lib/private/memcache/redis.php index e7425726b2b..dc52c03422a 100644 --- a/lib/private/memcache/redis.php +++ b/lib/private/memcache/redis.php @@ -23,7 +23,9 @@ namespace OC\Memcache; -class Redis extends Cache { +use OCP\IMemcache; + +class Redis extends Cache implements IMemcache { /** * @var \Redis $cache @@ -52,10 +54,10 @@ class Redis extends Cache { $timeout = 0.0; // unlimited } - self::$cache->connect( $host, $port, $timeout ); + self::$cache->connect($host, $port, $timeout); if (isset($config['dbindex'])) { - self::$cache->select( $config['dbindex'] ); + self::$cache->select($config['dbindex']); } } } @@ -94,19 +96,59 @@ class Redis extends Cache { } else { return false; } - } public function clear($prefix = '') { - $prefix = $this->getNamespace() . $prefix.'*'; + $prefix = $this->getNamespace() . $prefix . '*'; $it = null; self::$cache->setOption(\Redis::OPT_SCAN, \Redis::SCAN_RETRY); - while($keys = self::$cache->scan($it, $prefix)) { + while ($keys = self::$cache->scan($it, $prefix)) { self::$cache->delete($keys); } return true; } + /** + * Set a value in the cache if it's not already stored + * + * @param string $key + * @param mixed $value + * @param int $ttl Time To Live in seconds. Defaults to 60*60*24 + * @return bool + */ + public function add($key, $value, $ttl = 0) { + // dont encode ints for inc/dec + if (!is_int($value)) { + $value = json_encode($value); + } + return self::$cache->setnx($this->getPrefix() . $key, $value); + } + + /** + * Increase a stored number + * + * @param string $key + * @param int $step + * @return int | bool + */ + public function inc($key, $step = 1) { + return self::$cache->incrBy($this->getNamespace() . $key, $step); + } + + /** + * Decrease a stored number + * + * @param string $key + * @param int $step + * @return int | bool + */ + public function dec($key, $step = 1) { + if (!$this->hasKey($key)) { + return false; + } + return self::$cache->decrBy($this->getNamespace() . $key, $step); + } + static public function isAvailable() { return extension_loaded('redis'); } diff --git a/lib/private/memcache/xcache.php b/lib/private/memcache/xcache.php index 33cea23e62b..48b0bd8a289 100644 --- a/lib/private/memcache/xcache.php +++ b/lib/private/memcache/xcache.php @@ -24,12 +24,13 @@ */ namespace OC\Memcache; +use OCP\IMemcache; /** * See http://xcache.lighttpd.net/wiki/XcacheApi for provided constants and * functions etc. */ -class XCache extends Cache { +class XCache extends Cache implements IMemcache { /** * entries in XCache gets namespaced to prevent collisions between ownCloud instances and users */ @@ -38,28 +39,28 @@ class XCache extends Cache { } public function get($key) { - return xcache_get($this->getNamespace().$key); + return xcache_get($this->getNamespace() . $key); } - public function set($key, $value, $ttl=0) { - if($ttl>0) { - return xcache_set($this->getNamespace().$key, $value, $ttl); - }else{ - return xcache_set($this->getNamespace().$key, $value); + public function set($key, $value, $ttl = 0) { + if ($ttl > 0) { + return xcache_set($this->getNamespace() . $key, $value, $ttl); + } else { + return xcache_set($this->getNamespace() . $key, $value); } } public function hasKey($key) { - return xcache_isset($this->getNamespace().$key); + return xcache_isset($this->getNamespace() . $key); } public function remove($key) { - return xcache_unset($this->getNamespace().$key); + return xcache_unset($this->getNamespace() . $key); } - public function clear($prefix='') { + public function clear($prefix = '') { if (function_exists('xcache_unset_by_prefix')) { - return xcache_unset_by_prefix($this->getNamespace().$prefix); + return xcache_unset_by_prefix($this->getNamespace() . $prefix); } else { // Since we can not clear by prefix, we just clear the whole cache. xcache_clear_cache(\XC_TYPE_VAR, 0); @@ -67,7 +68,46 @@ class XCache extends Cache { return true; } - static public function isAvailable(){ + /** + * Set a value in the cache if it's not already stored + * + * @param string $key + * @param mixed $value + * @param int $ttl Time To Live in seconds. Defaults to 60*60*24 + * @return bool + */ + public function add($key, $value, $ttl = 0) { + if ($this->hasKey($key)) { + return false; + } else { + return $this->set($key, $value, $ttl); + } + } + + /** + * Increase a stored number + * + * @param string $key + * @param int $step + * @return int | bool + */ + public function inc($key, $step = 1) { + $this->add($key, 0); + return xcache_inc($this->getPrefix() . $key, $step); + } + + /** + * Decrease a stored number + * + * @param string $key + * @param int $step + * @return int | bool + */ + public function dec($key, $step = 1) { + return xcache_dec($this->getPrefix() . $key, $step); + } + + static public function isAvailable() { if (!extension_loaded('xcache')) { return false; } @@ -80,7 +120,7 @@ class XCache extends Cache { // AND administration functions are password-protected. return false; } - $var_size = (int) ini_get('xcache.var_size'); + $var_size = (int)ini_get('xcache.var_size'); if (!$var_size) { return false; } diff --git a/lib/public/imemcache.php b/lib/public/imemcache.php new file mode 100644 index 00000000000..bc7762f80f9 --- /dev/null +++ b/lib/public/imemcache.php @@ -0,0 +1,68 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +/** + * Public interface of ownCloud for apps to use. + * Cache interface + * + */ + +// use OCP namespace for all classes that are considered public. +// This means that they should be used by apps instead of the internal ownCloud classes +namespace OCP; + +/** + * This interface defines method for accessing the file based user cache. + * + * @since 8.1.0 + */ +interface IMemcache extends ICache { + /** + * Set a value in the cache if it's not already stored + * + * @param string $key + * @param mixed $value + * @param int $ttl Time To Live in seconds. Defaults to 60*60*24 + * @return bool + * @since 8.0.0 + */ + public function add($key, $value, $ttl = 0); + + /** + * Increase a stored number + * + * @param string $key + * @param int $step + * @return int | bool + * @since 8.0.0 + */ + public function inc($key, $step = 1); + + /** + * Decrease a stored number + * + * @param string $key + * @param int $step + * @return int | bool + * @since 8.0.0 + */ + public function dec($key, $step = 1); +} diff --git a/tests/lib/memcache/cache.php b/tests/lib/memcache/cache.php index e5ceae52fb0..80ad182b6bf 100644 --- a/tests/lib/memcache/cache.php +++ b/tests/lib/memcache/cache.php @@ -10,6 +10,11 @@ namespace Test\Memcache; abstract class Cache extends \Test_Cache { + /** + * @var \OCP\IMemcache cache; + */ + protected $instance; + public function testExistsAfterSet() { $this->assertFalse($this->instance->hasKey('foo')); $this->instance->set('foo', 'bar'); @@ -56,6 +61,37 @@ abstract class Cache extends \Test_Cache { $this->assertFalse($this->instance->hasKey('foo')); } + public function testAdd() { + $this->assertTrue($this->instance->add('foo', 'bar')); + $this->assertEquals('bar', $this->instance->get('foo')); + $this->assertFalse($this->instance->add('foo', 'asd')); + $this->assertEquals('bar', $this->instance->get('foo')); + } + + public function testInc() { + $this->assertEquals(1, $this->instance->inc('foo')); + $this->assertEquals(1, $this->instance->get('foo')); + $this->assertEquals(2, $this->instance->inc('foo')); + $this->assertEquals(12, $this->instance->inc('foo', 10)); + + $this->instance->set('foo', 'bar'); + $this->assertFalse($this->instance->inc('foo')); + $this->assertEquals('bar', $this->instance->get('foo')); + } + + public function testDec() { + $this->assertEquals(false, $this->instance->dec('foo')); + $this->instance->set('foo', 20); + $this->assertEquals(19, $this->instance->dec('foo')); + $this->assertEquals(19, $this->instance->get('foo')); + $this->assertEquals(9, $this->instance->dec('foo', 10)); + + $this->instance->set('foo', 'bar'); + $this->assertFalse($this->instance->dec('foo')); + $this->assertEquals('bar', $this->instance->get('foo')); + } + + protected function tearDown() { if ($this->instance) { $this->instance->clear(); From acf30ede95f4b8986787f81edcf025a84237b7c5 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 29 Apr 2015 16:34:36 +0200 Subject: [PATCH 2/6] add compare and swap to memcache --- lib/private/memcache/apc.php | 21 +++++++++++ lib/private/memcache/arraycache.php | 16 +++++++++ lib/private/memcache/castrait.php | 56 +++++++++++++++++++++++++++++ lib/private/memcache/memcached.php | 2 ++ lib/private/memcache/redis.php | 1 + lib/private/memcache/xcache.php | 28 +++++++++++++-- lib/public/imemcache.php | 10 ++++++ tests/lib/memcache/cache.php | 12 +++++++ 8 files changed, 144 insertions(+), 2 deletions(-) create mode 100644 lib/private/memcache/castrait.php diff --git a/lib/private/memcache/apc.php b/lib/private/memcache/apc.php index ba16972e6d6..50b942e7297 100644 --- a/lib/private/memcache/apc.php +++ b/lib/private/memcache/apc.php @@ -27,6 +27,10 @@ namespace OC\Memcache; use OCP\IMemcache; class APC extends Cache implements IMemcache { + use CASTrait { + cas as casEmulated; + } + public function get($key) { $result = apc_fetch($this->getPrefix() . $key, $success); if (!$success) { @@ -89,6 +93,23 @@ class APC extends Cache implements IMemcache { return apc_dec($this->getPrefix() . $key, $step); } + /** + * Compare and set + * + * @param string $key + * @param mixed $old + * @param mixed $new + * @return bool + */ + public function cas($key, $old, $new) { + // apc only does cas for ints + if (is_int($old) and is_int($new)) { + return apc_cas($this->getPrefix() . $key, $old, $new); + } else { + return $this->casEmulated($key, $old, $new); + } + } + static public function isAvailable() { if (!extension_loaded('apc')) { return false; diff --git a/lib/private/memcache/arraycache.php b/lib/private/memcache/arraycache.php index 6db920a69a8..2b1b87a9eb3 100644 --- a/lib/private/memcache/arraycache.php +++ b/lib/private/memcache/arraycache.php @@ -130,6 +130,22 @@ class ArrayCache extends Cache implements IMemcache { } } + /** + * Compare and set + * + * @param string $key + * @param mixed $old + * @param mixed $new + * @return bool + */ + public function cas($key, $old, $new) { + if ($this->get($key) === $old) { + return $this->set($key, $new); + } else { + return false; + } + } + /** * {@inheritDoc} */ diff --git a/lib/private/memcache/castrait.php b/lib/private/memcache/castrait.php new file mode 100644 index 00000000000..c52538023fb --- /dev/null +++ b/lib/private/memcache/castrait.php @@ -0,0 +1,56 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OC\Memcache; + +trait CASTrait { + abstract public function get($key); + + abstract public function set($key, $value, $ttl = 0); + + abstract public function remove($key); + + abstract public function add($key, $value, $ttl = 0); + + /** + * Compare and set + * + * @param string $key + * @param mixed $old + * @param mixed $new + * @return bool + */ + public function cas($key, $old, $new) { + //no native cas, emulate with locking + if ($this->add($key . '_lock', true)) { + if ($this->get($key) === $old) { + $this->set($key, $new); + $this->remove($key . '_lock'); + return true; + } else { + $this->remove($key . '_lock'); + return false; + } + } else { + return false; + } + } +} diff --git a/lib/private/memcache/memcached.php b/lib/private/memcache/memcached.php index 9566e54c42b..cf1d651b551 100644 --- a/lib/private/memcache/memcached.php +++ b/lib/private/memcache/memcached.php @@ -27,6 +27,8 @@ namespace OC\Memcache; use OCP\IMemcache; class Memcached extends Cache implements IMemcache { + use CASTrait; + /** * @var \Memcached $cache */ diff --git a/lib/private/memcache/redis.php b/lib/private/memcache/redis.php index dc52c03422a..78d061404ef 100644 --- a/lib/private/memcache/redis.php +++ b/lib/private/memcache/redis.php @@ -26,6 +26,7 @@ namespace OC\Memcache; use OCP\IMemcache; class Redis extends Cache implements IMemcache { + use CASTrait; /** * @var \Redis $cache diff --git a/lib/private/memcache/xcache.php b/lib/private/memcache/xcache.php index 48b0bd8a289..3a5bd73d8ad 100644 --- a/lib/private/memcache/xcache.php +++ b/lib/private/memcache/xcache.php @@ -24,6 +24,7 @@ */ namespace OC\Memcache; + use OCP\IMemcache; /** @@ -92,7 +93,6 @@ class XCache extends Cache implements IMemcache { * @return int | bool */ public function inc($key, $step = 1) { - $this->add($key, 0); return xcache_inc($this->getPrefix() . $key, $step); } @@ -107,11 +107,35 @@ class XCache extends Cache implements IMemcache { return xcache_dec($this->getPrefix() . $key, $step); } + /** + * Compare and set + * + * @param string $key + * @param mixed $old + * @param mixed $new + * @return bool + */ + public function cas($key, $old, $new) { + //no native cas, emulate with locking + if ($this->add($key . '_lock', true)) { + if ($this->get($key) === $old) { + $this->set($key, $new); + $this->remove($key . '_lock'); + return true; + } else { + $this->remove($key . '_lock'); + return false; + } + } else { + return false; + } + } + static public function isAvailable() { if (!extension_loaded('xcache')) { return false; } - if (\OC::$CLI) { + if (\OC::$CLI && !getenv('XCACHE_TEST')) { return false; } if (!function_exists('xcache_unset_by_prefix') && ini_get('xcache.admin.enable_auth')) { diff --git a/lib/public/imemcache.php b/lib/public/imemcache.php index bc7762f80f9..56a33c9572f 100644 --- a/lib/public/imemcache.php +++ b/lib/public/imemcache.php @@ -65,4 +65,14 @@ interface IMemcache extends ICache { * @since 8.0.0 */ public function dec($key, $step = 1); + + /** + * Compare and set + * + * @param string $key + * @param mixed $old + * @param mixed $new + * @return bool + */ + public function cas($key, $old, $new); } diff --git a/tests/lib/memcache/cache.php b/tests/lib/memcache/cache.php index 80ad182b6bf..9d977cf0247 100644 --- a/tests/lib/memcache/cache.php +++ b/tests/lib/memcache/cache.php @@ -91,6 +91,18 @@ abstract class Cache extends \Test_Cache { $this->assertEquals('bar', $this->instance->get('foo')); } + public function testCasNotChanged() { + $this->instance->set('foo', 'bar'); + $this->assertTrue($this->instance->cas('foo', 'bar', 'asd')); + $this->assertEquals('asd', $this->instance->get('foo')); + } + + public function testCasChanged() { + $this->instance->set('foo', 'bar1'); + $this->assertFalse($this->instance->cas('foo', 'bar', 'asd')); + $this->assertEquals('bar1', $this->instance->get('foo')); + } + protected function tearDown() { if ($this->instance) { From 96f9573a4b0addc4b0fad6a6ac2faef8da187aff Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 29 Apr 2015 17:19:02 +0200 Subject: [PATCH 3/6] add memcache based shared/exclusive locking --- lib/private/lock/memcachelockingprovider.php | 86 ++++++++++++++ lib/public/lock/ilockingprovider.php | 47 ++++++++ lib/public/lock/lockedexception.php | 25 ++++ tests/lib/lock/lockingprovider.php | 118 +++++++++++++++++++ tests/lib/lock/memcachelockingprovider.php | 45 +++++++ 5 files changed, 321 insertions(+) create mode 100644 lib/private/lock/memcachelockingprovider.php create mode 100644 lib/public/lock/ilockingprovider.php create mode 100644 lib/public/lock/lockedexception.php create mode 100644 tests/lib/lock/lockingprovider.php create mode 100644 tests/lib/lock/memcachelockingprovider.php diff --git a/lib/private/lock/memcachelockingprovider.php b/lib/private/lock/memcachelockingprovider.php new file mode 100644 index 00000000000..43fdf70bc8b --- /dev/null +++ b/lib/private/lock/memcachelockingprovider.php @@ -0,0 +1,86 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OC\Lock; + +use OCP\Lock\ILockingProvider; +use OCP\Lock\LockedException; +use OCP\IMemcache; + +class MemcacheLockingProvider implements ILockingProvider { + /** + * @var \OCP\IMemcache + */ + private $memcache; + + /** + * @param \OCP\IMemcache $memcache + */ + public function __construct(IMemcache $memcache) { + $this->memcache = $memcache; + } + + /** + * @param string $path + * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE + * @return bool + */ + public function isLocked($path, $type) { + $lockValue = $this->memcache->get($path); + if ($type === self::LOCK_SHARED) { + return $lockValue > 0; + } else if ($type === self::LOCK_EXCLUSIVE) { + return $lockValue === 'exclusive'; + } else { + return false; + } + } + + /** + * @param string $path + * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE + * @throws \OCP\Lock\LockedException + */ + public function acquireLock($path, $type) { + if ($type === self::LOCK_SHARED) { + if (!$this->memcache->inc($path)) { + throw new LockedException($path . ' is locked'); + } + } else { + $this->memcache->add($path, 0); + if (!$this->memcache->cas($path, 0, 'exclusive')) { + throw new LockedException($path . ' is locked'); + } + } + } + + /** + * @param string $path + * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE + */ + public function releaseLock($path, $type) { + if ($type === self::LOCK_SHARED) { + $this->memcache->dec($path); + } else if ($type === self::LOCK_EXCLUSIVE) { + $this->memcache->cas($path, 'exclusive', 0); + } + } +} diff --git a/lib/public/lock/ilockingprovider.php b/lib/public/lock/ilockingprovider.php new file mode 100644 index 00000000000..a584ec02ef6 --- /dev/null +++ b/lib/public/lock/ilockingprovider.php @@ -0,0 +1,47 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OCP\Lock; + +interface ILockingProvider { + const LOCK_SHARED = 1; + const LOCK_EXCLUSIVE = 2; + + /** + * @param string $path + * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE + * @return bool + */ + public function isLocked($path, $type); + + /** + * @param string $path + * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE + * @throws \OCP\Files\Lock\LockedException + */ + public function acquireLock($path, $type); + + /** + * @param string $path + * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE + */ + public function releaseLock($path, $type); +} diff --git a/lib/public/lock/lockedexception.php b/lib/public/lock/lockedexception.php new file mode 100644 index 00000000000..4c0ca9b8c5b --- /dev/null +++ b/lib/public/lock/lockedexception.php @@ -0,0 +1,25 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OCP\Lock; + +class LockedException extends \Exception { +} diff --git a/tests/lib/lock/lockingprovider.php b/tests/lib/lock/lockingprovider.php new file mode 100644 index 00000000000..e7b8028dc94 --- /dev/null +++ b/tests/lib/lock/lockingprovider.php @@ -0,0 +1,118 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace Test\Lock; + +use OCP\Lock\ILockingProvider; +use Test\TestCase; + +abstract class LockingProvider extends TestCase { + /** + * @var \OCP\Lock\ILockingProvider + */ + protected $instance; + + /** + * @return \OCP\Lock\ILockingProvider + */ + abstract protected function getInstance(); + + protected function setUp() { + parent::setUp(); + $this->instance = $this->getInstance(); + } + + public function testExclusiveLock() { + $this->instance->acquireLock('foo', ILockingProvider::LOCK_EXCLUSIVE); + $this->assertTrue($this->instance->isLocked('foo', ILockingProvider::LOCK_EXCLUSIVE)); + $this->assertFalse($this->instance->isLocked('foo', ILockingProvider::LOCK_SHARED)); + } + + public function testSharedLock() { + $this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED); + $this->assertFalse($this->instance->isLocked('foo', ILockingProvider::LOCK_EXCLUSIVE)); + $this->assertTrue($this->instance->isLocked('foo', ILockingProvider::LOCK_SHARED)); + } + + public function testDoubleSharedLock() { + $this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED); + $this->assertFalse($this->instance->isLocked('foo', ILockingProvider::LOCK_EXCLUSIVE)); + $this->assertTrue($this->instance->isLocked('foo', ILockingProvider::LOCK_SHARED)); + $this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED); + $this->assertTrue($this->instance->isLocked('foo', ILockingProvider::LOCK_SHARED)); + } + + public function testReleaseSharedLock() { + $this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED); + $this->assertFalse($this->instance->isLocked('foo', ILockingProvider::LOCK_EXCLUSIVE)); + $this->assertTrue($this->instance->isLocked('foo', ILockingProvider::LOCK_SHARED)); + $this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED); + $this->assertTrue($this->instance->isLocked('foo', ILockingProvider::LOCK_SHARED)); + $this->instance->releaseLock('foo', ILockingProvider::LOCK_SHARED); + $this->assertTrue($this->instance->isLocked('foo', ILockingProvider::LOCK_SHARED)); + $this->instance->releaseLock('foo', ILockingProvider::LOCK_SHARED); + $this->assertFalse($this->instance->isLocked('foo', ILockingProvider::LOCK_SHARED)); + } + + /** + * @expectedException \OCP\Lock\LockedException + */ + public function testDoubleExclusiveLock() { + $this->instance->acquireLock('foo', ILockingProvider::LOCK_EXCLUSIVE); + $this->assertTrue($this->instance->isLocked('foo', ILockingProvider::LOCK_EXCLUSIVE)); + $this->instance->acquireLock('foo', ILockingProvider::LOCK_EXCLUSIVE); + } + + public function testReleaseExclusiveLock() { + $this->instance->acquireLock('foo', ILockingProvider::LOCK_EXCLUSIVE); + $this->assertTrue($this->instance->isLocked('foo', ILockingProvider::LOCK_EXCLUSIVE)); + $this->instance->releaseLock('foo', ILockingProvider::LOCK_EXCLUSIVE); + $this->assertFalse($this->instance->isLocked('foo', ILockingProvider::LOCK_EXCLUSIVE)); + $this->instance->acquireLock('foo', ILockingProvider::LOCK_EXCLUSIVE); + } + + /** + * @expectedException \OCP\Lock\LockedException + */ + public function testExclusiveLockAfterShared() { + $this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED); + $this->assertTrue($this->instance->isLocked('foo', ILockingProvider::LOCK_SHARED)); + $this->instance->acquireLock('foo', ILockingProvider::LOCK_EXCLUSIVE); + } + + public function testExclusiveLockAfterSharedReleased() { + $this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED); + $this->assertTrue($this->instance->isLocked('foo', ILockingProvider::LOCK_SHARED)); + $this->instance->releaseLock('foo', ILockingProvider::LOCK_SHARED); + $this->instance->acquireLock('foo', ILockingProvider::LOCK_EXCLUSIVE); + $this->assertTrue($this->instance->isLocked('foo', ILockingProvider::LOCK_EXCLUSIVE)); + } + + + /** + * @expectedException \OCP\Lock\LockedException + */ + public function testSharedLockAfterExclusive() { + $this->instance->acquireLock('foo', ILockingProvider::LOCK_EXCLUSIVE); + $this->assertTrue($this->instance->isLocked('foo', ILockingProvider::LOCK_EXCLUSIVE)); + $this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED); + } +} diff --git a/tests/lib/lock/memcachelockingprovider.php b/tests/lib/lock/memcachelockingprovider.php new file mode 100644 index 00000000000..40478b22939 --- /dev/null +++ b/tests/lib/lock/memcachelockingprovider.php @@ -0,0 +1,45 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace Test\Lock; + +use OC\Memcache\ArrayCache; + +class MemcacheLockingProvider extends LockingProvider { + + /** + * @var \OCP\IMemcache + */ + private $memcache; + + /** + * @return \OCP\Lock\ILockingProvider + */ + protected function getInstance() { + $this->memcache = new ArrayCache(); + return new \OC\Lock\MemcacheLockingProvider($this->memcache); + } + + public function tearDown() { + $this->memcache->clear(); + parent::tearDown(); + } +} From a40a237441670a4c575a224158b4a2a7c2fb5266 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 30 Apr 2015 13:39:31 +0200 Subject: [PATCH 4/6] use trait for cas polyfill for xcache --- lib/private/memcache/xcache.php | 26 ++------------------------ 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/lib/private/memcache/xcache.php b/lib/private/memcache/xcache.php index 3a5bd73d8ad..0be79d06ed9 100644 --- a/lib/private/memcache/xcache.php +++ b/lib/private/memcache/xcache.php @@ -32,6 +32,8 @@ use OCP\IMemcache; * functions etc. */ class XCache extends Cache implements IMemcache { + use CASTrait; + /** * entries in XCache gets namespaced to prevent collisions between ownCloud instances and users */ @@ -107,30 +109,6 @@ class XCache extends Cache implements IMemcache { return xcache_dec($this->getPrefix() . $key, $step); } - /** - * Compare and set - * - * @param string $key - * @param mixed $old - * @param mixed $new - * @return bool - */ - public function cas($key, $old, $new) { - //no native cas, emulate with locking - if ($this->add($key . '_lock', true)) { - if ($this->get($key) === $old) { - $this->set($key, $new); - $this->remove($key . '_lock'); - return true; - } else { - $this->remove($key . '_lock'); - return false; - } - } else { - return false; - } - } - static public function isAvailable() { if (!extension_loaded('xcache')) { return false; From 8119b8b0403b83970f329a8512de980580b013d6 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 30 Apr 2015 14:05:05 +0200 Subject: [PATCH 5/6] add tests for castrait --- tests/lib/memcache/castrait.php | 73 +++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 tests/lib/memcache/castrait.php diff --git a/tests/lib/memcache/castrait.php b/tests/lib/memcache/castrait.php new file mode 100644 index 00000000000..17f412bb638 --- /dev/null +++ b/tests/lib/memcache/castrait.php @@ -0,0 +1,73 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace Test\Memcache; + +use Test\TestCase; + +class CasTrait extends TestCase { + /** + * @return \OC\Memcache\CasTrait + */ + private function getCache() { + $sourceCache = new \OC\Memcache\ArrayCache(); + $mock = $this->getMockForTrait('\OC\Memcache\CasTrait'); + + $mock->expects($this->any()) + ->method('set') + ->will($this->returnCallback(function ($key, $value, $ttl) use ($sourceCache) { + return $sourceCache->set($key, $value, $ttl); + })); + + $mock->expects($this->any()) + ->method('get') + ->will($this->returnCallback(function ($key) use ($sourceCache) { + return $sourceCache->get($key); + })); + + $mock->expects($this->any()) + ->method('add') + ->will($this->returnCallback(function ($key, $value, $ttl) use ($sourceCache) { + return $sourceCache->add($key, $value, $ttl); + })); + + $mock->expects($this->any()) + ->method('remove') + ->will($this->returnCallback(function ($key) use ($sourceCache) { + return $sourceCache->remove($key); + })); + return $mock; + } + + public function testCasNotChanged() { + $cache = $this->getCache(); + $cache->set('foo', 'bar'); + $this->assertTrue($cache->cas('foo', 'bar', 'asd')); + $this->assertEquals('asd', $cache->get('foo')); + } + + public function testCasChanged() { + $cache = $this->getCache(); + $cache->set('foo', 'bar1'); + $this->assertFalse($cache->cas('foo', 'bar', 'asd')); + $this->assertEquals('bar1', $cache->get('foo')); + } +} From ba7d221cffab4b42871e5926dd6c3e0a2d2b98dc Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 30 Apr 2015 14:16:09 +0200 Subject: [PATCH 6/6] allow getting the path from the lockedexception --- lib/private/lock/memcachelockingprovider.php | 4 ++-- lib/public/lock/lockedexception.php | 21 ++++++++++++++++++++ tests/lib/lock/lockingprovider.php | 19 ++++++++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/lib/private/lock/memcachelockingprovider.php b/lib/private/lock/memcachelockingprovider.php index 43fdf70bc8b..9c8c7235462 100644 --- a/lib/private/lock/memcachelockingprovider.php +++ b/lib/private/lock/memcachelockingprovider.php @@ -62,12 +62,12 @@ class MemcacheLockingProvider implements ILockingProvider { public function acquireLock($path, $type) { if ($type === self::LOCK_SHARED) { if (!$this->memcache->inc($path)) { - throw new LockedException($path . ' is locked'); + throw new LockedException($path); } } else { $this->memcache->add($path, 0); if (!$this->memcache->cas($path, 0, 'exclusive')) { - throw new LockedException($path . ' is locked'); + throw new LockedException($path); } } } diff --git a/lib/public/lock/lockedexception.php b/lib/public/lock/lockedexception.php index 4c0ca9b8c5b..87f7164b7e0 100644 --- a/lib/public/lock/lockedexception.php +++ b/lib/public/lock/lockedexception.php @@ -22,4 +22,25 @@ namespace OCP\Lock; class LockedException extends \Exception { + /** + * @var string + */ + private $path; + + /** + * LockedException constructor. + * + * @param string $path + */ + public function __construct($path) { + parent::__construct($path . ' is locked'); + $this->path = $path; + } + + /** + * @return string + */ + public function getPath() { + return $this->path; + } } diff --git a/tests/lib/lock/lockingprovider.php b/tests/lib/lock/lockingprovider.php index e7b8028dc94..08d879da8bb 100644 --- a/tests/lib/lock/lockingprovider.php +++ b/tests/lib/lock/lockingprovider.php @@ -22,6 +22,7 @@ namespace Test\Lock; use OCP\Lock\ILockingProvider; +use OCP\Lock\LockedException; use Test\TestCase; abstract class LockingProvider extends TestCase { @@ -115,4 +116,22 @@ abstract class LockingProvider extends TestCase { $this->assertTrue($this->instance->isLocked('foo', ILockingProvider::LOCK_EXCLUSIVE)); $this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED); } + + public function testLockedExceptionHasPathForShared() { + try { + $this->testSharedLockAfterExclusive(); + $this->fail('Expected locked exception'); + } catch (LockedException $e) { + $this->assertEquals('foo', $e->getPath()); + } + } + + public function testLockedExceptionHasPathForExclusive() { + try { + $this->testExclusiveLockAfterShared(); + $this->fail('Expected locked exception'); + } catch (LockedException $e) { + $this->assertEquals('foo', $e->getPath()); + } + } }