From a8552a1b24e7df8c4822b5b0dd7c690312ae810d Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 15 Jul 2015 15:44:51 +0200 Subject: split off keeping track of acquire locks --- tests/lib/lock/lockingprovider.php | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) (limited to 'tests') diff --git a/tests/lib/lock/lockingprovider.php b/tests/lib/lock/lockingprovider.php index efd6e1939f2..ca72c1bb7f3 100644 --- a/tests/lib/lock/lockingprovider.php +++ b/tests/lib/lock/lockingprovider.php @@ -120,6 +120,36 @@ abstract class LockingProvider extends TestCase { $this->assertFalse($this->instance->isLocked('asd', ILockingProvider::LOCK_EXCLUSIVE)); } + public function testReleaseAllAfterChange() { + $this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED); + $this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED); + $this->instance->acquireLock('bar', ILockingProvider::LOCK_SHARED); + $this->instance->acquireLock('asd', ILockingProvider::LOCK_EXCLUSIVE); + + $this->instance->changeLock('bar', ILockingProvider::LOCK_EXCLUSIVE); + + $this->instance->releaseAll(); + + $this->assertFalse($this->instance->isLocked('foo', ILockingProvider::LOCK_SHARED)); + $this->assertFalse($this->instance->isLocked('bar', ILockingProvider::LOCK_SHARED)); + $this->assertFalse($this->instance->isLocked('bar', ILockingProvider::LOCK_EXCLUSIVE)); + $this->assertFalse($this->instance->isLocked('asd', ILockingProvider::LOCK_EXCLUSIVE)); + } + + public function testReleaseAllAfterUnlock() { + $this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED); + $this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED); + $this->instance->acquireLock('bar', ILockingProvider::LOCK_SHARED); + $this->instance->acquireLock('asd', ILockingProvider::LOCK_EXCLUSIVE); + + $this->instance->releaseLock('bar', ILockingProvider::LOCK_SHARED); + + $this->instance->releaseAll(); + + $this->assertFalse($this->instance->isLocked('foo', ILockingProvider::LOCK_SHARED)); + $this->assertFalse($this->instance->isLocked('asd', ILockingProvider::LOCK_EXCLUSIVE)); + } + public function testReleaseAfterReleaseAll() { $this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED); $this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED); -- cgit v1.2.3 From 4ea7cbb0f5d97b0ae5fe8a6c3c43718d3fa5172e Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 15 Jul 2015 16:14:32 +0200 Subject: Add database backend for high level locking --- db_structure.xml | 39 ++++++++++ lib/private/lock/dblockingprovider.php | 131 +++++++++++++++++++++++++++++++++ lib/repair/dropoldtables.php | 1 - tests/lib/lock/dblockingprovider.php | 43 +++++++++++ version.php | 2 +- 5 files changed, 214 insertions(+), 2 deletions(-) create mode 100644 lib/private/lock/dblockingprovider.php create mode 100644 tests/lib/lock/dblockingprovider.php (limited to 'tests') diff --git a/db_structure.xml b/db_structure.xml index 6d1cf6973c5..95acefcfaee 100644 --- a/db_structure.xml +++ b/db_structure.xml @@ -1180,5 +1180,44 @@ + + + + *dbprefix*locks + + + + + lock + integer + 0 + true + 4 + + + + path + text + true + 64 + + + + + true + true + lock_path_index + + path + ascending + + + + + +
+ diff --git a/lib/private/lock/dblockingprovider.php b/lib/private/lock/dblockingprovider.php new file mode 100644 index 00000000000..70f4539eb28 --- /dev/null +++ b/lib/private/lock/dblockingprovider.php @@ -0,0 +1,131 @@ + + * + * @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\IDBConnection; +use OCP\Lock\LockedException; + +class DBLockingProvider extends AbstractLockingProvider { + /** + * @var \OCP\IDBConnection + */ + private $connection; + + /** + * @param \OCP\IDBConnection $connection + */ + public function __construct(IDBConnection $connection) { + $this->connection = $connection; + } + + protected function initLockField($path) { + $this->connection->insertIfNotExist('*PREFIX*locks', ['path' => $path, 'lock' => 0], ['path']); + } + + /** + * @param string $path + * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE + * @return bool + */ + public function isLocked($path, $type) { + $query = $this->connection->prepare('SELECT `lock` from `*PREFIX*locks` WHERE `path` = ?'); + $query->execute([$path]); + $lockValue = (int)$query->fetchColumn(); + if ($type === self::LOCK_SHARED) { + return $lockValue > 0; + } else if ($type === self::LOCK_EXCLUSIVE) { + return $lockValue === -1; + } 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) { + $this->initLockField($path); + if ($type === self::LOCK_SHARED) { + $result = $this->connection->executeUpdate( + 'UPDATE `*PREFIX*locks` SET `lock` = `lock` + 1 WHERE `path` = ? AND `lock` >= 0', + [$path] + ); + } else { + $result = $this->connection->executeUpdate( + 'UPDATE `*PREFIX*locks` SET `lock` = -1 WHERE `path` = ? AND `lock` = 0', + [$path] + ); + } + if ($result !== 1) { + throw new LockedException($path); + } + $this->markAcquire($path, $type); + } + + /** + * @param string $path + * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE + */ + public function releaseLock($path, $type) { + $this->initLockField($path); + if ($type === self::LOCK_SHARED) { + $this->connection->executeUpdate( + 'UPDATE `*PREFIX*locks` SET `lock` = `lock` - 1 WHERE `path` = ? AND `lock` > 0', + [$path] + ); + } else { + $this->connection->executeUpdate( + 'UPDATE `*PREFIX*locks` SET `lock` = 0 WHERE `path` = ? AND `lock` = -1', + [$path] + ); + } + $this->markRelease($path, $type); + } + + /** + * Change the type of an existing lock + * + * @param string $path + * @param int $targetType self::LOCK_SHARED or self::LOCK_EXCLUSIVE + * @throws \OCP\Lock\LockedException + */ + public function changeLock($path, $targetType) { + $this->initLockField($path); + if ($targetType === self::LOCK_SHARED) { + $result = $this->connection->executeUpdate( + 'UPDATE `*PREFIX*locks` SET `lock` = 1 WHERE `path` = ? AND `lock` = -1', + [$path] + ); + } else { + $result = $this->connection->executeUpdate( + 'UPDATE `*PREFIX*locks` SET `lock` = -1 WHERE `path` = ? AND `lock` = 1', + [$path] + ); + } + if ($result !== 1) { + throw new LockedException($path); + } + $this->markChange($path, $targetType); + } +} diff --git a/lib/repair/dropoldtables.php b/lib/repair/dropoldtables.php index cfe0df6cb5b..89f872e16cc 100644 --- a/lib/repair/dropoldtables.php +++ b/lib/repair/dropoldtables.php @@ -76,7 +76,6 @@ class DropOldTables extends BasicEmitter implements RepairStep { 'calendar_share_event', 'foldersize', 'fscache', - 'locks', 'log', 'media_albums', 'media_artists', diff --git a/tests/lib/lock/dblockingprovider.php b/tests/lib/lock/dblockingprovider.php new file mode 100644 index 00000000000..4229ffb9c51 --- /dev/null +++ b/tests/lib/lock/dblockingprovider.php @@ -0,0 +1,43 @@ + + * + * @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; + +class DBLockingProvider extends LockingProvider { + + /** + * @var \OCP\IDBConnection + */ + private $connection; + + /** + * @return \OCP\Lock\ILockingProvider + */ + protected function getInstance() { + $this->connection = \OC::$server->getDatabaseConnection(); + return new \OC\Lock\DBLockingProvider($this->connection); + } + + public function tearDown() { + $this->connection->executeQuery('DELETE FROM `*PREFIX*locks`'); + parent::tearDown(); + } +} diff --git a/version.php b/version.php index 7ccd2e6b548..a115f4b26be 100644 --- a/version.php +++ b/version.php @@ -22,7 +22,7 @@ // We only can count up. The 4. digit is only for the internal patchlevel to trigger DB upgrades // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // when updating major/minor version number. -$OC_Version=array(8, 2, 0, 3); +$OC_Version=array(8, 2, 0, 4); // The human readable string $OC_VersionString='8.2 pre alpha'; -- cgit v1.2.3 From 96a9d171b3fc31a6ad1ed89dca987765ed1ce721 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 16 Jul 2015 12:55:28 +0200 Subject: Fix db schema --- db_structure.xml | 35 +++++++++++++++++++++++++++- lib/private/lock/abstractlockingprovider.php | 2 -- lib/private/lock/dblockingprovider.php | 16 ++++++------- lib/repair/dropoldtables.php | 1 + tests/lib/lock/dblockingprovider.php | 2 +- 5 files changed, 44 insertions(+), 12 deletions(-) (limited to 'tests') diff --git a/db_structure.xml b/db_structure.xml index 95acefcfaee..5c2b26e5f15 100644 --- a/db_structure.xml +++ b/db_structure.xml @@ -1185,10 +1185,20 @@ - *dbprefix*locks + *dbprefix*file_locks + + id + integer + 0 + true + true + 4 + 1 + + lock integer @@ -1204,9 +1214,24 @@ 64 + + ttl + integer + true + 4 + true + true + lock_id_index + + id + ascending + + + + true lock_path_index @@ -1215,6 +1240,14 @@ + + lock_ttl_index + + ttl + ascending + + + diff --git a/lib/private/lock/abstractlockingprovider.php b/lib/private/lock/abstractlockingprovider.php index 290bb77e52f..03b4912ea13 100644 --- a/lib/private/lock/abstractlockingprovider.php +++ b/lib/private/lock/abstractlockingprovider.php @@ -1,8 +1,6 @@ * @author Robin Appelman - * @author Thomas Müller * * @copyright Copyright (c) 2015, ownCloud, Inc. * @license AGPL-3.0 diff --git a/lib/private/lock/dblockingprovider.php b/lib/private/lock/dblockingprovider.php index 70f4539eb28..aee66bca745 100644 --- a/lib/private/lock/dblockingprovider.php +++ b/lib/private/lock/dblockingprovider.php @@ -38,7 +38,7 @@ class DBLockingProvider extends AbstractLockingProvider { } protected function initLockField($path) { - $this->connection->insertIfNotExist('*PREFIX*locks', ['path' => $path, 'lock' => 0], ['path']); + $this->connection->insertIfNotExist('*PREFIX*file_locks', ['path' => $path, 'lock' => 0], ['path']); } /** @@ -47,7 +47,7 @@ class DBLockingProvider extends AbstractLockingProvider { * @return bool */ public function isLocked($path, $type) { - $query = $this->connection->prepare('SELECT `lock` from `*PREFIX*locks` WHERE `path` = ?'); + $query = $this->connection->prepare('SELECT `lock` from `*PREFIX*file_locks` WHERE `path` = ?'); $query->execute([$path]); $lockValue = (int)$query->fetchColumn(); if ($type === self::LOCK_SHARED) { @@ -68,12 +68,12 @@ class DBLockingProvider extends AbstractLockingProvider { $this->initLockField($path); if ($type === self::LOCK_SHARED) { $result = $this->connection->executeUpdate( - 'UPDATE `*PREFIX*locks` SET `lock` = `lock` + 1 WHERE `path` = ? AND `lock` >= 0', + 'UPDATE `*PREFIX*file_locks` SET `lock` = `lock` + 1 WHERE `path` = ? AND `lock` >= 0', [$path] ); } else { $result = $this->connection->executeUpdate( - 'UPDATE `*PREFIX*locks` SET `lock` = -1 WHERE `path` = ? AND `lock` = 0', + 'UPDATE `*PREFIX*file_locks` SET `lock` = -1 WHERE `path` = ? AND `lock` = 0', [$path] ); } @@ -91,12 +91,12 @@ class DBLockingProvider extends AbstractLockingProvider { $this->initLockField($path); if ($type === self::LOCK_SHARED) { $this->connection->executeUpdate( - 'UPDATE `*PREFIX*locks` SET `lock` = `lock` - 1 WHERE `path` = ? AND `lock` > 0', + 'UPDATE `*PREFIX*file_locks` SET `lock` = `lock` - 1 WHERE `path` = ? AND `lock` > 0', [$path] ); } else { $this->connection->executeUpdate( - 'UPDATE `*PREFIX*locks` SET `lock` = 0 WHERE `path` = ? AND `lock` = -1', + 'UPDATE `*PREFIX*file_locks` SET `lock` = 0 WHERE `path` = ? AND `lock` = -1', [$path] ); } @@ -114,12 +114,12 @@ class DBLockingProvider extends AbstractLockingProvider { $this->initLockField($path); if ($targetType === self::LOCK_SHARED) { $result = $this->connection->executeUpdate( - 'UPDATE `*PREFIX*locks` SET `lock` = 1 WHERE `path` = ? AND `lock` = -1', + 'UPDATE `*PREFIX*file_locks` SET `lock` = 1 WHERE `path` = ? AND `lock` = -1', [$path] ); } else { $result = $this->connection->executeUpdate( - 'UPDATE `*PREFIX*locks` SET `lock` = -1 WHERE `path` = ? AND `lock` = 1', + 'UPDATE `*PREFIX*file_locks` SET `lock` = -1 WHERE `path` = ? AND `lock` = 1', [$path] ); } diff --git a/lib/repair/dropoldtables.php b/lib/repair/dropoldtables.php index 89f872e16cc..cfe0df6cb5b 100644 --- a/lib/repair/dropoldtables.php +++ b/lib/repair/dropoldtables.php @@ -76,6 +76,7 @@ class DropOldTables extends BasicEmitter implements RepairStep { 'calendar_share_event', 'foldersize', 'fscache', + 'locks', 'log', 'media_albums', 'media_artists', diff --git a/tests/lib/lock/dblockingprovider.php b/tests/lib/lock/dblockingprovider.php index 4229ffb9c51..4818ef1ceca 100644 --- a/tests/lib/lock/dblockingprovider.php +++ b/tests/lib/lock/dblockingprovider.php @@ -37,7 +37,7 @@ class DBLockingProvider extends LockingProvider { } public function tearDown() { - $this->connection->executeQuery('DELETE FROM `*PREFIX*locks`'); + $this->connection->executeQuery('DELETE FROM `*PREFIX*file_locks`'); parent::tearDown(); } } -- cgit v1.2.3 From d979e54030b0069572affe173c06680597cc831d Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 10 Aug 2015 14:39:34 +0200 Subject: log a warning while trying to acquire a db lock from within a transaction --- lib/private/lock/dblockingprovider.php | 14 +++++++++++++- lib/private/server.php | 8 ++++---- tests/lib/lock/dblockingprovider.php | 2 +- 3 files changed, 18 insertions(+), 6 deletions(-) (limited to 'tests') diff --git a/lib/private/lock/dblockingprovider.php b/lib/private/lock/dblockingprovider.php index 5480a6e53af..5241a0440b3 100644 --- a/lib/private/lock/dblockingprovider.php +++ b/lib/private/lock/dblockingprovider.php @@ -22,6 +22,7 @@ namespace OC\Lock; use OCP\IDBConnection; +use OCP\ILogger; use OCP\Lock\LockedException; /** @@ -33,11 +34,18 @@ class DBLockingProvider extends AbstractLockingProvider { */ private $connection; + /** + * @var \OCP\ILogger + */ + private $logger; + /** * @param \OCP\IDBConnection $connection + * @param \OCP\ILogger $logger */ - public function __construct(IDBConnection $connection) { + public function __construct(IDBConnection $connection, ILogger $logger) { $this->connection = $connection; + $this->logger = $logger; } protected function initLockField($path) { @@ -68,6 +76,10 @@ class DBLockingProvider extends AbstractLockingProvider { * @throws \OCP\Lock\LockedException */ public function acquireLock($path, $type) { + if ($this->connection->inTransaction()){ + $this->logger->warning('Trying to acquire a lock while inside a transition'); + } + $this->connection->beginTransaction(); $this->initLockField($path); if ($type === self::LOCK_SHARED) { diff --git a/lib/private/server.php b/lib/private/server.php index 51e70405432..be2cfc2cf88 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -432,10 +432,10 @@ class Server extends SimpleContainer implements IServerContainer { /** @var \OC\Memcache\Factory $memcacheFactory */ $memcacheFactory = $c->getMemCacheFactory(); $memcache = $memcacheFactory->createLocking('lock'); - if (!($memcache instanceof \OC\Memcache\NullCache)) { - return new MemcacheLockingProvider($memcache); - } - return new DBLockingProvider($c->getDatabaseConnection()); +// if (!($memcache instanceof \OC\Memcache\NullCache)) { +// return new MemcacheLockingProvider($memcache); +// } + return new DBLockingProvider($c->getDatabaseConnection(), $c->getLogger()); } return new NoopLockingProvider(); }); diff --git a/tests/lib/lock/dblockingprovider.php b/tests/lib/lock/dblockingprovider.php index 4818ef1ceca..fd6550d9c47 100644 --- a/tests/lib/lock/dblockingprovider.php +++ b/tests/lib/lock/dblockingprovider.php @@ -33,7 +33,7 @@ class DBLockingProvider extends LockingProvider { */ protected function getInstance() { $this->connection = \OC::$server->getDatabaseConnection(); - return new \OC\Lock\DBLockingProvider($this->connection); + return new \OC\Lock\DBLockingProvider($this->connection, \OC::$server->getLogger()); } public function tearDown() { -- cgit v1.2.3