Merge pull request #7148 from nextcloud/rename-locks-stable11

Unlock files even if an exception occurs (stable11)

1 files changed, 83 insertions, 78 deletions

diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php
index e0638ea4ff7..881f7ca5e0b 100644
--- a/lib/private/Files/View.php
+++ b/lib/private/Files/View.php
@@ -739,94 +739,99 @@ class View {
 			$this->lockFile($path1, ILockingProvider::LOCK_SHARED, true);
 			try {
 				$this->lockFile($path2, ILockingProvider::LOCK_SHARED, true);
-			} catch (LockedException $e) {
-				$this->unlockFile($path1, ILockingProvider::LOCK_SHARED);
-				throw $e;
-			}
 
-			$run = true;
-			if ($this->shouldEmitHooks($path1) && (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
-				$this->emit_file_hooks_pre($exists, $path2, $run);
-			} elseif ($this->shouldEmitHooks($path1)) {
-				\OC_Hook::emit(
-					Filesystem::CLASSNAME, Filesystem::signal_rename,
-					array(
-						Filesystem::signal_param_oldpath => $this->getHookPath($path1),
-						Filesystem::signal_param_newpath => $this->getHookPath($path2),
-						Filesystem::signal_param_run => &$run
-					)
-				);
-			}
-			if ($run) {
-				$this->verifyPath(dirname($path2), basename($path2));
-
-				$manager = Filesystem::getMountManager();
-				$mount1 = $this->getMount($path1);
-				$mount2 = $this->getMount($path2);
-				$storage1 = $mount1->getStorage();
-				$storage2 = $mount2->getStorage();
-				$internalPath1 = $mount1->getInternalPath($absolutePath1);
-				$internalPath2 = $mount2->getInternalPath($absolutePath2);
-
-				$this->changeLock($path1, ILockingProvider::LOCK_EXCLUSIVE, true);
-				$this->changeLock($path2, ILockingProvider::LOCK_EXCLUSIVE, true);
-
-				if ($internalPath1 === '' and $mount1 instanceof MoveableMount) {
-					if ($this->isTargetAllowed($absolutePath2)) {
-						/**
-						 * @var \OC\Files\Mount\MountPoint | \OC\Files\Mount\MoveableMount $mount1
-						 */
-						$sourceMountPoint = $mount1->getMountPoint();
-						$result = $mount1->moveMount($absolutePath2);
-						$manager->moveMount($sourceMountPoint, $mount1->getMountPoint());
-					} else {
-						$result = false;
-					}
-					// moving a file/folder within the same mount point
-				} elseif ($storage1 === $storage2) {
-					if ($storage1) {
-						$result = $storage1->rename($internalPath1, $internalPath2);
-					} else {
-						$result = false;
-					}
-					// moving a file/folder between storages (from $storage1 to $storage2)
-				} else {
-					$result = $storage2->moveFromStorage($storage1, $internalPath1, $internalPath2);
+				$run = true;
+				if ($this->shouldEmitHooks($path1) && (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
+					$this->emit_file_hooks_pre($exists, $path2, $run);
+				} elseif ($this->shouldEmitHooks($path1)) {
+					\OC_Hook::emit(
+						Filesystem::CLASSNAME, Filesystem::signal_rename,
+						array(
+							Filesystem::signal_param_oldpath => $this->getHookPath($path1),
+							Filesystem::signal_param_newpath => $this->getHookPath($path2),
+							Filesystem::signal_param_run => &$run
+						)
+					);
 				}
+				if ($run) {
+					$this->verifyPath(dirname($path2), basename($path2));
 
-				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
+					$manager = Filesystem::getMountManager();
+					$mount1 = $this->getMount($path1);
+					$mount2 = $this->getMount($path2);
+					$storage1 = $mount1->getStorage();
+					$storage2 = $mount2->getStorage();
+					$internalPath1 = $mount1->getInternalPath($absolutePath1);
+					$internalPath2 = $mount2->getInternalPath($absolutePath2);
 
-					$this->writeUpdate($storage2, $internalPath2);
-				} else if ($result) {
-					if ($internalPath1 !== '') { // don't do a cache update for moved mounts
-						$this->renameUpdate($storage1, $storage2, $internalPath1, $internalPath2);
-					}
-				}
+					$this->changeLock($path1, ILockingProvider::LOCK_EXCLUSIVE, true);
+					try {
+						$this->changeLock($path2, ILockingProvider::LOCK_EXCLUSIVE, true);
+
+						if ($internalPath1 === '' and $mount1 instanceof MoveableMount) {
+							if ($this->isTargetAllowed($absolutePath2)) {
+								/**
+								 * @var \OC\Files\Mount\MountPoint | \OC\Files\Mount\MoveableMount $mount1
+								 */
+								$sourceMountPoint = $mount1->getMountPoint();
+								$result = $mount1->moveMount($absolutePath2);
+								$manager->moveMount($sourceMountPoint, $mount1->getMountPoint());
+							} else {
+								$result = false;
+							}
+							// moving a file/folder within the same mount point
+						} elseif ($storage1 === $storage2) {
+							if ($storage1) {
+								$result = $storage1->rename($internalPath1, $internalPath2);
+							} else {
+								$result = false;
+							}
+							// moving a file/folder between storages (from $storage1 to $storage2)
+						} else {
+							$result = $storage2->moveFromStorage($storage1, $internalPath1, $internalPath2);
+						}
 
-				$this->changeLock($path1, ILockingProvider::LOCK_SHARED, true);
-				$this->changeLock($path2, ILockingProvider::LOCK_SHARED, true);
+						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
 
-				if ((Cache\Scanner::isPartialFile($path1) && !Cache\Scanner::isPartialFile($path2)) && $result !== false) {
-					if ($this->shouldEmitHooks()) {
-						$this->emit_file_hooks_post($exists, $path2);
+							$this->writeUpdate($storage2, $internalPath2);
+						} else if ($result) {
+							if ($internalPath1 !== '') { // don't do a cache update for moved mounts
+								$this->renameUpdate($storage1, $storage2, $internalPath1, $internalPath2);
+							}
+						}
+
+					} catch (\Exception $e) {
+						throw $e;
+					} finally {
+						$this->changeLock($path1, ILockingProvider::LOCK_SHARED, true);
+						$this->changeLock($path2, ILockingProvider::LOCK_SHARED, true);
 					}
-				} elseif ($result) {
-					if ($this->shouldEmitHooks($path1) and $this->shouldEmitHooks($path2)) {
-						\OC_Hook::emit(
-							Filesystem::CLASSNAME,
-							Filesystem::signal_post_rename,
-							array(
-								Filesystem::signal_param_oldpath => $this->getHookPath($path1),
-								Filesystem::signal_param_newpath => $this->getHookPath($path2)
-							)
-						);
+
+					if ((Cache\Scanner::isPartialFile($path1) && !Cache\Scanner::isPartialFile($path2)) && $result !== false) {
+						if ($this->shouldEmitHooks()) {
+							$this->emit_file_hooks_post($exists, $path2);
+						}
+					} elseif ($result) {
+						if ($this->shouldEmitHooks($path1) and $this->shouldEmitHooks($path2)) {
+							\OC_Hook::emit(
+								Filesystem::CLASSNAME,
+								Filesystem::signal_post_rename,
+								array(
+									Filesystem::signal_param_oldpath => $this->getHookPath($path1),
+									Filesystem::signal_param_newpath => $this->getHookPath($path2)
+								)
+							);
+						}
 					}
 				}
+			} catch (\Exception $e) {
+				throw $e;
+			} finally {
+				$this->unlockFile($path1, ILockingProvider::LOCK_SHARED, true);
+				$this->unlockFile($path2, ILockingProvider::LOCK_SHARED, true);
 			}
-			$this->unlockFile($path1, ILockingProvider::LOCK_SHARED, true);
-			$this->unlockFile($path2, ILockingProvider::LOCK_SHARED, true);
 		}
 		return $result;
 	}