Merge pull request #10116 from nextcloud/lock-negative

prevent lock values from going negative with memcache backend

lib/private/Lock/MemcacheLockingProvider.php

@@ -90,14 +90,20 @@ class MemcacheLockingProvider extends AbstractLockingProvider {
 	 */
 	public function releaseLock(string $path, int $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');

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);
+	}
 }