Merge pull request #17017 from owncloud/cache-update-write-lock

update the file cache within the write lock

lib/private/files/cache/scanner.php

@@ -124,13 +124,18 @@ class Scanner extends BasicEmitter {
 	 * @param int $reuseExisting
 	 * @param int $parentId
 	 * @param array | null $cacheData existing data in the cache for the file to be scanned
+	 * @param bool $lock set to false to disable getting an additional read lock during scanning
 	 * @return array an array of metadata of the scanned file
+	 * @throws \OC\ServerNotAvailableException
+	 * @throws \OCP\Lock\LockedException
 	 */
-	public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData = null) {
+	public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData = null, $lock = true) {
 		if (!self::isPartialFile($file)
 			and !Filesystem::isFileBlacklisted($file)
 		) {
-			$this->storage->acquireLock($file, ILockingProvider::LOCK_SHARED, $this->lockingProvider);
+			if ($lock) {
+				$this->storage->acquireLock($file, ILockingProvider::LOCK_SHARED, $this->lockingProvider);
+			}
 			$this->emit('\OC\Files\Cache\Scanner', 'scanFile', array($file, $this->storageId));
 			\OC_Hook::emit('\OC\Files\Cache\Scanner', 'scan_file', array('path' => $file, 'storage' => $this->storageId));
 			$data = $this->getData($file);
@@ -187,7 +192,9 @@ class Scanner extends BasicEmitter {
 			} else {
 				$this->removeFromCache($file);
 			}
-			$this->storage->releaseLock($file, ILockingProvider::LOCK_SHARED, $this->lockingProvider);
+			if ($lock) {
+				$this->storage->releaseLock($file, ILockingProvider::LOCK_SHARED, $this->lockingProvider);
+			}
 			return $data;
 		}
 		return null;
@@ -245,19 +252,24 @@ class Scanner extends BasicEmitter {
 	 * @param string $path
 	 * @param bool $recursive
 	 * @param int $reuse
+	 * @param bool $lock set to false to disable getting an additional read lock during scanning
 	 * @return array an array of the meta data of the scanned file or folder
 	 */
-	public function scan($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1) {
+	public function scan($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $lock = true) {
 		if ($reuse === -1) {
 			$reuse = ($recursive === self::SCAN_SHALLOW) ? self::REUSE_ETAG | self::REUSE_SIZE : self::REUSE_ETAG;
 		}
-		$this->storage->acquireLock($path, ILockingProvider::LOCK_SHARED, $this->lockingProvider);
+		if ($lock) {
-		$data = $this->scanFile($path, $reuse);
+			$this->storage->acquireLock($path, ILockingProvider::LOCK_SHARED, $this->lockingProvider);
+		}
+		$data = $this->scanFile($path, $reuse, -1, null, $lock);
 		if ($data and $data['mimetype'] === 'httpd/unix-directory') {
-			$size = $this->scanChildren($path, $recursive, $reuse, $data);
+			$size = $this->scanChildren($path, $recursive, $reuse, $data, $lock);
 			$data['size'] = $size;
 		}
-		$this->storage->releaseLock($path, ILockingProvider::LOCK_SHARED, $this->lockingProvider);
+		if ($lock) {
+			$this->storage->releaseLock($path, ILockingProvider::LOCK_SHARED, $this->lockingProvider);
+		}
 		return $data;
 	}
 
@@ -303,9 +315,10 @@ class Scanner extends BasicEmitter {
 	 * @param bool $recursive
 	 * @param int $reuse
 	 * @param array $folderData existing cache data for the folder to be scanned
+	 * @param bool $lock set to false to disable getting an additional read lock during scanning
 	 * @return int the size of the scanned folder or -1 if the size is unknown at this stage
 	 */
-	protected function scanChildren($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $folderData = null) {
+	protected function scanChildren($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $folderData = null, $lock = true) {
 		if ($reuse === -1) {
 			$reuse = ($recursive === self::SCAN_SHALLOW) ? self::REUSE_ETAG | self::REUSE_SIZE : self::REUSE_ETAG;
 		}
@@ -328,7 +341,7 @@ class Scanner extends BasicEmitter {
 			$child = ($path) ? $path . '/' . $file : $file;
 			try {
 				$existingData = isset($existingChildren[$file]) ? $existingChildren[$file] : null;
-				$data = $this->scanFile($child, $reuse, $folderId, $existingData);
+				$data = $this->scanFile($child, $reuse, $folderId, $existingData, $lock);
 				if ($data) {
 					if ($data['mimetype'] === 'httpd/unix-directory' and $recursive === self::SCAN_RECURSIVE) {
 						$childQueue[$child] = $data;
@@ -363,7 +376,7 @@ class Scanner extends BasicEmitter {
 		}
 
 		foreach ($childQueue as $child => $childData) {
-			$childSize = $this->scanChildren($child, self::SCAN_RECURSIVE, $reuse, $childData);
+			$childSize = $this->scanChildren($child, self::SCAN_RECURSIVE, $reuse, $childData, $lock);
 			if ($childSize === -1) {
 				$size = -1;
 			} else if ($size !== -1) {

lib/private/files/cache/updater.php

@@ -111,7 +111,7 @@ class Updater {
 			$this->propagator->addChange($path);
 			$cache = $storage->getCache($internalPath);
 			$scanner = $storage->getScanner($internalPath);
-			$data = $scanner->scan($internalPath, Scanner::SCAN_SHALLOW);
+			$data = $scanner->scan($internalPath, Scanner::SCAN_SHALLOW, -1, false);
 			$this->correctParentStorageMtime($storage, $internalPath);
 			$cache->correctFolderSize($internalPath, $data);
 			$this->propagator->propagateChanges($time);

lib/private/files/objectstore/noopscanner.php

@@ -40,7 +40,7 @@ class NoopScanner extends Scanner {
 	 * @param array|null $cacheData existing data in the cache for the file to be scanned
 	 * @return array an array of metadata of the scanned file
 	 */
-	public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData = null) {
+	public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData = null, $lock = true) {
 		return array();
 	}
 
@@ -52,7 +52,7 @@ class NoopScanner extends Scanner {
 	 * @param int $reuse
 	 * @return array with the meta data of the scanned file or folder
 	 */
-	public function scan($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1) {
+	public function scan($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $lock = true) {
 		return array();
 	}
 
@@ -65,7 +65,7 @@ class NoopScanner extends Scanner {
 	 * @param array $folderData existing cache data for the folder to be scanned
 	 * @return int the size of the scanned folder or -1 if the size is unknown at this stage
 	 */
-	protected function scanChildren($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $folderData = null) {
+	protected function scanChildren($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $folderData = null, $lock = true) {
 		return 0;
 	}
 

lib/private/files/view.php

@@ -557,11 +557,9 @@ class View {
 					fclose($target);
 					fclose($data);
 
-					$this->changeLock($path, ILockingProvider::LOCK_SHARED);
-
 					$this->updater->update($path);
 
-					$this->unlockFile($path, ILockingProvider::LOCK_SHARED);
+					$this->unlockFile($path, ILockingProvider::LOCK_EXCLUSIVE);
 
 					if ($this->shouldEmitHooks($path) && $result !== false) {
 						$this->emit_file_hooks_post($exists, $path);
@@ -682,6 +680,19 @@ class View {
 					$result = $storage2->moveFromStorage($storage1, $internalPath1, $internalPath2);
 				}
 
+				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);
+				} else if ($result) {
+					if ($internalPath1 !== '') { // dont do a cache update for moved mounts
+						$this->updater->rename($path1, $path2);
+					} else { // only do etag propagation
+						$this->getUpdater()->getPropagator()->addChange($path1);
+						$this->getUpdater()->getPropagator()->addChange($path2);
+						$this->getUpdater()->getPropagator()->propagateChanges();
+					}
+				}
+
 				$this->unlockFile($path1, ILockingProvider::LOCK_EXCLUSIVE);
 				$this->unlockFile($path2, ILockingProvider::LOCK_EXCLUSIVE);
 
@@ -691,19 +702,10 @@ class View {
 				}
 
 				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);
 					if ($this->shouldEmitHooks()) {
 						$this->emit_file_hooks_post($exists, $path2);
 					}
 				} elseif ($result) {
-					if ($internalPath1 !== '') { // dont do a cache update for moved mounts
-						$this->updater->rename($path1, $path2);
-					} else { // only do etag propagation
-						$this->getUpdater()->getPropagator()->addChange($path1);
-						$this->getUpdater()->getPropagator()->addChange($path2);
-						$this->getUpdater()->getPropagator()->propagateChanges();
-					}
 					if ($this->shouldEmitHooks($path1) and $this->shouldEmitHooks($path2)) {
 						\OC_Hook::emit(
 							Filesystem::CLASSNAME,
@@ -787,11 +789,9 @@ class View {
 					$result = $storage2->copyFromStorage($storage1, $internalPath1, $internalPath2);
 				}
 
-				$this->changeLock($path2, ILockingProvider::LOCK_SHARED);
-
 				$this->updater->update($path2);
 
-				$this->unlockFile($path2, ILockingProvider::LOCK_SHARED);
+				$this->unlockFile($path2, ILockingProvider::LOCK_EXCLUSIVE);
 				$this->unlockFile($path1, ILockingProvider::LOCK_SHARED);
 
 				if ($this->shouldEmitHooks() && $result !== false) {
@@ -1014,10 +1014,6 @@ class View {
 					throw $e;
 				}
 
-				if ((in_array('write', $hooks) || in_array('delete', $hooks)) && ($operation !== 'fopen' || $result === false)) {
-					$this->changeLock($path, ILockingProvider::LOCK_SHARED);
-				}
-
 				if (in_array('delete', $hooks) and $result) {
 					$this->updater->remove($path);
 				}
@@ -1028,6 +1024,10 @@ class View {
 					$this->updater->update($path, $extraParam);
 				}
 
+				if ((in_array('write', $hooks) || in_array('delete', $hooks)) && ($operation !== 'fopen' || $result === false)) {
+					$this->changeLock($path, ILockingProvider::LOCK_SHARED);
+				}
+
 				if ($operation === 'fopen' and is_resource($result)) {
 					$result = CallbackWrapper::wrap($result, null, null, function () use ($hooks, $path) {
 						if (in_array('write', $hooks)) {