Merge pull request #20053 from owncloud/getfileinfo-locking

Don't lock if we're only reading cache metadata

2 files changed, 62 insertions, 39 deletions

diff --git a/lib/private/files/cache/watcher.php b/lib/private/files/cache/watcher.php
index ff97a1f9c01..e660e56bfee 100644
--- a/lib/private/files/cache/watcher.php
+++ b/lib/private/files/cache/watcher.php
@@ -74,37 +74,58 @@ class Watcher {
 	}
 
 	/**
-	 * check $path for updates
+	 * check $path for updates and update if needed
 	 *
 	 * @param string $path
 	 * @param array $cachedEntry
 	 * @return boolean true if path was updated
 	 */
 	public function checkUpdate($path, $cachedEntry = null) {
-		if ($this->watchPolicy === self::CHECK_ALWAYS or ($this->watchPolicy === self::CHECK_ONCE and array_search($path, $this->checkedPaths) === false)) {
-			if (is_null($cachedEntry)) {
-				$cachedEntry = $this->cache->get($path);
-			}
-			$this->checkedPaths[] = $path;
-			if ($this->storage->hasUpdated($path, $cachedEntry['storage_mtime'])) {
-				if ($this->storage->is_dir($path)) {
-					$this->scanner->scan($path, Scanner::SCAN_SHALLOW);
-				} else {
-					$this->scanner->scanFile($path);
-				}
-				if ($cachedEntry['mimetype'] === 'httpd/unix-directory') {
-					$this->cleanFolder($path);
-				}
-				$this->cache->correctFolderSize($path);
-				return true;
-			}
-			return false;
+		if (is_null($cachedEntry)) {
+			$cachedEntry = $this->cache->get($path);
+		}
+		if ($this->needsUpdate($path, $cachedEntry)) {
+			$this->update($path, $cachedEntry);
+			return true;
 		} else {
 			return false;
 		}
 	}
 
 	/**
+	 * Update the cache for changes to $path
+	 *
+	 * @param string $path
+	 * @param array $cachedData
+	 */
+	public function update($path, $cachedData) {
+		if ($this->storage->is_dir($path)) {
+			$this->scanner->scan($path, Scanner::SCAN_SHALLOW);
+		} else {
+			$this->scanner->scanFile($path);
+		}
+		if ($cachedData['mimetype'] === 'httpd/unix-directory') {
+			$this->cleanFolder($path);
+		}
+		$this->cache->correctFolderSize($path);
+	}
+
+	/**
+	 * Check if the cache for $path needs to be updated
+	 *
+	 * @param string $path
+	 * @param array $cachedData
+	 * @return bool
+	 */
+	public function needsUpdate($path, $cachedData) {
+		if ($this->watchPolicy === self::CHECK_ALWAYS or ($this->watchPolicy === self::CHECK_ONCE and array_search($path, $this->checkedPaths) === false)) {
+			$this->checkedPaths[] = $path;
+			return $this->storage->hasUpdated($path, $cachedData['storage_mtime']);
+		}
+		return false;
+	}
+
+	/**
 	 * remove deleted files in $path from the cache
 	 *
 	 * @param string $path
diff --git a/lib/private/files/view.php b/lib/private/files/view.php
index a11df53705c..887b18530d7 100644
--- a/lib/private/files/view.php
+++ b/lib/private/files/view.php
@@ -1191,13 +1191,13 @@ class View {
 		if ($storage) {
 			$cache = $storage->getCache($internalPath);
 
-			try {
-				$this->lockFile($relativePath, ILockingProvider::LOCK_SHARED);
-				$data = $cache->get($internalPath);
-				$watcher = $storage->getWatcher($internalPath);
+			$data = $cache->get($internalPath);
+			$watcher = $storage->getWatcher($internalPath);
 
+			try {
 				// if the file is not in the cache or needs to be updated, trigger the scanner and reload the data
 				if (!$data) {
+					$this->lockFile($relativePath, ILockingProvider::LOCK_SHARED);
 					if (!$storage->file_exists($internalPath)) {
 						$this->unlockFile($relativePath, ILockingProvider::LOCK_SHARED);
 						return false;
@@ -1205,14 +1205,16 @@ class View {
 					$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->unlockFile($relativePath, ILockingProvider::LOCK_SHARED);
+				} else if (!Cache\Scanner::isPartialFile($internalPath) && $watcher->needsUpdate($internalPath, $data)) {
+					$this->lockFile($relativePath, ILockingProvider::LOCK_SHARED);
+					$watcher->update($internalPath, $data);
 					$this->updater->propagate($path);
 					$data = $cache->get($internalPath);
+					$this->unlockFile($relativePath, ILockingProvider::LOCK_SHARED);
 				}
-				$this->unlockFile($relativePath, ILockingProvider::LOCK_SHARED);
 			} catch (LockedException $e) {
-				// dont try to update the cache when the file is locked
-				$data = $cache->get($internalPath);
+				// if the file is locked we just use the old cache info
 			}
 
 			if ($data and isset($data['fileid'])) {
@@ -1278,12 +1280,11 @@ class View {
 			 */
 			$files = array();
 
+			$data = $cache->get($internalPath);
+			$watcher = $storage->getWatcher($internalPath);
 			try {
-				$this->lockFile($directory, ILockingProvider::LOCK_SHARED);
-
-				$data = $cache->get($internalPath);
-				$watcher = $storage->getWatcher($internalPath);
 				if (!$data or $data['size'] === -1) {
+					$this->lockFile($directory, ILockingProvider::LOCK_SHARED);
 					if (!$storage->file_exists($internalPath)) {
 						$this->unlockFile($directory, ILockingProvider::LOCK_SHARED);
 						return array();
@@ -1291,20 +1292,21 @@ class View {
 					$scanner = $storage->getScanner($internalPath);
 					$scanner->scan($internalPath, Cache\Scanner::SCAN_SHALLOW);
 					$data = $cache->get($internalPath);
-				} else if ($watcher->checkUpdate($internalPath, $data)) {
+					$this->unlockFile($directory, ILockingProvider::LOCK_SHARED);
+				} else if ($watcher->needsUpdate($internalPath, $data)) {
+					$this->lockFile($directory, ILockingProvider::LOCK_SHARED);
+					$watcher->update($internalPath, $data);
 					$this->updater->propagate($path);
 					$data = $cache->get($internalPath);
+					$this->unlockFile($directory, ILockingProvider::LOCK_SHARED);
 				}
-
-				$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);
+				// if the file is locked we just use the old cache info
 			}
 
+			$folderId = $data['fileid'];
+			$contents = $cache->getFolderContentsById($folderId); //TODO: mimetype_filter
+
 			foreach ($contents as $content) {
 				if ($content['permissions'] === 0) {
 					$content['permissions'] = $storage->getPermissions($content['path']);