From 24f0151f812568f63584fbece5a8c50947b51dcd Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 16 Jun 2015 14:37:41 +0200 Subject: [PATCH 1/4] handle locked exceptions when getting file/directory info in sabre --- lib/private/connector/sabre/directory.php | 7 ++++++- lib/private/connector/sabre/objecttree.php | 2 ++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/private/connector/sabre/directory.php b/lib/private/connector/sabre/directory.php index 85756f112e..e99411068f 100644 --- a/lib/private/connector/sabre/directory.php +++ b/lib/private/connector/sabre/directory.php @@ -30,6 +30,7 @@ namespace OC\Connector\Sabre; use OC\Connector\Sabre\Exception\InvalidPath; use OC\Connector\Sabre\Exception\FileLocked; use OCP\Lock\LockedException; +use Sabre\DAV\Exception\Locked; class Directory extends \OC\Connector\Sabre\Node implements \Sabre\DAV\ICollection, \Sabre\DAV\IQuota { @@ -191,7 +192,11 @@ class Directory extends \OC\Connector\Sabre\Node if (!is_null($this->dirContent)) { return $this->dirContent; } - $folderContent = $this->fileView->getDirectoryContent($this->path); + try { + $folderContent = $this->fileView->getDirectoryContent($this->path); + } catch (LockedException $e) { + throw new Locked(); + } $nodes = array(); foreach ($folderContent as $info) { diff --git a/lib/private/connector/sabre/objecttree.php b/lib/private/connector/sabre/objecttree.php index c96a745fcd..1e9b9ba59e 100644 --- a/lib/private/connector/sabre/objecttree.php +++ b/lib/private/connector/sabre/objecttree.php @@ -150,6 +150,8 @@ class ObjectTree extends \Sabre\DAV\Tree { throw new \Sabre\DAV\Exception\ServiceUnavailable('Storage not available'); } catch (StorageInvalidException $e) { throw new \Sabre\DAV\Exception\NotFound('Storage ' . $path . ' is invalid'); + } catch (LockedException $e) { + throw new \Sabre\DAV\Exception\Locked(); } } From 72eedda16cec0b15850371035c517e98b0e9e064 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 16 Jun 2015 14:37:15 +0200 Subject: [PATCH 2/4] use old cache data when locked --- lib/private/files/view.php | 79 ++++++++++++++++++++++++-------------- 1 file changed, 50 insertions(+), 29 deletions(-) diff --git a/lib/private/files/view.php b/lib/private/files/view.php index 0a597aee21..711acc0bf5 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -988,7 +988,7 @@ class View { return false; } - if(in_array('write', $hooks) || in_array('delete', $hooks) || in_array('read', $hooks)) { + 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); } @@ -1148,6 +1148,7 @@ class View { if (Cache\Scanner::isPartialFile($path)) { return $this->getPartFileInfo($path); } + $relativePath = $path; $path = Filesystem::normalizePath($this->fakeRoot . '/' . $path); $mount = Filesystem::getMountManager()->find($path); @@ -1157,19 +1158,26 @@ class View { if ($storage) { $cache = $storage->getCache($internalPath); - $data = $cache->get($internalPath); - $watcher = $storage->getWatcher($internalPath); - - // if the file is not in the cache or needs to be updated, trigger the scanner and reload the data - if (!$data) { - if (!$storage->file_exists($internalPath)) { - return false; - } - $scanner = $storage->getScanner($internalPath); - $scanner->scan($internalPath, Cache\Scanner::SCAN_SHALLOW); + try { + $this->lockFile($relativePath, ILockingProvider::LOCK_SHARED); $data = $cache->get($internalPath); - } else if (!Cache\Scanner::isPartialFile($internalPath) && $watcher->checkUpdate($internalPath, $data)) { - $this->updater->propagate($path); + $watcher = $storage->getWatcher($internalPath); + + // if the file is not in the cache or needs to be updated, trigger the scanner and reload the data + if (!$data) { + if (!$storage->file_exists($internalPath)) { + return false; + } + $scanner = $storage->getScanner($internalPath); + $scanner->scan($internalPath, Cache\Scanner::SCAN_SHALLOW); + $data = $cache->get($internalPath); + } else if (!Cache\Scanner::isPartialFile($internalPath) && $watcher->checkUpdate($internalPath, $data)) { + $this->updater->propagate($path); + $data = $cache->get($internalPath); + } + $this->unlockFile($relativePath, ILockingProvider::LOCK_SHARED); + } catch (LockedException $e) { + // dont try to update the cache when the file is locked $data = $cache->get($internalPath); } @@ -1231,26 +1239,37 @@ class View { $cache = $storage->getCache($internalPath); $user = \OC_User::getUser(); - $data = $cache->get($internalPath); - $watcher = $storage->getWatcher($internalPath); - if (!$data or $data['size'] === -1) { - if (!$storage->file_exists($internalPath)) { - return array(); - } - $scanner = $storage->getScanner($internalPath); - $scanner->scan($internalPath, Cache\Scanner::SCAN_SHALLOW); - $data = $cache->get($internalPath); - } else if ($watcher->checkUpdate($internalPath, $data)) { - $this->updater->propagate($path); - $data = $cache->get($internalPath); - } - - $folderId = $data['fileid']; /** * @var \OC\Files\FileInfo[] $files */ $files = array(); - $contents = $cache->getFolderContentsById($folderId); //TODO: mimetype_filter + + try { + $this->lockFile($directory, ILockingProvider::LOCK_SHARED); + + $data = $cache->get($internalPath); + $watcher = $storage->getWatcher($internalPath); + if (!$data or $data['size'] === -1) { + if (!$storage->file_exists($internalPath)) { + return array(); + } + $scanner = $storage->getScanner($internalPath); + $scanner->scan($internalPath, Cache\Scanner::SCAN_SHALLOW); + $data = $cache->get($internalPath); + } else if ($watcher->checkUpdate($internalPath, $data)) { + $this->updater->propagate($path); + $data = $cache->get($internalPath); + } + + $folderId = $data['fileid']; + $contents = $cache->getFolderContentsById($folderId); //TODO: mimetype_filter + + $this->unlockFile($directory, ILockingProvider::LOCK_SHARED); + } catch (LockedException $e) { + // dont try to update the cache when the file is locked + $contents = $cache->getFolderContents($internalPath); + } + foreach ($contents as $content) { if ($content['permissions'] === 0) { $content['permissions'] = $storage->getPermissions($content['path']); @@ -1805,6 +1824,8 @@ class View { * @return bool False if the path is excluded from locking, true otherwise */ public function unlockFile($path, $type) { + $path = '/' . trim($path, '/'); + $absolutePath = $this->getAbsolutePath($path); $absolutePath = Filesystem::normalizePath($absolutePath); if (!$this->shouldLockFile($absolutePath)) { From 6018567df2bc0c5f7cd50b8e27d054d0184a56a5 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 18 Jun 2015 14:49:31 +0200 Subject: [PATCH 3/4] unlock the file if the file doesnt exists --- lib/private/files/view.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/private/files/view.php b/lib/private/files/view.php index 711acc0bf5..46158b42f1 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -1166,6 +1166,7 @@ class View { // if the file is not in the cache or needs to be updated, trigger the scanner and reload the data if (!$data) { if (!$storage->file_exists($internalPath)) { + $this->unlockFile($relativePath, ILockingProvider::LOCK_SHARED); return false; } $scanner = $storage->getScanner($internalPath); @@ -1251,6 +1252,7 @@ class View { $watcher = $storage->getWatcher($internalPath); if (!$data or $data['size'] === -1) { if (!$storage->file_exists($internalPath)) { + $this->unlockFile($directory, ILockingProvider::LOCK_SHARED); return array(); } $scanner = $storage->getScanner($internalPath); From 17be0993b404d22e74caf89a578eebfc0b706ff7 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 18 Jun 2015 15:33:15 +0200 Subject: [PATCH 4/4] remove unneeded normalize --- lib/private/files/view.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/private/files/view.php b/lib/private/files/view.php index 46158b42f1..5794389626 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -1826,8 +1826,6 @@ class View { * @return bool False if the path is excluded from locking, true otherwise */ public function unlockFile($path, $type) { - $path = '/' . trim($path, '/'); - $absolutePath = $this->getAbsolutePath($path); $absolutePath = Filesystem::normalizePath($absolutePath); if (!$this->shouldLockFile($absolutePath)) {