From cdf7f91259d5e0e261832d0edffadf3760575223 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 4 May 2015 14:02:27 +0200 Subject: [PATCH 01/35] expose locking provider in the server container --- lib/private/server.php | 18 ++++++++++++++++++ lib/public/iservercontainer.php | 8 ++++++++ 2 files changed, 26 insertions(+) diff --git a/lib/private/server.php b/lib/private/server.php index aeea4a6485..7c8e59c8aa 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -43,6 +43,7 @@ use OC\Command\AsyncBus; use OC\Diagnostics\NullQueryLogger; use OC\Diagnostics\EventLogger; use OC\Diagnostics\QueryLogger; +use OC\Lock\MemcacheLockingProvider; use OC\Mail\Mailer; use OC\Memcache\ArrayCache; use OC\Http\Client\ClientService; @@ -420,6 +421,13 @@ class Server extends SimpleContainer implements IServerContainer { $this->getLogger() ); }); + $this->registerService('LockingProvider', function (Server $c) { + /** @var \OC\Memcache\Factory $memcacheFactory */ + $memcacheFactory = $c->getMemCacheFactory(); + return new MemcacheLockingProvider( + $memcacheFactory->createDistributed('lock') + ); + }); } /** @@ -908,4 +916,14 @@ class Server extends SimpleContainer implements IServerContainer { public function getTrustedDomainHelper() { return $this->query('TrustedDomainHelper'); } + + /** + * Get the locking provider + * + * @return \OCP\Lock\ILockingProvider + * @since 8.1.0 + */ + public function getLockingProvider() { + return $this->query('LockingProvider'); + } } diff --git a/lib/public/iservercontainer.php b/lib/public/iservercontainer.php index 5dd545aed3..8f0bede6cc 100644 --- a/lib/public/iservercontainer.php +++ b/lib/public/iservercontainer.php @@ -413,4 +413,12 @@ interface IServerContainer { * @since 8.1.0 */ public function getMailer(); + + /** + * Get the locking provider + * + * @return \OCP\Lock\ILockingProvider + * @since 8.1.0 + */ + public function getLockingProvider(); } From 536e187e5125aefec75037648181afc42df2a9d0 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 4 May 2015 14:21:34 +0200 Subject: [PATCH 02/35] add locking to the storage api --- apps/files_sharing/lib/sharedstorage.php | 26 +++++++++++++++++++ lib/private/files/storage/common.php | 20 ++++++++++++++ lib/private/files/storage/storage.php | 15 +++++++++++ lib/private/files/storage/wrapper/jail.php | 20 ++++++++++++++ lib/private/files/storage/wrapper/wrapper.php | 20 ++++++++++++++ lib/public/files/storage.php | 16 ++++++++++++ 6 files changed, 117 insertions(+) diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php index ee86787c18..ad69583639 100644 --- a/apps/files_sharing/lib/sharedstorage.php +++ b/apps/files_sharing/lib/sharedstorage.php @@ -31,6 +31,9 @@ namespace OC\Files\Storage; use OC\Files\Filesystem; use OCA\Files_Sharing\ISharedStorage; +use OCA\Files_Sharing\Propagator; +use OCA\Files_Sharing\SharedMount; +use OCP\Lock\ILockingProvider; /** * Convert target path to source path and pass the function call to the correct storage provider @@ -608,4 +611,27 @@ class Shared extends \OC\Files\Storage\Common implements ISharedStorage { list($targetStorage, $targetInternalPath) = $this->resolvePath($targetInternalPath); return $targetStorage->moveFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath); } + + /** + * @param string $path + * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE + * @param \OCP\Lock\ILockingProvider $provider + * @throws \OCP\Lock\LockedException + */ + public function acquireLock($path, $type, ILockingProvider $provider) { + /** @var \OCP\Files\Storage $targetStorage */ + list($targetStorage, $targetInternalPath) = $this->resolvePath($path); + $targetStorage->acquireLock($targetInternalPath, $type, $provider); + } + + /** + * @param string $path + * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE + * @param \OCP\Lock\ILockingProvider $provider + */ + public function releaseLock($path, $type, ILockingProvider $provider) { + /** @var \OCP\Files\Storage $targetStorage */ + list($targetStorage, $targetInternalPath) = $this->resolvePath($path); + $targetStorage->releaseLock($targetInternalPath, $type, $provider); + } } diff --git a/lib/private/files/storage/common.php b/lib/private/files/storage/common.php index 1257a14dd0..045011725e 100644 --- a/lib/private/files/storage/common.php +++ b/lib/private/files/storage/common.php @@ -43,6 +43,7 @@ use OCP\Files\FileNameTooLongException; use OCP\Files\InvalidCharacterInPathException; use OCP\Files\InvalidPathException; use OCP\Files\ReservedWordException; +use OCP\Lock\ILockingProvider; /** * Storage backend class for providing common filesystem operation methods @@ -621,4 +622,23 @@ abstract class Common implements Storage { return $data; } + + /** + * @param string $path + * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE + * @param \OCP\Lock\ILockingProvider $provider + * @throws \OCP\Lock\LockedException + */ + public function acquireLock($path, $type, ILockingProvider $provider) { + $provider->acquireLock($this->getId() . '::' . $path, $type); + } + + /** + * @param string $path + * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE + * @param \OCP\Lock\ILockingProvider $provider + */ + public function releaseLock($path, $type, ILockingProvider $provider) { + $provider->releaseLock($this->getId() . '::' . $path, $type); + } } diff --git a/lib/private/files/storage/storage.php b/lib/private/files/storage/storage.php index 07b5633c90..8b34908e61 100644 --- a/lib/private/files/storage/storage.php +++ b/lib/private/files/storage/storage.php @@ -21,6 +21,7 @@ */ namespace OC\Files\Storage; +use OCP\Lock\ILockingProvider; /** * Provide a common interface to all different storage options @@ -76,4 +77,18 @@ interface Storage extends \OCP\Files\Storage { */ public function getMetaData($path); + /** + * @param string $path + * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE + * @param \OCP\Lock\ILockingProvider $provider + * @throws \OCP\Lock\LockedException + */ + public function acquireLock($path, $type, ILockingProvider $provider); + + /** + * @param string $path + * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE + * @param \OCP\Lock\ILockingProvider $provider + */ + public function releaseLock($path, $type, ILockingProvider $provider); } diff --git a/lib/private/files/storage/wrapper/jail.php b/lib/private/files/storage/wrapper/jail.php index b86b4e6405..229d12e82f 100644 --- a/lib/private/files/storage/wrapper/jail.php +++ b/lib/private/files/storage/wrapper/jail.php @@ -23,6 +23,7 @@ namespace OC\Files\Storage\Wrapper; use OC\Files\Cache\Wrapper\CacheJail; +use OCP\Lock\ILockingProvider; /** * Jail to a subdirectory of the wrapped storage @@ -424,4 +425,23 @@ class Jail extends Wrapper { public function getETag($path) { return $this->storage->getETag($this->getSourcePath($path)); } + + /** + * @param string $path + * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE + * @param \OCP\Lock\ILockingProvider $provider + * @throws \OCP\Lock\LockedException + */ + public function acquireLock($path, $type, ILockingProvider $provider) { + $this->storage->acquireLock($this->getSourcePath($path), $type, $provider); + } + + /** + * @param string $path + * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE + * @param \OCP\Lock\ILockingProvider $provider + */ + public function releaseLock($path, $type, ILockingProvider $provider) { + $this->storage->releaseLock($this->getSourcePath($path), $type, $provider); + } } diff --git a/lib/private/files/storage/wrapper/wrapper.php b/lib/private/files/storage/wrapper/wrapper.php index 14024addec..6aca771c0b 100644 --- a/lib/private/files/storage/wrapper/wrapper.php +++ b/lib/private/files/storage/wrapper/wrapper.php @@ -25,6 +25,7 @@ namespace OC\Files\Storage\Wrapper; use OCP\Files\InvalidPathException; +use OCP\Lock\ILockingProvider; class Wrapper implements \OC\Files\Storage\Storage { /** @@ -541,4 +542,23 @@ class Wrapper implements \OC\Files\Storage\Storage { public function getMetaData($path) { return $this->storage->getMetaData($path); } + + /** + * @param string $path + * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE + * @param \OCP\Lock\ILockingProvider $provider + * @throws \OCP\Lock\LockedException + */ + public function acquireLock($path, $type, ILockingProvider $provider) { + $this->storage->acquireLock($path, $type, $provider); + } + + /** + * @param string $path + * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE + * @param \OCP\Lock\ILockingProvider $provider + */ + public function releaseLock($path, $type, ILockingProvider $provider) { + $this->storage->releaseLock($path, $type, $provider); + } } diff --git a/lib/public/files/storage.php b/lib/public/files/storage.php index b89fb49a4b..ea1da57595 100644 --- a/lib/public/files/storage.php +++ b/lib/public/files/storage.php @@ -33,6 +33,7 @@ // This means that they should be used by apps instead of the internal ownCloud classes namespace OCP\Files; use OCP\Files\InvalidPathException; +use OCP\Lock\ILockingProvider; /** * Provide a common interface to all different storage options @@ -413,4 +414,19 @@ interface Storage { * @since 8.1.0 */ public function moveFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath); + + /** + * @param string $path + * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE + * @param \OCP\Lock\ILockingProvider $provider + * @throws \OCP\Lock\LockedException + */ + public function acquireLock($path, $type, ILockingProvider $provider); + + /** + * @param string $path + * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE + * @param \OCP\Lock\ILockingProvider $provider + */ + public function releaseLock($path, $type, ILockingProvider $provider); } From bf7002bc655967dd8dd1970db45e5affce47c3c3 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 4 May 2015 15:05:09 +0200 Subject: [PATCH 03/35] add locking to the view apo --- lib/private/files/view.php | 73 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 71 insertions(+), 2 deletions(-) diff --git a/lib/private/files/view.php b/lib/private/files/view.php index 63af2b616c..c82f8e1faf 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -47,6 +47,7 @@ use OCP\Files\FileNameTooLongException; use OCP\Files\InvalidCharacterInPathException; use OCP\Files\InvalidPathException; use OCP\Files\ReservedWordException; +use OCP\Lock\ILockingProvider; /** * Class to provide access to ownCloud filesystem via a "view", and methods for @@ -71,6 +72,11 @@ class View { /** @var \OC\Files\Cache\Updater */ protected $updater; + /** + * @var \OCP\Lock\ILockingProvider + */ + private $lockingProvider; + /** * @param string $root * @throws \Exception If $root contains an invalid path @@ -79,12 +85,13 @@ class View { if (is_null($root)) { throw new \InvalidArgumentException('Root can\'t be null'); } - if(!Filesystem::isValidPath($root)) { + if (!Filesystem::isValidPath($root)) { throw new \Exception(); } $this->fakeRoot = $root; $this->updater = new Updater($this); + $this->lockingProvider = \OC::$server->getLockingProvider(); } public function getAbsolutePath($path = '/') { @@ -137,7 +144,7 @@ class View { return $path; } - if (rtrim($path,'/') === rtrim($this->fakeRoot, '/')) { + if (rtrim($path, '/') === rtrim($this->fakeRoot, '/')) { return '/'; } @@ -1553,4 +1560,66 @@ class View { throw new InvalidPathException($l10n->t('File name is too long')); } } + + /** + * get all parent folders of $path + * + * @param string $path + * @return string[] + */ + private function getParents($path) { + $parts = explode('/', $path); + + // remove the singe file + array_pop($parts); + $result = array('/'); + $resultPath = ''; + foreach ($parts as $part) { + if ($part) { + $resultPath .= '/' . $part; + $result[] = $resultPath; + } + } + return $result; + } + + private function lockPath($path, $type) { + $mount = $this->getMount($path); + $mount->getStorage()->acquireLock($mount->getInternalPath($path), $type, $this->lockingProvider); + } + + private function unlockPath($path, $type) { + $mount = $this->getMount($path); + $mount->getStorage()->releaseLock($mount->getInternalPath($path), $type, $this->lockingProvider); + } + + /** + * Lock a path and all it's parents + * + * @param string $path + * @param int $type + */ + public function lockFile($path, $type) { + $this->lockPath($path, $type); + + $parents = $this->getParents($path); + foreach ($parents as $parent) { + $this->lockPath($parent, ILockingProvider::LOCK_SHARED); + } + } + + /** + * Unlock a path and all it's parents + * + * @param string $path + * @param int $type + */ + public function unlockFile($path, $type) { + $this->unlockPath($path, $type); + + $parents = $this->getParents($path); + foreach ($parents as $parent) { + $this->unlockPath($parent, ILockingProvider::LOCK_SHARED); + } + } } From e64360e72db8514b305c628a38cd30809fb2913d Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 4 May 2015 15:08:32 +0200 Subject: [PATCH 04/35] always use arraycache for unit tests --- lib/private/server.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/server.php b/lib/private/server.php index 7c8e59c8aa..900d754012 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -224,7 +224,7 @@ class Server extends SimpleContainer implements IServerContainer { $this->registerService('MemCacheFactory', function (Server $c) { $config = $c->getConfig(); - if($config->getSystemValue('installed', false)) { + if($config->getSystemValue('installed', false) && !(defined('PHPUNIT_RUN') && PHPUNIT_RUN)) { $v = \OC_App::getAppVersions(); $v['core'] = implode('.', \OC_Util::getVersion()); $version = implode(',', $v); From 7e418c7d699718b6d934de6184f6c96fdf9437d6 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 5 May 2015 13:48:49 +0200 Subject: [PATCH 05/35] high level locking wip --- lib/private/files/view.php | 64 +++++++++++++++++++++++++++++++++++--- tests/lib/files/view.php | 27 ++++++++++++++++ 2 files changed, 86 insertions(+), 5 deletions(-) diff --git a/lib/private/files/view.php b/lib/private/files/view.php index c82f8e1faf..eb6fd0bb7a 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -41,6 +41,7 @@ namespace OC\Files; +use Icewind\Streams\CallbackWrapper; use OC\Files\Cache\Updater; use OC\Files\Mount\MoveableMount; use OCP\Files\FileNameTooLongException; @@ -636,6 +637,9 @@ class View { $internalPath1 = $mount1->getInternalPath($absolutePath1); $internalPath2 = $mount2->getInternalPath($absolutePath2); + $this->lockFile($path1, ILockingProvider::LOCK_EXCLUSIVE); + $this->lockFile($path2, ILockingProvider::LOCK_EXCLUSIVE); + if ($internalPath1 === '' and $mount1 instanceof MoveableMount) { if ($this->isTargetAllowed($absolutePath2)) { /** @@ -656,6 +660,10 @@ class View { } else { $result = $storage2->moveFromStorage($storage1, $internalPath1, $internalPath2); } + + $this->unlockFile($path1, ILockingProvider::LOCK_EXCLUSIVE); + $this->unlockFile($path2, ILockingProvider::LOCK_EXCLUSIVE); + if ((Cache\Scanner::isPartialFile($path1) && !Cache\Scanner::isPartialFile($path2)) && $result !== false) { // if it was a rename from a part file to a regular file it was a write and not a rename operation $this->updater->update($path2); @@ -734,6 +742,9 @@ class View { $internalPath1 = $mount1->getInternalPath($absolutePath1); $storage2 = $mount2->getStorage(); $internalPath2 = $mount2->getInternalPath($absolutePath2); + + $this->lockFile($path2, ILockingProvider::LOCK_EXCLUSIVE); + if ($mount1->getMountPoint() == $mount2->getMountPoint()) { if ($storage1) { $result = $storage1->copy($internalPath1, $internalPath2); @@ -744,6 +755,9 @@ class View { $result = $storage2->copyFromStorage($storage1, $internalPath1, $internalPath2); } $this->updater->update($path2); + + $this->unlockFile($path2, ILockingProvider::LOCK_EXCLUSIVE); + if ($this->shouldEmitHooks() && $result !== false) { \OC_Hook::emit( Filesystem::CLASSNAME, @@ -939,10 +953,24 @@ class View { $run = $this->runHooks($hooks, $path); list($storage, $internalPath) = Filesystem::resolvePath($absolutePath . $postFix); if ($run and $storage) { - if (!is_null($extraParam)) { - $result = $storage->$operation($internalPath, $extraParam); + if (in_array('write', $hooks) || in_array('delete', $hooks)) { + $this->lockFile($path, ILockingProvider::LOCK_EXCLUSIVE); } else { - $result = $storage->$operation($internalPath); + $this->lockFile($path, ILockingProvider::LOCK_SHARED); + } + try { + if (!is_null($extraParam)) { + $result = $storage->$operation($internalPath, $extraParam); + } else { + $result = $storage->$operation($internalPath); + } + } catch (\Exception $e) { + if (in_array('write', $hooks) || in_array('delete', $hooks)) { + $this->lockFile($path, ILockingProvider::LOCK_EXCLUSIVE); + } else { + $this->lockFile($path, ILockingProvider::LOCK_SHARED); + } + throw $e; } if (in_array('delete', $hooks) and $result) { @@ -955,6 +983,23 @@ class View { $this->updater->update($path, $extraParam); } + if ($operation === 'fopen') { + $result = CallbackWrapper::wrap($result, null, null, function () use ($hooks, $path) { + if (in_array('write', $hooks)) { + $this->unlockFile($path, ILockingProvider::LOCK_EXCLUSIVE); + } else { + $this->unlockFile($path, ILockingProvider::LOCK_SHARED); + } + }); + } else { + if (in_array('write', $hooks) || in_array('delete', $hooks)) { + $this->unlockFile($path, ILockingProvider::LOCK_EXCLUSIVE); + } else { + $this->unlockFile($path, ILockingProvider::LOCK_SHARED); + } + } + + if ($this->shouldEmitHooks($path) && $result !== false) { if ($operation != 'fopen') { //no post hooks for fopen, the file stream is still open $this->runHooks($hooks, $path, true); @@ -1568,6 +1613,9 @@ class View { * @return string[] */ private function getParents($path) { + if (!$path) { + return []; + } $parts = explode('/', $path); // remove the singe file @@ -1585,12 +1633,16 @@ class View { private function lockPath($path, $type) { $mount = $this->getMount($path); - $mount->getStorage()->acquireLock($mount->getInternalPath($path), $type, $this->lockingProvider); + if ($mount) { + $mount->getStorage()->acquireLock($mount->getInternalPath($this->getAbsolutePath($path)), $type, $this->lockingProvider); + } } private function unlockPath($path, $type) { $mount = $this->getMount($path); - $mount->getStorage()->releaseLock($mount->getInternalPath($path), $type, $this->lockingProvider); + if ($mount) { + $mount->getStorage()->releaseLock($mount->getInternalPath($this->getAbsolutePath($path)), $type, $this->lockingProvider); + } } /** @@ -1600,6 +1652,7 @@ class View { * @param int $type */ public function lockFile($path, $type) { + $path = rtrim($path, '/'); $this->lockPath($path, $type); $parents = $this->getParents($path); @@ -1615,6 +1668,7 @@ class View { * @param int $type */ public function unlockFile($path, $type) { + $path = rtrim($path, '/'); $this->unlockPath($path, $type); $parents = $this->getParents($path); diff --git a/tests/lib/files/view.php b/tests/lib/files/view.php index 6bc6355713..9931074247 100644 --- a/tests/lib/files/view.php +++ b/tests/lib/files/view.php @@ -11,6 +11,7 @@ use OC\Files\Cache\Watcher; use OC\Files\Storage\Common; use OC\Files\Mount\MountPoint; use OC\Files\Storage\Temporary; +use OCP\Lock\ILockingProvider; class TemporaryNoTouch extends \OC\Files\Storage\Temporary { public function touch($path, $mtime = null) { @@ -1080,4 +1081,30 @@ class View extends \Test\TestCase { public function testNullAsRoot() { new \OC\Files\View(null); } + + /** + * e.g. reading from a folder that's being renamed + * + * @expectedException \OCP\Lock\LockedException + */ + public function testReadFromWriteLockedPath() { + $view = new \OC\Files\View(); + $storage = new Temporary(array()); + Filesystem::mount($storage, [], '/'); + $view->lockFile('/foo/bar', ILockingProvider::LOCK_EXCLUSIVE); + $view->lockFile('/foo/bar/asd', ILockingProvider::LOCK_SHARED); + } + + /** + * e.g. writing a file that's being downloaded + * + * @expectedException \OCP\Lock\LockedException + */ + public function testWriteToReadLockedFile() { + $view = new \OC\Files\View(); + $storage = new Temporary(array()); + Filesystem::mount($storage, [], '/'); + $view->lockFile('/foo/bar', ILockingProvider::LOCK_SHARED); + $view->lockFile('/foo/bar', ILockingProvider::LOCK_EXCLUSIVE); + } } From a1a25a9b5bddab05b6e8250557e8b7b6e17da1ff Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 11 May 2015 13:36:25 +0200 Subject: [PATCH 06/35] fix unlocking when moving mount points --- lib/private/files/view.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/private/files/view.php b/lib/private/files/view.php index eb6fd0bb7a..9540a2e2a9 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -664,6 +664,11 @@ class View { $this->unlockFile($path1, ILockingProvider::LOCK_EXCLUSIVE); $this->unlockFile($path2, ILockingProvider::LOCK_EXCLUSIVE); + if ($internalPath1 === '' and $mount1 instanceof MoveableMount) { + // since $path2 now points to a different storage we need to unlock the path on the old storage separately + $storage2->releaseLock($internalPath2, ILockingProvider::LOCK_EXCLUSIVE, $this->lockingProvider); + } + if ((Cache\Scanner::isPartialFile($path1) && !Cache\Scanner::isPartialFile($path2)) && $result !== false) { // if it was a rename from a part file to a regular file it was a write and not a rename operation $this->updater->update($path2); From d519aba878cca25acbe0aecc39efa9cb76bda4bb Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 11 May 2015 17:22:39 +0200 Subject: [PATCH 07/35] fix test --- tests/lib/files/view.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/lib/files/view.php b/tests/lib/files/view.php index 9931074247..06a42d6343 100644 --- a/tests/lib/files/view.php +++ b/tests/lib/files/view.php @@ -1090,7 +1090,7 @@ class View extends \Test\TestCase { public function testReadFromWriteLockedPath() { $view = new \OC\Files\View(); $storage = new Temporary(array()); - Filesystem::mount($storage, [], '/'); + \OC\Files\Filesystem::mount($storage, [], '/'); $view->lockFile('/foo/bar', ILockingProvider::LOCK_EXCLUSIVE); $view->lockFile('/foo/bar/asd', ILockingProvider::LOCK_SHARED); } @@ -1103,7 +1103,7 @@ class View extends \Test\TestCase { public function testWriteToReadLockedFile() { $view = new \OC\Files\View(); $storage = new Temporary(array()); - Filesystem::mount($storage, [], '/'); + \OC\Files\Filesystem::mount($storage, [], '/'); $view->lockFile('/foo/bar', ILockingProvider::LOCK_SHARED); $view->lockFile('/foo/bar', ILockingProvider::LOCK_EXCLUSIVE); } From 5edf294ce597c59c204c37e9fb753807ab40082f Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 13 May 2015 11:03:38 +0200 Subject: [PATCH 08/35] Add CAS methods to Null memcache This prevents breaking ownCloud completely when memcache is not enabled and the locking code is triggered --- lib/private/memcache/null.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lib/private/memcache/null.php b/lib/private/memcache/null.php index 3b8ce0b42d..c9b07f48ce 100644 --- a/lib/private/memcache/null.php +++ b/lib/private/memcache/null.php @@ -39,6 +39,18 @@ class Null extends Cache { return true; } + public function inc($key) { + return true; + } + + public function dec($key) { + return true; + } + + public function cas($key, $old, $new) { + return true; + } + public function clear($prefix = '') { return true; } From 0775e9c1ca95a3f93b1069610fd89a0801f9f897 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 13 May 2015 11:36:40 +0200 Subject: [PATCH 09/35] Use md5 for lock key --- lib/private/files/storage/common.php | 4 ++-- lib/private/files/view.php | 16 ++++++++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/lib/private/files/storage/common.php b/lib/private/files/storage/common.php index 045011725e..1c54f7cbc1 100644 --- a/lib/private/files/storage/common.php +++ b/lib/private/files/storage/common.php @@ -630,7 +630,7 @@ abstract class Common implements Storage { * @throws \OCP\Lock\LockedException */ public function acquireLock($path, $type, ILockingProvider $provider) { - $provider->acquireLock($this->getId() . '::' . $path, $type); + $provider->acquireLock(md5($this->getId() . '::' . $path), $type); } /** @@ -639,6 +639,6 @@ abstract class Common implements Storage { * @param \OCP\Lock\ILockingProvider $provider */ public function releaseLock($path, $type, ILockingProvider $provider) { - $provider->releaseLock($this->getId() . '::' . $path, $type); + $provider->releaseLock(md5($this->getId() . '::' . $path), $type); } } diff --git a/lib/private/files/view.php b/lib/private/files/view.php index 9540a2e2a9..166989ed77 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -1639,14 +1639,26 @@ class View { private function lockPath($path, $type) { $mount = $this->getMount($path); if ($mount) { - $mount->getStorage()->acquireLock($mount->getInternalPath($this->getAbsolutePath($path)), $type, $this->lockingProvider); + $mount->getStorage()->acquireLock( + $mount->getInternalPath( + $this->getAbsolutePath($path) + ), + $type, + $this->lockingProvider + ); } } private function unlockPath($path, $type) { $mount = $this->getMount($path); if ($mount) { - $mount->getStorage()->releaseLock($mount->getInternalPath($this->getAbsolutePath($path)), $type, $this->lockingProvider); + $mount->getStorage()->releaseLock( + $mount->getInternalPath( + $this->getAbsolutePath($path) + ), + $type, + $this->lockingProvider + ); } } From 8d53dc803f978f0b2e5db1c3503dc5fb0d55b0c5 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 13 May 2015 12:12:39 +0200 Subject: [PATCH 10/35] Use md5 + prefix for file locking keys in memcache Also trim slashes from paths to make sure the locks are based on the same paths. --- lib/private/files/storage/common.php | 4 ++-- lib/private/files/view.php | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/private/files/storage/common.php b/lib/private/files/storage/common.php index 1c54f7cbc1..f0c9e1bfa0 100644 --- a/lib/private/files/storage/common.php +++ b/lib/private/files/storage/common.php @@ -630,7 +630,7 @@ abstract class Common implements Storage { * @throws \OCP\Lock\LockedException */ public function acquireLock($path, $type, ILockingProvider $provider) { - $provider->acquireLock(md5($this->getId() . '::' . $path), $type); + $provider->acquireLock('files/' . md5($this->getId() . '::' . trim($path, '/')), $type); } /** @@ -639,6 +639,6 @@ abstract class Common implements Storage { * @param \OCP\Lock\ILockingProvider $provider */ public function releaseLock($path, $type, ILockingProvider $provider) { - $provider->releaseLock(md5($this->getId() . '::' . $path), $type); + $provider->releaseLock('files/' . md5($this->getId() . '::' . trim($path, '/')), $type); } } diff --git a/lib/private/files/view.php b/lib/private/files/view.php index 166989ed77..b499bbb364 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -1621,9 +1621,11 @@ class View { if (!$path) { return []; } + + $path = trim($path, '/'); $parts = explode('/', $path); - // remove the singe file + // remove the single file array_pop($parts); $result = array('/'); $resultPath = ''; From 35c377f7a9a0413bea5fa6768467c6e5f7c2d83e Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 13 May 2015 13:37:18 +0200 Subject: [PATCH 11/35] phpdoc and minor issues --- lib/private/files/storage/storage.php | 4 ++-- lib/private/files/view.php | 26 +++++++++++++++++--------- lib/public/files/storage.php | 4 ++-- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/lib/private/files/storage/storage.php b/lib/private/files/storage/storage.php index 8b34908e61..642544bad3 100644 --- a/lib/private/files/storage/storage.php +++ b/lib/private/files/storage/storage.php @@ -78,7 +78,7 @@ interface Storage extends \OCP\Files\Storage { public function getMetaData($path); /** - * @param string $path + * @param string $path The path of the file to acquire the lock for * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE * @param \OCP\Lock\ILockingProvider $provider * @throws \OCP\Lock\LockedException @@ -86,7 +86,7 @@ interface Storage extends \OCP\Files\Storage { public function acquireLock($path, $type, ILockingProvider $provider); /** - * @param string $path + * @param string $path The path of the file to release the lock for * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE * @param \OCP\Lock\ILockingProvider $provider */ diff --git a/lib/private/files/view.php b/lib/private/files/view.php index b499bbb364..b8bbd60f02 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -971,9 +971,9 @@ class View { } } catch (\Exception $e) { if (in_array('write', $hooks) || in_array('delete', $hooks)) { - $this->lockFile($path, ILockingProvider::LOCK_EXCLUSIVE); + $this->unlockFile($path, ILockingProvider::LOCK_EXCLUSIVE); } else { - $this->lockFile($path, ILockingProvider::LOCK_SHARED); + $this->unlockFile($path, ILockingProvider::LOCK_SHARED); } throw $e; } @@ -1638,6 +1638,10 @@ class View { return $result; } + /** + * @param string $path the path of the file to lock, relative to the view + * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE + */ private function lockPath($path, $type) { $mount = $this->getMount($path); if ($mount) { @@ -1651,6 +1655,10 @@ class View { } } + /** + * @param string $path the path of the file to unlock, relative to the view + * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE + */ private function unlockPath($path, $type) { $mount = $this->getMount($path); if ($mount) { @@ -1665,13 +1673,13 @@ class View { } /** - * Lock a path and all it's parents + * Lock a path and all its parents up to the root of the view * - * @param string $path - * @param int $type + * @param string $path the path of the file to lock relative to the view + * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE */ public function lockFile($path, $type) { - $path = rtrim($path, '/'); + $path = '/' . trim($path, '/'); $this->lockPath($path, $type); $parents = $this->getParents($path); @@ -1681,10 +1689,10 @@ class View { } /** - * Unlock a path and all it's parents + * Unlock a path and all its parents up to the root of the view * - * @param string $path - * @param int $type + * @param string $path the path of the file to lock relative to the view + * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE */ public function unlockFile($path, $type) { $path = rtrim($path, '/'); diff --git a/lib/public/files/storage.php b/lib/public/files/storage.php index ea1da57595..68d00fab34 100644 --- a/lib/public/files/storage.php +++ b/lib/public/files/storage.php @@ -416,7 +416,7 @@ interface Storage { public function moveFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath); /** - * @param string $path + * @param string $path The path of the file to acquire the lock for * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE * @param \OCP\Lock\ILockingProvider $provider * @throws \OCP\Lock\LockedException @@ -424,7 +424,7 @@ interface Storage { public function acquireLock($path, $type, ILockingProvider $provider); /** - * @param string $path + * @param string $path The path of the file to acquire the lock for * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE * @param \OCP\Lock\ILockingProvider $provider */ From 1270c6800dd439e411278764e3a2ddb2f68040ac Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 13 May 2015 13:54:15 +0200 Subject: [PATCH 12/35] dont lock on meta data operations --- lib/private/files/view.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/private/files/view.php b/lib/private/files/view.php index b8bbd60f02..b1333b4150 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -960,7 +960,7 @@ class View { if ($run and $storage) { if (in_array('write', $hooks) || in_array('delete', $hooks)) { $this->lockFile($path, ILockingProvider::LOCK_EXCLUSIVE); - } else { + } else if (in_array('read', $hooks)) { $this->lockFile($path, ILockingProvider::LOCK_SHARED); } try { @@ -972,7 +972,7 @@ class View { } catch (\Exception $e) { if (in_array('write', $hooks) || in_array('delete', $hooks)) { $this->unlockFile($path, ILockingProvider::LOCK_EXCLUSIVE); - } else { + } else if (in_array('read', $hooks)) { $this->unlockFile($path, ILockingProvider::LOCK_SHARED); } throw $e; @@ -992,14 +992,14 @@ class View { $result = CallbackWrapper::wrap($result, null, null, function () use ($hooks, $path) { if (in_array('write', $hooks)) { $this->unlockFile($path, ILockingProvider::LOCK_EXCLUSIVE); - } else { + } else if (in_array('read', $hooks)) { $this->unlockFile($path, ILockingProvider::LOCK_SHARED); } }); } else { if (in_array('write', $hooks) || in_array('delete', $hooks)) { $this->unlockFile($path, ILockingProvider::LOCK_EXCLUSIVE); - } else { + } else if (in_array('read', $hooks)) { $this->unlockFile($path, ILockingProvider::LOCK_SHARED); } } From 2d63fd77de1cbfdb0a3474cadf99f1dcf4962d5f Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 13 May 2015 14:07:18 +0200 Subject: [PATCH 13/35] dont apply callback wrapper when fopen failed --- lib/private/files/view.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/files/view.php b/lib/private/files/view.php index b1333b4150..6bf864c00a 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -988,7 +988,7 @@ class View { $this->updater->update($path, $extraParam); } - if ($operation === 'fopen') { + if ($operation === 'fopen' and $result) { $result = CallbackWrapper::wrap($result, null, null, function () use ($hooks, $path) { if (in_array('write', $hooks)) { $this->unlockFile($path, ILockingProvider::LOCK_EXCLUSIVE); From e08423f95631a1b06f2de0216c5765552092075c Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 19 May 2015 17:12:09 +0200 Subject: [PATCH 14/35] release all locks on shutdown --- lib/base.php | 2 ++ lib/private/lock/memcachelockingprovider.php | 27 ++++++++++++++++++++ lib/public/lock/ilockingprovider.php | 5 ++++ 3 files changed, 34 insertions(+) diff --git a/lib/base.php b/lib/base.php index b7f19c9640..09159dc22a 100644 --- a/lib/base.php +++ b/lib/base.php @@ -666,6 +666,8 @@ class OC { //make sure temporary files are cleaned up $tmpManager = \OC::$server->getTempManager(); register_shutdown_function(array($tmpManager, 'clean')); + $lockProvider = \OC::$server->getLockingProvider(); + register_shutdown_function(array($lockProvider, 'releaseAll')); if ($systemConfig->getValue('installed', false) && !self::checkUpgrade(false)) { if (\OC::$server->getConfig()->getAppValue('core', 'backgroundjobs_mode', 'ajax') == 'ajax') { diff --git a/lib/private/lock/memcachelockingprovider.php b/lib/private/lock/memcachelockingprovider.php index 9c8c723546..b2938e3a8c 100644 --- a/lib/private/lock/memcachelockingprovider.php +++ b/lib/private/lock/memcachelockingprovider.php @@ -31,6 +31,11 @@ class MemcacheLockingProvider implements ILockingProvider { */ private $memcache; + private $acquiredLocks = [ + 'shared' => [], + 'exclusive' => [] + ]; + /** * @param \OCP\IMemcache $memcache */ @@ -64,11 +69,16 @@ class MemcacheLockingProvider implements ILockingProvider { if (!$this->memcache->inc($path)) { throw new LockedException($path); } + if (!isset($this->acquiredLocks['shared'][$path])) { + $this->acquiredLocks['shared'][$path] = 0; + } + $this->acquiredLocks['shared'][$path]++; } else { $this->memcache->add($path, 0); if (!$this->memcache->cas($path, 0, 'exclusive')) { throw new LockedException($path); } + $this->acquiredLocks['exclusive'][$path] = true; } } @@ -79,8 +89,25 @@ class MemcacheLockingProvider implements ILockingProvider { public function releaseLock($path, $type) { if ($type === self::LOCK_SHARED) { $this->memcache->dec($path); + $this->acquiredLocks['shared'][$path]--; } else if ($type === self::LOCK_EXCLUSIVE) { $this->memcache->cas($path, 'exclusive', 0); + unset($this->acquiredLocks['exclusive'][$path]); + } + } + + /** + * release all lock acquired by this instance + */ + public function releaseAll() { + foreach ($this->acquiredLocks['shared'] as $path => $count) { + for ($i = 0; $i < $count; $i++) { + $this->releaseLock($path, self::LOCK_SHARED); + } + } + + foreach ($this->acquiredLocks['exclusive'] as $path => $hasLock) { + $this->releaseLock($path, self::LOCK_EXCLUSIVE); } } } diff --git a/lib/public/lock/ilockingprovider.php b/lib/public/lock/ilockingprovider.php index 0b17580faa..18fa47fa63 100644 --- a/lib/public/lock/ilockingprovider.php +++ b/lib/public/lock/ilockingprovider.php @@ -44,4 +44,9 @@ interface ILockingProvider { * @param int $type self::LOCK_SHARED or self::LOCK_EXCLUSIVE */ public function releaseLock($path, $type); + + /** + * release all lock acquired by this instance + */ + public function releaseAll(); } From b98dd3ceb84f1372415fd69bc7992d0bba954f3f Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 20 May 2015 13:08:56 +0200 Subject: [PATCH 15/35] release all locks after test --- tests/lib/testcase.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/lib/testcase.php b/tests/lib/testcase.php index c1ebfb025c..d8595c83a9 100644 --- a/tests/lib/testcase.php +++ b/tests/lib/testcase.php @@ -43,6 +43,7 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase { protected function tearDown() { $hookExceptions = \OC_Hook::$thrownExceptions; \OC_Hook::$thrownExceptions = []; + \OC::$server->getLockingProvider()->releaseAll(); if(!empty($hookExceptions)) { throw $hookExceptions[0]; } From f0b86727299728bb7243e99584a21038c54123cd Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 20 May 2015 15:22:53 +0200 Subject: [PATCH 16/35] fix locking root of a view --- lib/private/files/view.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/files/view.php b/lib/private/files/view.php index 6bf864c00a..db904e57cb 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -1618,11 +1618,11 @@ class View { * @return string[] */ private function getParents($path) { + $path = trim($path, '/'); if (!$path) { return []; } - $path = trim($path, '/'); $parts = explode('/', $path); // remove the single file From 006eaa84aa25b6301fc75cdf2fbf2594e21b4271 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 20 May 2015 16:25:41 +0200 Subject: [PATCH 17/35] dont release shared lock if we dont have any --- lib/private/lock/memcachelockingprovider.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/private/lock/memcachelockingprovider.php b/lib/private/lock/memcachelockingprovider.php index b2938e3a8c..1334d00731 100644 --- a/lib/private/lock/memcachelockingprovider.php +++ b/lib/private/lock/memcachelockingprovider.php @@ -88,8 +88,10 @@ class MemcacheLockingProvider implements ILockingProvider { */ public function releaseLock($path, $type) { if ($type === self::LOCK_SHARED) { - $this->memcache->dec($path); - $this->acquiredLocks['shared'][$path]--; + if (isset($this->acquiredLocks['shared'][$path]) and $this->acquiredLocks['shared'][$path] > 0) { + $this->memcache->dec($path); + $this->acquiredLocks['shared'][$path]--; + } } else if ($type === self::LOCK_EXCLUSIVE) { $this->memcache->cas($path, 'exclusive', 0); unset($this->acquiredLocks['exclusive'][$path]); From 6df502a5aa29b137d17bb7c17cb2552719386203 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Thu, 21 May 2015 13:06:20 +0200 Subject: [PATCH 18/35] Fix Null memcache fallback to match interface --- lib/private/memcache/null.php | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/private/memcache/null.php b/lib/private/memcache/null.php index c9b07f48ce..982f7edc80 100644 --- a/lib/private/memcache/null.php +++ b/lib/private/memcache/null.php @@ -22,7 +22,7 @@ namespace OC\Memcache; -class Null extends Cache { +class Null extends Cache implements \OCP\IMemcache { public function get($key) { return null; } @@ -39,11 +39,15 @@ class Null extends Cache { return true; } - public function inc($key) { + public function add($key, $value, $ttl = 0) { return true; } - public function dec($key) { + public function inc($key, $step = 1) { + return true; + } + + public function dec($key, $step = 1) { return true; } From c72ea9f7d72d3ab22ff56195235808b17cecb0ba Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 21 May 2015 15:57:33 +0200 Subject: [PATCH 19/35] unit test for releaseall --- tests/lib/lock/lockingprovider.php | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/lib/lock/lockingprovider.php b/tests/lib/lock/lockingprovider.php index 08d879da8b..337aa4cea7 100644 --- a/tests/lib/lock/lockingprovider.php +++ b/tests/lib/lock/lockingprovider.php @@ -107,6 +107,30 @@ abstract class LockingProvider extends TestCase { $this->assertTrue($this->instance->isLocked('foo', ILockingProvider::LOCK_EXCLUSIVE)); } + public function testReleaseAll() { + $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->releaseAll(); + + $this->assertFalse($this->instance->isLocked('foo', ILockingProvider::LOCK_SHARED)); + $this->assertFalse($this->instance->isLocked('bar', 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); + + $this->instance->releaseAll(); + + $this->assertFalse($this->instance->isLocked('foo', ILockingProvider::LOCK_SHARED)); + + $this->instance->releaseLock('foo', ILockingProvider::LOCK_SHARED); + } + /** * @expectedException \OCP\Lock\LockedException From 2f4f468399d316157979f747d2418fb5cff8d3e0 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Thu, 21 May 2015 16:11:10 +0200 Subject: [PATCH 20/35] Added config switch for file locking --- config/config.sample.php | 13 +++++ lib/private/lock/nooplockingprovider.php | 60 ++++++++++++++++++++++++ lib/private/server.php | 15 ++++-- 3 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 lib/private/lock/nooplockingprovider.php diff --git a/config/config.sample.php b/config/config.sample.php index ed86dd9413..a9fafe7b4f 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -1026,6 +1026,19 @@ $CONFIG = array( */ 'max_filesize_animated_gifs_public_sharing' => 10, + +/** + * Enables the EXPERIMENTAL file locking. + * This is disabled by default as it is experimental. + * + * Prevents concurrent processes to access the same files + * at the same time. Can help prevent side effects that would + * be caused by concurrent operations. + * + * WARNING: EXPERIMENTAL + */ +'filelocking.enabled' => false, + /** * This entry is just here to show a warning in case somebody copied the sample * configuration. DO NOT ADD THIS SWITCH TO YOUR CONFIGURATION! diff --git a/lib/private/lock/nooplockingprovider.php b/lib/private/lock/nooplockingprovider.php new file mode 100644 index 0000000000..0ca8edb4d0 --- /dev/null +++ b/lib/private/lock/nooplockingprovider.php @@ -0,0 +1,60 @@ + + * + * @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; + +/** + * Locking provider that does nothing. + * + * To be used when locking is disabled. + */ +class NoopLockingProvider implements ILockingProvider { + + /** + * {@inheritdoc} + */ + public function isLocked($path, $type) { + return false; + } + + /** + * {@inheritdoc} + */ + public function acquireLock($path, $type) { + // do nothing + } + + /** + * {@inheritdoc} + */ + public function releaseLock($path, $type) { + // do nothing + } + + /** + * release all lock acquired by this instance + */ + public function releaseAll() { + // do nothing + } +} diff --git a/lib/private/server.php b/lib/private/server.php index 900d754012..88f57e73a3 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -44,6 +44,7 @@ use OC\Diagnostics\NullQueryLogger; use OC\Diagnostics\EventLogger; use OC\Diagnostics\QueryLogger; use OC\Lock\MemcacheLockingProvider; +use OC\Lock\NoopLockingProvider; use OC\Mail\Mailer; use OC\Memcache\ArrayCache; use OC\Http\Client\ClientService; @@ -422,11 +423,15 @@ class Server extends SimpleContainer implements IServerContainer { ); }); $this->registerService('LockingProvider', function (Server $c) { - /** @var \OC\Memcache\Factory $memcacheFactory */ - $memcacheFactory = $c->getMemCacheFactory(); - return new MemcacheLockingProvider( - $memcacheFactory->createDistributed('lock') - ); + if ($c->getConfig()->getSystemValue('filelocking.enabled', false)) { + /** @var \OC\Memcache\Factory $memcacheFactory */ + $memcacheFactory = $c->getMemCacheFactory(); + $memcache = $memcacheFactory->createDistributed('lock'); + if (!($memcache instanceof \OC\Memcache\Null)) { + return new MemcacheLockingProvider($memcache); + } + } + return new NoopLockingProvider(); }); } From 668fafd4d2e56ee47ec67ba0a9768b7e5c3ccafd Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 21 May 2015 17:52:13 +0200 Subject: [PATCH 21/35] close file handle after sending sabre response --- lib/private/connector/sabre/filesplugin.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/private/connector/sabre/filesplugin.php b/lib/private/connector/sabre/filesplugin.php index 3c79f5a7a2..09d931be60 100644 --- a/lib/private/connector/sabre/filesplugin.php +++ b/lib/private/connector/sabre/filesplugin.php @@ -89,6 +89,12 @@ class FilesPlugin extends \Sabre\DAV\ServerPlugin { $this->server->on('afterBind', array($this, 'sendFileIdHeader')); $this->server->on('afterWriteContent', array($this, 'sendFileIdHeader')); $this->server->on('afterMethod:GET', [$this,'httpGet']); + $this->server->on('afterResponse', function($request, ResponseInterface $response) { + $body = $response->getBody(); + if (is_resource($body)) { + fclose($body); + } + }); } /** From 437c0b55a625ce3c2c6a81740fda8cb7a0bf1998 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 22 May 2015 13:43:44 +0200 Subject: [PATCH 22/35] unlock source file when we cant lock the target in a rename --- lib/private/files/view.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/private/files/view.php b/lib/private/files/view.php index db904e57cb..d6cc62dff2 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -49,6 +49,7 @@ use OCP\Files\InvalidCharacterInPathException; use OCP\Files\InvalidPathException; use OCP\Files\ReservedWordException; use OCP\Lock\ILockingProvider; +use OCP\Lock\LockedException; /** * Class to provide access to ownCloud filesystem via a "view", and methods for @@ -638,7 +639,12 @@ class View { $internalPath2 = $mount2->getInternalPath($absolutePath2); $this->lockFile($path1, ILockingProvider::LOCK_EXCLUSIVE); - $this->lockFile($path2, ILockingProvider::LOCK_EXCLUSIVE); + try { + $this->lockFile($path2, ILockingProvider::LOCK_EXCLUSIVE); + } catch (LockedException $e) { + $this->unlockFile($path1, ILockingProvider::LOCK_EXCLUSIVE); + throw $e; + } if ($internalPath1 === '' and $mount1 instanceof MoveableMount) { if ($this->isTargetAllowed($absolutePath2)) { From 72847dbc7794f250fb59ddf5e679d226909ec065 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 22 May 2015 13:52:42 +0200 Subject: [PATCH 23/35] always use locking in unit tests --- lib/private/server.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/server.php b/lib/private/server.php index 88f57e73a3..551012d835 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -423,7 +423,7 @@ class Server extends SimpleContainer implements IServerContainer { ); }); $this->registerService('LockingProvider', function (Server $c) { - if ($c->getConfig()->getSystemValue('filelocking.enabled', false)) { + if ($c->getConfig()->getSystemValue('filelocking.enabled', false) or (defined('PHPUNIT_RUN') && PHPUNIT_RUN)) { /** @var \OC\Memcache\Factory $memcacheFactory */ $memcacheFactory = $c->getMemCacheFactory(); $memcache = $memcacheFactory->createDistributed('lock'); From 6b965d71d170759a6c4c5d755a37f891f0166912 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 26 May 2015 14:41:37 +0200 Subject: [PATCH 24/35] add seperate config option for locking memcache backend --- config/config.sample.php | 6 ++++++ lib/private/memcache/factory.php | 23 ++++++++++++++++++++++- lib/private/server.php | 5 +++-- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/config/config.sample.php b/config/config.sample.php index a9fafe7b4f..23a27fa3ec 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -1039,6 +1039,12 @@ $CONFIG = array( */ 'filelocking.enabled' => false, +/** + * Memory caching backend for file locking + * Because most memcache backends can clean values without warning using redis is recommended + */ +'memcache.locking' => '\OC\Memcache\Redis', + /** * This entry is just here to show a warning in case somebody copied the sample * configuration. DO NOT ADD THIS SWITCH TO YOUR CONFIGURATION! diff --git a/lib/private/memcache/factory.php b/lib/private/memcache/factory.php index bbfd775c77..6e9aaa11d8 100644 --- a/lib/private/memcache/factory.php +++ b/lib/private/memcache/factory.php @@ -46,13 +46,19 @@ class Factory implements ICacheFactory { */ private $distributedCacheClass; + /** + * @var string $lockingCacheClass + */ + private $lockingCacheClass; + /** * @param string $globalPrefix * @param string|null $localCacheClass * @param string|null $distributedCacheClass + * @param string|null $lockingCacheClass */ public function __construct($globalPrefix, - $localCacheClass = null, $distributedCacheClass = null) + $localCacheClass = null, $distributedCacheClass = null, $lockingCacheClass = null) { $this->globalPrefix = $globalPrefix; @@ -62,8 +68,23 @@ class Factory implements ICacheFactory { if (!($distributedCacheClass && $distributedCacheClass::isAvailable())) { $distributedCacheClass = $localCacheClass; } + if (!($lockingCacheClass && $lockingCacheClass::isAvailable())) { + // dont fallback since the fallback might not be suitable for storing lock + $lockingCacheClass = '\OC\Memcache\Null'; + } $this->localCacheClass = $localCacheClass; $this->distributedCacheClass = $distributedCacheClass; + $this->lockingCacheClass = $lockingCacheClass; + } + + /** + * create a cache instance for storing locks + * + * @param string $prefix + * @return \OCP\IMemcache + */ + public function createLocking($prefix = '') { + return new $this->lockingCacheClass($this->globalPrefix . '/' . $prefix); } /** diff --git a/lib/private/server.php b/lib/private/server.php index 551012d835..ab1506fa13 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -234,7 +234,8 @@ class Server extends SimpleContainer implements IServerContainer { $prefix = md5($instanceId.'-'.$version.'-'.$path); return new \OC\Memcache\Factory($prefix, $config->getSystemValue('memcache.local', null), - $config->getSystemValue('memcache.distributed', null) + $config->getSystemValue('memcache.distributed', null), + $config->getSystemValue('memcache.locking', null) ); } @@ -426,7 +427,7 @@ class Server extends SimpleContainer implements IServerContainer { if ($c->getConfig()->getSystemValue('filelocking.enabled', false) or (defined('PHPUNIT_RUN') && PHPUNIT_RUN)) { /** @var \OC\Memcache\Factory $memcacheFactory */ $memcacheFactory = $c->getMemCacheFactory(); - $memcache = $memcacheFactory->createDistributed('lock'); + $memcache = $memcacheFactory->createLocking('lock'); if (!($memcache instanceof \OC\Memcache\Null)) { return new MemcacheLockingProvider($memcache); } From 72776b165f3017057f7eb56534d346357d048db7 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 26 May 2015 15:12:49 +0200 Subject: [PATCH 25/35] use arraycache for locking in unit tests --- lib/private/server.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/private/server.php b/lib/private/server.php index ab1506fa13..aea5be5afa 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -240,6 +240,7 @@ class Server extends SimpleContainer implements IServerContainer { } return new \OC\Memcache\Factory('', + new ArrayCache(), new ArrayCache(), new ArrayCache() ); From 8665a98744a8d3545859b809fc479ea216e45176 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 27 May 2015 15:19:46 +0200 Subject: [PATCH 26/35] add locking for non-chunking webdav upload --- lib/private/connector/sabre/file.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/private/connector/sabre/file.php b/lib/private/connector/sabre/file.php index 8e4460ef3b..325206f060 100644 --- a/lib/private/connector/sabre/file.php +++ b/lib/private/connector/sabre/file.php @@ -45,6 +45,7 @@ use OCP\Files\InvalidPathException; use OCP\Files\LockNotAcquiredException; use OCP\Files\NotPermittedException; use OCP\Files\StorageNotAvailableException; +use OCP\Lock\ILockingProvider; use Sabre\DAV\Exception; use Sabre\DAV\Exception\BadRequest; use Sabre\DAV\Exception\Forbidden; @@ -110,6 +111,8 @@ class File extends Node implements IFile { $partFilePath = $this->path; } + $this->fileView->lockFile($this->path, ILockingProvider::LOCK_EXCLUSIVE); + // the part file and target file might be on a different storage in case of a single file storage (e.g. single file share) /** @var \OC\Files\Storage\Storage $partStorage */ list($partStorage, $internalPartPath) = $this->fileView->resolvePath($partFilePath); @@ -232,6 +235,8 @@ class File extends Node implements IFile { throw new ServiceUnavailable("Failed to check file size: " . $e->getMessage()); } + $this->fileView->unlockFile($this->path, ILockingProvider::LOCK_EXCLUSIVE); + return '"' . $this->info->getEtag() . '"'; } From ba174ac626ae54b75565870fdeb55398436599e4 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 29 May 2015 09:59:20 +0200 Subject: [PATCH 27/35] Convert LockedException to FileLocked in Sabre connector For Sabre to be able to return the proper error code instead of 500, the LockedException is now rethrown as FileLocked exception in the Sabre connector --- lib/private/connector/sabre/file.php | 14 +++++++++++++- lib/private/connector/sabre/objecttree.php | 8 ++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/lib/private/connector/sabre/file.php b/lib/private/connector/sabre/file.php index 325206f060..3e1b29a4f2 100644 --- a/lib/private/connector/sabre/file.php +++ b/lib/private/connector/sabre/file.php @@ -46,6 +46,7 @@ use OCP\Files\LockNotAcquiredException; use OCP\Files\NotPermittedException; use OCP\Files\StorageNotAvailableException; use OCP\Lock\ILockingProvider; +use OCP\Lock\LockedException; use Sabre\DAV\Exception; use Sabre\DAV\Exception\BadRequest; use Sabre\DAV\Exception\Forbidden; @@ -80,6 +81,7 @@ class File extends Node implements IFile { * @throws Exception * @throws EntityTooLarge * @throws ServiceUnavailable + * @throws FileLocked * @return string|null */ public function put($data) { @@ -111,7 +113,11 @@ class File extends Node implements IFile { $partFilePath = $this->path; } - $this->fileView->lockFile($this->path, ILockingProvider::LOCK_EXCLUSIVE); + try { + $this->fileView->lockFile($this->path, ILockingProvider::LOCK_EXCLUSIVE); + } catch (LockedException $e) { + throw new FileLocked($e->getMessage(), $e->getCode(), $e); + } // the part file and target file might be on a different storage in case of a single file storage (e.g. single file share) /** @var \OC\Files\Storage\Storage $partStorage */ @@ -257,6 +263,8 @@ class File extends Node implements IFile { throw new ServiceUnavailable("Encryption not ready: " . $e->getMessage()); } catch (StorageNotAvailableException $e) { throw new ServiceUnavailable("Failed to open file: " . $e->getMessage()); + } catch (LockedException $e) { + throw new FileLocked($e->getMessage(), $e->getCode(), $e); } } @@ -278,6 +286,8 @@ class File extends Node implements IFile { } } catch (StorageNotAvailableException $e) { throw new ServiceUnavailable("Failed to unlink: " . $e->getMessage()); + } catch (LockedException $e) { + throw new FileLocked($e->getMessage(), $e->getCode(), $e); } } @@ -383,6 +393,8 @@ class File extends Node implements IFile { return $info->getEtag(); } catch (StorageNotAvailableException $e) { throw new ServiceUnavailable("Failed to put file: " . $e->getMessage()); + } catch (LockedException $e) { + throw new FileLocked($e->getMessage(), $e->getCode(), $e); } } diff --git a/lib/private/connector/sabre/objecttree.php b/lib/private/connector/sabre/objecttree.php index 17d9aff8f6..ed42a31336 100644 --- a/lib/private/connector/sabre/objecttree.php +++ b/lib/private/connector/sabre/objecttree.php @@ -26,10 +26,12 @@ namespace OC\Connector\Sabre; use OC\Connector\Sabre\Exception\InvalidPath; +use OC\Connector\Sabre\Exception\FileLocked; use OC\Files\FileInfo; use OC\Files\Mount\MoveableMount; use OCP\Files\StorageInvalidException; use OCP\Files\StorageNotAvailableException; +use OCP\Lock\LockedException; class ObjectTree extends \Sabre\DAV\Tree { @@ -221,8 +223,10 @@ class ObjectTree extends \Sabre\DAV\Tree { if (!$renameOkay) { throw new \Sabre\DAV\Exception\Forbidden(''); } - } catch (\OCP\Files\StorageNotAvailableException $e) { + } catch (StorageNotAvailableException $e) { throw new \Sabre\DAV\Exception\ServiceUnavailable($e->getMessage()); + } catch (LockedException $e) { + throw new FileLocked($e->getMessage(), $e->getCode(), $e); } $this->markDirty($sourceDir); @@ -258,7 +262,7 @@ class ObjectTree extends \Sabre\DAV\Tree { try { $this->fileView->copy($source, $destination); - } catch (\OCP\Files\StorageNotAvailableException $e) { + } catch (StorageNotAvailableException $e) { throw new \Sabre\DAV\Exception\ServiceUnavailable($e->getMessage()); } From 0451a6652d7dcac3887bdf760593482eaa19e681 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 29 May 2015 10:55:25 +0200 Subject: [PATCH 28/35] Move locking exceptions --- lib/private/connector/sabre/directory.php | 17 +++++++++++++---- lib/private/connector/sabre/objecttree.php | 2 ++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/lib/private/connector/sabre/directory.php b/lib/private/connector/sabre/directory.php index ef35b300ea..82e1b55d76 100644 --- a/lib/private/connector/sabre/directory.php +++ b/lib/private/connector/sabre/directory.php @@ -28,6 +28,8 @@ namespace OC\Connector\Sabre; use OC\Connector\Sabre\Exception\InvalidPath; +use OC\Connector\Sabre\Exception\FileLocked; +use OCP\Lock\LockedException; class Directory extends \OC\Connector\Sabre\Node implements \Sabre\DAV\ICollection, \Sabre\DAV\IQuota { @@ -102,6 +104,8 @@ class Directory extends \OC\Connector\Sabre\Node return $node->put($data); } catch (\OCP\Files\StorageNotAvailableException $e) { throw new \Sabre\DAV\Exception\ServiceUnavailable($e->getMessage()); + } catch (LockedException $e) { + throw new FileLocked($e->getMessage(), $e->getCode(), $e); } } @@ -127,6 +131,8 @@ class Directory extends \OC\Connector\Sabre\Node throw new \Sabre\DAV\Exception\ServiceUnavailable($e->getMessage()); } catch (\OCP\Files\InvalidPathException $ex) { throw new InvalidPath($ex->getMessage()); + } catch (LockedException $e) { + throw new FileLocked($e->getMessage(), $e->getCode(), $e); } } @@ -211,11 +217,14 @@ class Directory extends \OC\Connector\Sabre\Node throw new \Sabre\DAV\Exception\Forbidden(); } - if (!$this->fileView->rmdir($this->path)) { - // assume it wasn't possible to remove due to permission issue - throw new \Sabre\DAV\Exception\Forbidden(); + try { + if (!$this->fileView->rmdir($this->path)) { + // assume it wasn't possible to remove due to permission issue + throw new \Sabre\DAV\Exception\Forbidden(); + } + } catch (LockedException $e) { + throw new FileLocked($e->getMessage(), $e->getCode(), $e); } - } /** diff --git a/lib/private/connector/sabre/objecttree.php b/lib/private/connector/sabre/objecttree.php index ed42a31336..c56fd7ee4d 100644 --- a/lib/private/connector/sabre/objecttree.php +++ b/lib/private/connector/sabre/objecttree.php @@ -264,6 +264,8 @@ class ObjectTree extends \Sabre\DAV\Tree { $this->fileView->copy($source, $destination); } catch (StorageNotAvailableException $e) { throw new \Sabre\DAV\Exception\ServiceUnavailable($e->getMessage()); + } catch (LockedException $e) { + throw new FileLocked($e->getMessage(), $e->getCode(), $e); } list($destinationDir,) = \Sabre\HTTP\URLUtil::splitPath($destination); From 270a10b7545ba8fdf8d8736ae6b967929875cfdd Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 29 May 2015 11:24:06 +0200 Subject: [PATCH 29/35] Return 423 instead of 503 for locked files --- lib/private/connector/sabre/exception/filelocked.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/connector/sabre/exception/filelocked.php b/lib/private/connector/sabre/exception/filelocked.php index 2405059bfb..1657a7ae37 100644 --- a/lib/private/connector/sabre/exception/filelocked.php +++ b/lib/private/connector/sabre/exception/filelocked.php @@ -42,6 +42,6 @@ class FileLocked extends \Sabre\DAV\Exception { */ public function getHTTPCode() { - return 503; + return 423; } } From 43772e2a9a4b1400e7e3a8874130ac817aa64058 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Fri, 29 May 2015 11:54:05 +0200 Subject: [PATCH 30/35] Adding information on file locking status to admin section --- settings/admin.php | 8 ++++++++ settings/templates/admin.php | 37 ++++++++++++++++++++++++------------ 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/settings/admin.php b/settings/admin.php index 5720bd9f99..f2e01adab1 100644 --- a/settings/admin.php +++ b/settings/admin.php @@ -29,6 +29,8 @@ * */ +use OC\Lock\NoopLockingProvider; + OC_Util::checkAdminUser(); OC_App::setActiveNavigationEntry("admin"); @@ -175,6 +177,11 @@ $template->assign('fileSharingSettings', $fileSharingSettings); $template->assign('filesExternal', $filesExternal); $template->assign('updaterAppPanel', $updaterAppPanel); $template->assign('ocDefaultEncryptionModulePanel', $ocDefaultEncryptionModulePanel); +if (\OC::$server->getLockingProvider() instanceof NoopLockingProvider) { + $template->assign('fileLockingEnabled', false); +} else { + $template->assign('fileLockingEnabled', true); +} $formsMap = array_map(function ($form) { if (preg_match('%(]*>.*?)%i', $form, $regs)) { @@ -200,6 +207,7 @@ $formsAndMore = array_merge($formsAndMore, $formsMap); $formsAndMore[] = ['anchor' => 'backgroundjobs', 'section-name' => $l->t('Cron')]; $formsAndMore[] = ['anchor' => 'mail_general_settings', 'section-name' => $l->t('Email server')]; $formsAndMore[] = ['anchor' => 'log-section', 'section-name' => $l->t('Log')]; +$formsAndMore[] = ['anchor' => 'server-status', 'section-name' => $l->t('Server Status')]; $formsAndMore[] = ['anchor' => 'admin-tips', 'section-name' => $l->t('Tips & tricks')]; if ($updaterAppPanel) { $formsAndMore[] = ['anchor' => 'updater', 'section-name' => $l->t('Updates')]; diff --git a/settings/templates/admin.php b/settings/templates/admin.php index f9a99b589a..3d253d4cbb 100644 --- a/settings/templates/admin.php +++ b/settings/templates/admin.php @@ -7,6 +7,7 @@ /** * @var array $_ * @var \OCP\IL10N $l + * @var OC_Defaults $theme */ style('settings', 'settings'); @@ -15,32 +16,32 @@ script('core', ['multiselect', 'setupchecks']); vendor_script('select2/select2'); vendor_style('select2/select2'); -$levels = array('Debug', 'Info', 'Warning', 'Error', 'Fatal'); -$levelLabels = array( +$levels = ['Debug', 'Info', 'Warning', 'Error', 'Fatal']; +$levelLabels = [ $l->t( 'Everything (fatal issues, errors, warnings, info, debug)' ), $l->t( 'Info, warnings, errors and fatal issues' ), $l->t( 'Warnings, errors and fatal issues' ), $l->t( 'Errors and fatal issues' ), $l->t( 'Fatal issues only' ), -); +]; -$mail_smtpauthtype = array( +$mail_smtpauthtype = [ '' => $l->t('None'), 'LOGIN' => $l->t('Login'), 'PLAIN' => $l->t('Plain'), 'NTLM' => $l->t('NT LAN Manager'), -); +]; -$mail_smtpsecure = array( +$mail_smtpsecure = [ '' => $l->t('None'), 'ssl' => $l->t('SSL'), 'tls' => $l->t('TLS'), -); +]; -$mail_smtpmode = array( +$mail_smtpmode = [ 'php', 'smtp', -); +]; if ($_['sendmail_is_available']) { $mail_smtpmode[] = 'sendmail'; } @@ -137,7 +138,7 @@ if (!$_['isLocaleWorking']) { ?>
t('We strongly suggest installing the required packages on your system to support one of the following locales: %s.', array($locales))); + p($l->t('We strongly suggest installing the required packages on your system to support one of the following locales: %s.', [$locales])); ?> - t("Last cron job execution: %s.", array($relative_time)));?> + t("Last cron job execution: %s.", [$relative_time]));?> - t("Last cron job execution: %s. Something seems wrong.", array($relative_time)));?> + t("Last cron job execution: %s. Something seems wrong.", [$relative_time]));?> @@ -527,6 +528,18 @@ if ($_['cronErrors']) {
  • t('Hardening and security guidance'));?> ↗
    • +
    +

    t('Server Status'));?>

    +
      +
    • + t('Experimental File Lock is enabled.')); + } else { + p($l->t('Experimental File Lock is disabled.')); + } ?> +
      • +
    +

    t('Version'));?>

    From a1372b2fb5acc51eacf70012a6702a96c78e61ff Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 29 May 2015 14:34:21 +0200 Subject: [PATCH 31/35] add method to atomically change between shared and exclusive lock --- lib/private/lock/memcachelockingprovider.php | 20 ++++++++ lib/public/lock/ilockingprovider.php | 9 ++++ tests/lib/lock/lockingprovider.php | 53 ++++++++++++++++++++ 3 files changed, 82 insertions(+) diff --git a/lib/private/lock/memcachelockingprovider.php b/lib/private/lock/memcachelockingprovider.php index 1334d00731..368ff45032 100644 --- a/lib/private/lock/memcachelockingprovider.php +++ b/lib/private/lock/memcachelockingprovider.php @@ -98,6 +98,26 @@ class MemcacheLockingProvider implements ILockingProvider { } } + /** + * 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) { + if ($targetType === self::LOCK_SHARED) { + if (!$this->memcache->cas($path, 'exclusive', 1)) { + throw new LockedException($path); + } + } else if ($targetType === self::LOCK_EXCLUSIVE) { + // we can only change a shared lock to an exclusive if there's only a single owner of the shared lock + if (!$this->memcache->cas($path, 1, 'exclusive')) { + throw new LockedException($path); + } + } + } + /** * release all lock acquired by this instance */ diff --git a/lib/public/lock/ilockingprovider.php b/lib/public/lock/ilockingprovider.php index 18fa47fa63..6a963b9d7a 100644 --- a/lib/public/lock/ilockingprovider.php +++ b/lib/public/lock/ilockingprovider.php @@ -45,6 +45,15 @@ interface ILockingProvider { */ public function releaseLock($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); + /** * release all lock acquired by this instance */ diff --git a/tests/lib/lock/lockingprovider.php b/tests/lib/lock/lockingprovider.php index 337aa4cea7..efd6e1939f 100644 --- a/tests/lib/lock/lockingprovider.php +++ b/tests/lib/lock/lockingprovider.php @@ -158,4 +158,57 @@ abstract class LockingProvider extends TestCase { $this->assertEquals('foo', $e->getPath()); } } + + public function testChangeLockToExclusive() { + $this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED); + $this->instance->changeLock('foo', ILockingProvider::LOCK_EXCLUSIVE); + $this->assertFalse($this->instance->isLocked('foo', ILockingProvider::LOCK_SHARED)); + $this->assertTrue($this->instance->isLocked('foo', ILockingProvider::LOCK_EXCLUSIVE)); + } + + public function testChangeLockToShared() { + $this->instance->acquireLock('foo', ILockingProvider::LOCK_EXCLUSIVE); + $this->instance->changeLock('foo', ILockingProvider::LOCK_SHARED); + $this->assertFalse($this->instance->isLocked('foo', ILockingProvider::LOCK_EXCLUSIVE)); + $this->assertTrue($this->instance->isLocked('foo', ILockingProvider::LOCK_SHARED)); + } + + /** + * @expectedException \OCP\Lock\LockedException + */ + public function testChangeLockToExclusiveDoubleShared() { + $this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED); + $this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED); + $this->instance->changeLock('foo', ILockingProvider::LOCK_EXCLUSIVE); + } + + /** + * @expectedException \OCP\Lock\LockedException + */ + public function testChangeLockToExclusiveNoShared() { + $this->instance->changeLock('foo', ILockingProvider::LOCK_EXCLUSIVE); + } + + /** + * @expectedException \OCP\Lock\LockedException + */ + public function testChangeLockToExclusiveFromExclusive() { + $this->instance->acquireLock('foo', ILockingProvider::LOCK_EXCLUSIVE); + $this->instance->changeLock('foo', ILockingProvider::LOCK_EXCLUSIVE); + } + + /** + * @expectedException \OCP\Lock\LockedException + */ + public function testChangeLockToSharedNoExclusive() { + $this->instance->changeLock('foo', ILockingProvider::LOCK_SHARED); + } + + /** + * @expectedException \OCP\Lock\LockedException + */ + public function testChangeLockToSharedFromShared() { + $this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED); + $this->instance->changeLock('foo', ILockingProvider::LOCK_SHARED); + } } From 661c9e2444c097aad7e47b77df4fc2b10c346d04 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 29 May 2015 14:40:06 +0200 Subject: [PATCH 32/35] add changeLock to the storage api --- apps/files_sharing/lib/sharedstorage.php | 11 +++++++++++ lib/private/files/storage/common.php | 9 +++++++++ lib/private/files/storage/storage.php | 8 ++++++++ lib/private/files/storage/wrapper/jail.php | 9 +++++++++ lib/private/files/storage/wrapper/wrapper.php | 9 +++++++++ lib/public/files/storage.php | 8 ++++++++ 6 files changed, 54 insertions(+) diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php index ad69583639..bf61dda371 100644 --- a/apps/files_sharing/lib/sharedstorage.php +++ b/apps/files_sharing/lib/sharedstorage.php @@ -634,4 +634,15 @@ class Shared extends \OC\Files\Storage\Common implements ISharedStorage { list($targetStorage, $targetInternalPath) = $this->resolvePath($path); $targetStorage->releaseLock($targetInternalPath, $type, $provider); } + + /** + * @param string $path + * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE + * @param \OCP\Lock\ILockingProvider $provider + */ + public function changeLock($path, $type, ILockingProvider $provider) { + /** @var \OCP\Files\Storage $targetStorage */ + list($targetStorage, $targetInternalPath) = $this->resolvePath($path); + $targetStorage->changeLock($targetInternalPath, $type, $provider); + } } diff --git a/lib/private/files/storage/common.php b/lib/private/files/storage/common.php index f0c9e1bfa0..847cb8492f 100644 --- a/lib/private/files/storage/common.php +++ b/lib/private/files/storage/common.php @@ -641,4 +641,13 @@ abstract class Common implements Storage { public function releaseLock($path, $type, ILockingProvider $provider) { $provider->releaseLock('files/' . md5($this->getId() . '::' . trim($path, '/')), $type); } + + /** + * @param string $path + * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE + * @param \OCP\Lock\ILockingProvider $provider + */ + public function changeLock($path, $type, ILockingProvider $provider) { + $provider->changeLock('files/' . md5($this->getId() . '::' . trim($path, '/')), $type); + } } diff --git a/lib/private/files/storage/storage.php b/lib/private/files/storage/storage.php index 642544bad3..bd809099e1 100644 --- a/lib/private/files/storage/storage.php +++ b/lib/private/files/storage/storage.php @@ -91,4 +91,12 @@ interface Storage extends \OCP\Files\Storage { * @param \OCP\Lock\ILockingProvider $provider */ public function releaseLock($path, $type, ILockingProvider $provider); + + /** + * @param string $path The path of the file to change the lock for + * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE + * @param \OCP\Lock\ILockingProvider $provider + * @throws \OCP\Lock\LockedException + */ + public function changeLock($path, $type, ILockingProvider $provider); } diff --git a/lib/private/files/storage/wrapper/jail.php b/lib/private/files/storage/wrapper/jail.php index 229d12e82f..2857e74de4 100644 --- a/lib/private/files/storage/wrapper/jail.php +++ b/lib/private/files/storage/wrapper/jail.php @@ -444,4 +444,13 @@ class Jail extends Wrapper { public function releaseLock($path, $type, ILockingProvider $provider) { $this->storage->releaseLock($this->getSourcePath($path), $type, $provider); } + + /** + * @param string $path + * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE + * @param \OCP\Lock\ILockingProvider $provider + */ + public function changeLock($path, $type, ILockingProvider $provider) { + $this->storage->changeLock($this->getSourcePath($path), $type, $provider); + } } diff --git a/lib/private/files/storage/wrapper/wrapper.php b/lib/private/files/storage/wrapper/wrapper.php index 6aca771c0b..d1414880be 100644 --- a/lib/private/files/storage/wrapper/wrapper.php +++ b/lib/private/files/storage/wrapper/wrapper.php @@ -561,4 +561,13 @@ class Wrapper implements \OC\Files\Storage\Storage { public function releaseLock($path, $type, ILockingProvider $provider) { $this->storage->releaseLock($path, $type, $provider); } + + /** + * @param string $path + * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE + * @param \OCP\Lock\ILockingProvider $provider + */ + public function changeLock($path, $type, ILockingProvider $provider) { + $this->storage->changeLock($path, $type, $provider); + } } diff --git a/lib/public/files/storage.php b/lib/public/files/storage.php index 68d00fab34..ee160c50e8 100644 --- a/lib/public/files/storage.php +++ b/lib/public/files/storage.php @@ -429,4 +429,12 @@ interface Storage { * @param \OCP\Lock\ILockingProvider $provider */ public function releaseLock($path, $type, ILockingProvider $provider); + + /** + * @param string $path The path of the file to change the lock for + * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE + * @param \OCP\Lock\ILockingProvider $provider + * @throws \OCP\Lock\LockedException + */ + public function changeLock($path, $type, ILockingProvider $provider); } From ce04cf6610ffd823ef1cb5ab3ee215b69afffcb4 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 29 May 2015 16:35:49 +0200 Subject: [PATCH 33/35] shared lock around hooks --- lib/private/files/view.php | 53 ++++++++++++++++---- lib/private/lock/memcachelockingprovider.php | 4 ++ 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/lib/private/files/view.php b/lib/private/files/view.php index d6cc62dff2..b98842f5eb 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -534,6 +534,7 @@ class View { and !Filesystem::isFileBlacklisted($path) ) { $path = $this->getRelativePath($absolutePath); + $exists = $this->file_exists($path); $run = true; if ($this->shouldEmitHooks($path)) { @@ -613,6 +614,10 @@ class View { if ($path1 == null or $path2 == null) { return false; } + + $this->lockFile($path1, ILockingProvider::LOCK_SHARED); + $this->lockFile($path2, ILockingProvider::LOCK_SHARED); + $run = true; if ($this->shouldEmitHooks() && (Cache\Scanner::isPartialFile($path1) && !Cache\Scanner::isPartialFile($path2))) { // if it was a rename from a part file to a regular file it was a write and not a rename operation @@ -638,13 +643,8 @@ class View { $internalPath1 = $mount1->getInternalPath($absolutePath1); $internalPath2 = $mount2->getInternalPath($absolutePath2); - $this->lockFile($path1, ILockingProvider::LOCK_EXCLUSIVE); - try { - $this->lockFile($path2, ILockingProvider::LOCK_EXCLUSIVE); - } catch (LockedException $e) { - $this->unlockFile($path1, ILockingProvider::LOCK_EXCLUSIVE); - throw $e; - } + $this->changeLock($path1, ILockingProvider::LOCK_EXCLUSIVE); + $this->changeLock($path2, ILockingProvider::LOCK_EXCLUSIVE); if ($internalPath1 === '' and $mount1 instanceof MoveableMount) { if ($this->isTargetAllowed($absolutePath2)) { @@ -702,6 +702,8 @@ class View { } return $result; } else { + $this->unlockFile($path1, ILockingProvider::LOCK_SHARED); + $this->unlockFile($path2, ILockingProvider::LOCK_SHARED); return false; } } else { @@ -733,6 +735,10 @@ class View { return false; } $run = true; + + $this->lockFile($path2, ILockingProvider::LOCK_SHARED); + $this->lockFile($path1, ILockingProvider::LOCK_SHARED); + $exists = $this->file_exists($path2); if ($this->shouldEmitHooks()) { \OC_Hook::emit( @@ -754,7 +760,7 @@ class View { $storage2 = $mount2->getStorage(); $internalPath2 = $mount2->getInternalPath($absolutePath2); - $this->lockFile($path2, ILockingProvider::LOCK_EXCLUSIVE); + $this->changeLock($path2, ILockingProvider::LOCK_EXCLUSIVE); if ($mount1->getMountPoint() == $mount2->getMountPoint()) { if ($storage1) { @@ -768,6 +774,7 @@ class View { $this->updater->update($path2); $this->unlockFile($path2, ILockingProvider::LOCK_EXCLUSIVE); + $this->unlockFile($path1, ILockingProvider::LOCK_SHARED); if ($this->shouldEmitHooks() && $result !== false) { \OC_Hook::emit( @@ -782,6 +789,8 @@ class View { } return $result; } else { + $this->unlockFile($path2, ILockingProvider::LOCK_SHARED); + $this->unlockFile($path1, ILockingProvider::LOCK_SHARED); return false; } } else { @@ -961,13 +970,16 @@ class View { return false; } + if(in_array('write', $hooks) || in_array('delete', $hooks) || in_array('read', $hooks)) { + // always a shared lock during pre-hooks so the hook can read the file + $this->lockFile($path, ILockingProvider::LOCK_SHARED); + } + $run = $this->runHooks($hooks, $path); list($storage, $internalPath) = Filesystem::resolvePath($absolutePath . $postFix); if ($run and $storage) { if (in_array('write', $hooks) || in_array('delete', $hooks)) { - $this->lockFile($path, ILockingProvider::LOCK_EXCLUSIVE); - } else if (in_array('read', $hooks)) { - $this->lockFile($path, ILockingProvider::LOCK_SHARED); + $this->changeLock($path, ILockingProvider::LOCK_EXCLUSIVE); } try { if (!is_null($extraParam)) { @@ -1017,6 +1029,8 @@ class View { } } return $result; + } else { + $this->unlockFile($path, ILockingProvider::LOCK_SHARED); } } return null; @@ -1661,6 +1675,23 @@ class View { } } + /** + * @param string $path the path of the file to lock, relative to the view + * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE + */ + private function changeLock($path, $type) { + $mount = $this->getMount($path); + if ($mount) { + $mount->getStorage()->changeLock( + $mount->getInternalPath( + $this->getAbsolutePath($path) + ), + $type, + $this->lockingProvider + ); + } + } + /** * @param string $path the path of the file to unlock, relative to the view * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE diff --git a/lib/private/lock/memcachelockingprovider.php b/lib/private/lock/memcachelockingprovider.php index 368ff45032..26957f8ced 100644 --- a/lib/private/lock/memcachelockingprovider.php +++ b/lib/private/lock/memcachelockingprovider.php @@ -110,11 +110,15 @@ class MemcacheLockingProvider implements ILockingProvider { if (!$this->memcache->cas($path, 'exclusive', 1)) { throw new LockedException($path); } + unset($this->acquiredLocks['exclusive'][$path]); + $this->acquiredLocks['shared'][$path]++; } else if ($targetType === self::LOCK_EXCLUSIVE) { // we can only change a shared lock to an exclusive if there's only a single owner of the shared lock if (!$this->memcache->cas($path, 1, 'exclusive')) { throw new LockedException($path); } + $this->acquiredLocks['exclusive'][$path] = true; + $this->acquiredLocks['shared'][$path]--; } } From 8902e2be733242bf02cb13ba6b374cdb45aa9e2f Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 1 Jun 2015 13:25:27 +0200 Subject: [PATCH 34/35] fix nooplockingprovider --- lib/private/lock/nooplockingprovider.php | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/private/lock/nooplockingprovider.php b/lib/private/lock/nooplockingprovider.php index 0ca8edb4d0..4f33b88155 100644 --- a/lib/private/lock/nooplockingprovider.php +++ b/lib/private/lock/nooplockingprovider.php @@ -51,10 +51,17 @@ class NoopLockingProvider implements ILockingProvider { // do nothing } - /** - * release all lock acquired by this instance + /**1 + * {@inheritdoc} */ public function releaseAll() { // do nothing } + + /** + * {@inheritdoc} + */ + public function changeLock($path, $targetType) { + // do nothing + } } From 2104c2ffdd4fb2f63c4150bd2ffd0ac35d5ea859 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Mon, 1 Jun 2015 14:10:00 +0200 Subject: [PATCH 35/35] Fixing undefined index 'foo' --- lib/private/lock/memcachelockingprovider.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/private/lock/memcachelockingprovider.php b/lib/private/lock/memcachelockingprovider.php index 26957f8ced..3f32ab1d8c 100644 --- a/lib/private/lock/memcachelockingprovider.php +++ b/lib/private/lock/memcachelockingprovider.php @@ -111,6 +111,9 @@ class MemcacheLockingProvider implements ILockingProvider { throw new LockedException($path); } unset($this->acquiredLocks['exclusive'][$path]); + if (!isset($this->acquiredLocks['shared'][$path])) { + $this->acquiredLocks['shared'][$path] = 0; + } $this->acquiredLocks['shared'][$path]++; } else if ($targetType === self::LOCK_EXCLUSIVE) { // we can only change a shared lock to an exclusive if there's only a single owner of the shared lock