From 9126cdc7a88460c4c8c11c4e6ce29b7a2df932b5 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 22 Jan 2019 15:32:48 +0100 Subject: [PATCH] cleanup shared lock if changing to exclusive lock failed Signed-off-by: Robin Appelman --- lib/private/Files/View.php | 8 +++++++- tests/lib/Files/ViewTest.php | 31 +++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php index cd1219fd77e90..0acd918f899f1 100644 --- a/lib/private/Files/View.php +++ b/lib/private/Files/View.php @@ -1139,7 +1139,13 @@ private function basicOperation($operation, $path, $hooks = [], $extraParam = nu 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 33d5cc0a8a665..8ac7433aaa288 100644 --- a/tests/lib/Files/ViewTest.php +++ b/tests/lib/Files/ViewTest.php @@ -1987,6 +1987,37 @@ function () { $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 *