cleanup shared lock if changing to exclusive lock failed

Signed-off-by: Robin Appelman <robin@icewind.nl>

diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php
index 21df67cf557429877a0d9b709b419008f4a715b1..6066aed411f077e013617a056de3188fd2e9c314 100644 (file)
--- a/lib/private/Files/View.php
+++ b/lib/private/Files/View.php
@@ -1137,7 +1137,13 @@ class View {
                        list($storage, $internalPath) = Filesystem::resolvePath($absolutePath . $postFix);
                        if ($run and $storage) {
                                if (in_array('write', $hooks) || in_array('delete', $hooks)) {
-                                       $this->changeLock($path, ILockingProvider::LOCK_EXCLUSIVE);
+                                       try {
+                                               $this->changeLock($path, ILockingProvider::LOCK_EXCLUSIVE);
+                                       } catch (LockedException $e) {
+                                               // release the shared lock we acquired before quiting
+                                               $this->unlockFile($path, ILockingProvider::LOCK_SHARED);
+                                               throw $e;
+                                       }
                                }
                                try {
                                        if (!is_null($extraParam)) {

diff --git a/tests/lib/Files/ViewTest.php b/tests/lib/Files/ViewTest.php
index 9b435f2b9351d8f8aff91c9891921e1d77348af1..7a0f8c274dfe714fca87207386526d2126607b3f 100644 (file)
--- a/tests/lib/Files/ViewTest.php
+++ b/tests/lib/Files/ViewTest.php
@@ -1997,6 +1997,37 @@ class ViewTest extends \Test\TestCase {
                $this->assertNull($this->getFileLockType($view, $path), 'File got unlocked after exception');
        }
 
+       public function testLockBasicOperationUnlocksAfterLockException() {
+               $view = new View('/' . $this->user . '/files/');
+
+               $storage = new Temporary([]);
+
+               Filesystem::mount($storage, array(), $this->user . '/');
+
+               $storage->mkdir('files');
+               $storage->mkdir('files/dir');
+               $storage->file_put_contents('files/test.txt', 'blah');
+               $storage->getScanner()->scan('files');
+
+               // get a shared lock
+               $handle = $view->fopen('test.txt', 'r');
+
+               $thrown = false;
+               try {
+                       // try (and fail) to get a write lock
+                       $view->unlink('test.txt');
+               } catch (\Exception $e) {
+                       $thrown = true;
+                       $this->assertInstanceOf(LockedException::class, $e);
+               }
+               $this->assertTrue($thrown, 'Exception was rethrown');
+
+               // clean shared lock
+               fclose($handle);
+
+               $this->assertNull($this->getFileLockType($view, 'test.txt'), 'File got unlocked');
+       }
+
        /**
         * Test locks for fopen with fclose at the end
         *