From 188c868bd6aaede2e3fc8c7917841a82933aff76 Mon Sep 17 00:00:00 2001 From: Josh Date: Thu, 23 Jul 2026 20:34:19 -0400 Subject: [PATCH 1/3] fix(view): release target lock when copy source is locked Assisted-by: Copilot:GPT-5.6 Signed-off-by: Josh --- lib/private/Files/View.php | 57 ++++++++++++++++++++++++++++++++------ 1 file changed, 49 insertions(+), 8 deletions(-) diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php index c4162c7df046f..6cce1146a3c57 100644 --- a/lib/private/Files/View.php +++ b/lib/private/Files/View.php @@ -955,12 +955,15 @@ public function copy($source, $target, $preserveMtime = false) { } $run = true; - $this->lockFile($target, ILockingProvider::LOCK_SHARED); - $this->lockFile($source, ILockingProvider::LOCK_SHARED); + $targetLocked = $this->lockFile($target, ILockingProvider::LOCK_SHARED); + $sourceLocked = false; $lockTypePath1 = ILockingProvider::LOCK_SHARED; $lockTypePath2 = ILockingProvider::LOCK_SHARED; + $operationException = null; try { + $sourceLocked = $this->lockFile($source, ILockingProvider::LOCK_SHARED); + $exists = $this->file_exists($target); if ($this->shouldEmitHooks($source) && $this->shouldEmitHooks($target)) { \OC_Hook::emit( @@ -1014,15 +1017,53 @@ public function copy($source, $target, $preserveMtime = false) { $this->emit_file_hooks_post($exists, $target); } } - } catch (\Exception $e) { - $this->unlockFile($target, $lockTypePath2); - $this->unlockFile($source, $lockTypePath1); + } catch (\Throwable $e) { + $operationException = $e; throw $e; - } + } finally { + $cleanupException = null; + + // Preserve the prior target-then-source unlock order, but do not + // leave the source locked if releasing the target itself fails. + try { + if ($targetLocked) { + $this->unlockFile($target, $lockTypePath2); + } + } catch (\Throwable $unlockException) { + if ($operationException !== null) { + $this->logger->error('Failed to release target lock after copy operation', [ + 'app' => 'core', + 'source' => $source, + 'target' => $target, + 'exception' => $unlockException, + ]); + } else { + $cleanupException = $unlockException; + } + } - $this->unlockFile($target, $lockTypePath2); - $this->unlockFile($source, $lockTypePath1); + try { + if ($sourceLocked) { + $this->unlockFile($source, $lockTypePath1); + } + } catch (\Throwable $unlockException) { + if ($operationException !== null || $cleanupException !== null) { + $this->logger->error('Failed to release source lock after copy operation', [ + 'app' => 'core', + 'source' => $source, + 'target' => $target, + 'exception' => $unlockException, + ]); + } else { + $cleanupException = $unlockException; + } + } + + if ($operationException === null && $cleanupException !== null) { + throw $cleanupException; + } } + return $result; } From 1a6b45a0195c857e5c33285cf5234159015ed67d Mon Sep 17 00:00:00 2001 From: Josh Date: Thu, 23 Jul 2026 20:42:15 -0400 Subject: [PATCH 2/3] test(files): cover copy cleanup when source lock fails Assisted-by: Copilot:GPT-5.6 Signed-off-by: Josh --- tests/lib/Files/ViewTest.php | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/lib/Files/ViewTest.php b/tests/lib/Files/ViewTest.php index c21bf6fc40c9d..de7a9a8fc6c60 100644 --- a/tests/lib/Files/ViewTest.php +++ b/tests/lib/Files/ViewTest.php @@ -2315,6 +2315,37 @@ function (): void { } } + public function testCopyUnlocksTargetWhenSourceLockCannotBeAcquired(): void { + $view = new View('/' . self::$user . '/files/'); + $storage = new Temporary([]); + + Filesystem::mount($storage, [], self::$user . '/'); + $storage->mkdir('files'); + + $sourcePath = 'source.txt'; + $targetPath = 'target.txt'; + $view->file_put_contents($sourcePath, 'content'); + + $view->lockFile($sourcePath, ILockingProvider::LOCK_EXCLUSIVE); + + try { + $view->copy($sourcePath, $targetPath); + $this->fail('Expected source-lock acquisition to fail'); + } catch (LockedException) { + $this->assertNull( + $this->getFileLockType($view, $targetPath), + 'The target lock acquired before the source lock must be released' + ); + $this->assertSame( + ILockingProvider::LOCK_EXCLUSIVE, + $this->getFileLockType($view, $sourcePath), + 'The pre-existing source lock must not be released by copy()' + ); + } finally { + $view->unlockFile($sourcePath, ILockingProvider::LOCK_EXCLUSIVE); + } + } + /** * Test rename operation: unlock first path when second path was locked */ From 894d0927e23188d3e3857bd25edb72ba604818ed Mon Sep 17 00:00:00 2001 From: Josh Date: Thu, 23 Jul 2026 20:54:47 -0400 Subject: [PATCH 3/3] chore(view): fix typo / missing bracket in refactor Signed-off-by: Josh --- lib/private/Files/View.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php index 6cce1146a3c57..a59297be7dae4 100644 --- a/lib/private/Files/View.php +++ b/lib/private/Files/View.php @@ -963,8 +963,8 @@ public function copy($source, $target, $preserveMtime = false) { $operationException = null; try { $sourceLocked = $this->lockFile($source, ILockingProvider::LOCK_SHARED); - $exists = $this->file_exists($target); + if ($this->shouldEmitHooks($source) && $this->shouldEmitHooks($target)) { \OC_Hook::emit( Filesystem::CLASSNAME, @@ -977,6 +977,7 @@ public function copy($source, $target, $preserveMtime = false) { ); $this->emit_file_hooks_pre($exists, $target, $run); } + if ($run) { $mount1 = $this->getMount($source); $mount2 = $this->getMount($target); @@ -1061,6 +1062,7 @@ public function copy($source, $target, $preserveMtime = false) { if ($operationException === null && $cleanupException !== null) { throw $cleanupException; + } } }