Merge pull request #16963 from owncloud/fileinfo-lock

Acquire read lock when getting file or directory info

3 files changed, 57 insertions, 29 deletions

diff --git a/lib/private/connector/sabre/directory.php b/lib/private/connector/sabre/directory.php
index 85756f112e7..e99411068f1 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 c96a745fcd4..1e9b9ba59e2 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();
 			}
 		}
 
diff --git a/lib/private/files/view.php b/lib/private/files/view.php
index 2297221408b..47cbf35836c 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,27 @@ class View {
 		if ($storage) {
 			$cache = $storage->getCache($internalPath);
 
-			$data = $cache->get($internalPath);
-			$watcher = $storage->getWatcher($internalPath);
+			try {
+				$this->lockFile($relativePath, ILockingProvider::LOCK_SHARED);
+				$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;
+				// 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);
+					$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);
 				}
-				$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);
+				$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 +1240,38 @@ 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)) {
+						$this->unlockFile($directory, ILockingProvider::LOCK_SHARED);
+						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']);