Merge pull request #10180 from nextcloud/lock-negative-13

[13] prevent lock values from going negative with memcache backend

2 files changed, 20 insertions, 2 deletions

diff --git a/lib/private/Lock/MemcacheLockingProvider.php b/lib/private/Lock/MemcacheLockingProvider.php
index ddd73aec095..893010fdf1e 100644
--- a/lib/private/Lock/MemcacheLockingProvider.php
+++ b/lib/private/Lock/MemcacheLockingProvider.php
@@ -89,14 +89,20 @@ class MemcacheLockingProvider extends AbstractLockingProvider {
 	 */
 	public function releaseLock($path, $type) {
 		if ($type === self::LOCK_SHARED) {
+			$newValue = 0;
 			if ($this->getOwnSharedLockCount($path) === 1) {
 				$removed = $this->memcache->cad($path, 1); // if we're the only one having a shared lock we can remove it in one go
 				if (!$removed) { //someone else also has a shared lock, decrease only
-					$this->memcache->dec($path);
+					$newValue = $this->memcache->dec($path);
 				}
 			} else {
 				// if we own more than one lock ourselves just decrease
-				$this->memcache->dec($path);
+				$newValue = $this->memcache->dec($path);
+			}
+
+			// if we somehow release more locks then exists, reset the lock
+			if ($newValue < 0) {
+				$this->memcache->cad($path, $newValue);
 			}
 		} else if ($type === self::LOCK_EXCLUSIVE) {
 			$this->memcache->cad($path, 'exclusive');
diff --git a/tests/lib/Lock/LockingProvider.php b/tests/lib/Lock/LockingProvider.php
index 49742baa561..9c0461e2e60 100644
--- a/tests/lib/Lock/LockingProvider.php
+++ b/tests/lib/Lock/LockingProvider.php
@@ -243,4 +243,16 @@ abstract class LockingProvider extends TestCase {
 		$this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED);
 		$this->instance->changeLock('foo', ILockingProvider::LOCK_SHARED);
 	}
+
+	public function testReleaseNonExistingShared() {
+		$this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED);
+		$this->instance->releaseLock('foo', ILockingProvider::LOCK_SHARED);
+
+		// releasing a lock once to many should not result in a locked state
+		$this->instance->releaseLock('foo', ILockingProvider::LOCK_SHARED);
+
+		$this->instance->acquireLock('foo', ILockingProvider::LOCK_EXCLUSIVE);
+		$this->assertTrue($this->instance->isLocked('foo', ILockingProvider::LOCK_EXCLUSIVE));
+		$this->instance->releaseLock('foo', ILockingProvider::LOCK_EXCLUSIVE);
+	}
 }