Skip to content

fix(lock): valid lock types and modernize locking providers #62421

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
joshtrichards wants to merge 3 commits into
base: master
Choose a base branch
from
+224 −147
Draft

fix(lock): valid lock types and modernize locking providers #62421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e92aead
fix(lock): reject invalid lock types and modernize memcache provider
joshtrichards Jul 22, 2026
75fff30
fix(lock): reject invalid lock types and modernize db provider
joshtrichards Jul 22, 2026
a1cda60
test(lock): add lock type validation rejection checks to LockingProvider
joshtrichards Jul 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
195 changes: 108 additions & 87 deletions lib/private/Lock/DBLockingProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php

declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-FileCopyrightText: 2016 ownCloud, Inc.
Expand All @@ -12,13 +13,13 @@
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IDBConnection;
use OCP\Lock\ILockingProvider;
use OCP\Lock\LockedException;

/**
* Locking provider that stores the locks in the database
* Locking provider that stores locks in the database.
*/
class DBLockingProvider extends AbstractLockingProvider {
/** @var array<string, bool> */
private array $sharedLocks = [];

public function __construct(
Expand All @@ -31,87 +32,93 @@
}

/**
* Check if we have an open shared lock for a path
* Whether this request tracks a shared lock retained in the database.
*/
protected function isLocallyLocked(string $path): bool {
return isset($this->sharedLocks[$path]) && $this->sharedLocks[$path];
return $this->sharedLocks[$path] ?? false;
}

/** @inheritDoc */
#[\Override]
protected function markAcquire(string $path, int $targetType): void {
parent::markAcquire($path, $targetType);
if ($this->cacheSharedLocks) {
if ($targetType === self::LOCK_SHARED) {
$this->sharedLocks[$path] = true;
}

if (!$this->cacheSharedLocks) {
return;
}

if ($targetType === self::LOCK_SHARED) {
$this->sharedLocks[$path] = true;
}
}

/**
* Change the type of an existing tracked lock
*/
#[\Override]
protected function markChange(string $path, int $targetType): void {
parent::markChange($path, $targetType);
if ($this->cacheSharedLocks) {
if ($targetType === self::LOCK_SHARED) {
$this->sharedLocks[$path] = true;
} elseif ($targetType === self::LOCK_EXCLUSIVE) {
$this->sharedLocks[$path] = false;
}

if (!$this->cacheSharedLocks) {
return;
}

if ($targetType === self::LOCK_SHARED) {
$this->sharedLocks[$path] = true;
} elseif ($targetType === self::LOCK_EXCLUSIVE) {
$this->sharedLocks[$path] = false;
}
}

/**
* Insert a file locking row if it does not exists.
* Insert a file-lock row if it does not already exist.
*/
protected function initLockField(string $path, int $lock = 0): int {
$expire = $this->getExpireTime();
return $this->connection->insertIgnoreConflict('file_locks', [
'key' => $path,
'lock' => $lock,
'ttl' => $expire
'ttl' => $expire,
]);
}

protected function getExpireTime(): int {
return $this->timeFactory->getTime() + $this->ttl;
}

/** @inheritDoc */
#[\Override]
public function isLocked(string $path, int $type): bool {
$this->assertValidLockType($type);

Check failure on line 87 in lib/private/Lock/DBLockingProvider.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

UndefinedMethod 

lib/private/Lock/DBLockingProvider.php:87:10: UndefinedMethod: Method OC\Lock\DBLockingProvider::assertValidLockType does not exist (see https://psalm.dev/022)

if ($this->hasAcquiredLock($path, $type)) {
return true;
}

$query = $this->connection->getQueryBuilder();
$query->select('lock')
->from('file_locks')
->where($query->expr()->eq('key', $query->createNamedParameter($path)));

$result = $query->executeQuery();
$lockValue = (int)$result->fetchOne();

if ($type === self::LOCK_SHARED) {
if ($this->isLocallyLocked($path)) {
// if we have a shared lock we kept open locally but it's released we always have at least 1 shared lock in the db
return $lockValue > 1;
} else {
return $lockValue > 0;
}
} elseif ($type === self::LOCK_EXCLUSIVE) {
return $lockValue === -1;
} else {
return false;
// A shared lock retained for this request remains in the database after it is
// released from active bookkeeping.
return $this->isLocallyLocked($path)
? $lockValue > 1
: $lockValue > 0;
}

return $lockValue === -1;
}

/** @inheritDoc */
#[\Override]
public function acquireLock(string $path, int $type, ?string $readablePath = null): void {
$this->assertValidLockType($type);

Check failure on line 114 in lib/private/Lock/DBLockingProvider.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

UndefinedMethod 

lib/private/Lock/DBLockingProvider.php:114:10: UndefinedMethod: Method OC\Lock\DBLockingProvider::assertValidLockType does not exist (see https://psalm.dev/022)

$expire = $this->getExpireTime();

if ($type === self::LOCK_SHARED) {
if (!$this->isLocallyLocked($path)) {
$result = $this->initLockField($path, 1);

if ($result <= 0) {
$query = $this->connection->getQueryBuilder();
$query->update('file_locks')
Expand All @@ -124,12 +131,20 @@
} else {
$result = 1;
}
} else {
} elseif ($type === self::LOCK_EXCLUSIVE) {
$existing = 0;
if ($this->hasAcquiredLock($path, ILockingProvider::LOCK_SHARED) === false && $this->isLocallyLocked($path)) {

// A shared lock retained for this request may remain in the database after it
// has been released from active bookkeeping.
if (
$this->hasAcquiredLock($path, self::LOCK_SHARED) === false
&& $this->isLocallyLocked($path)
) {
$existing = 1;
}

$result = $this->initLockField($path, -1);

if ($result <= 0) {
$query = $this->connection->getQueryBuilder();
$query->update('file_locks')
Expand All @@ -140,109 +155,115 @@
$result = $query->executeStatement();
}
}

if ($result !== 1) {
throw new LockedException($path, null, null, $readablePath);
}

$this->markAcquire($path, $type);
}

/** @inheritDoc */
#[\Override]
public function releaseLock(string $path, int $type): void {
$this->assertValidLockType($type);

Check failure on line 168 in lib/private/Lock/DBLockingProvider.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

UndefinedMethod 

lib/private/Lock/DBLockingProvider.php:168:10: UndefinedMethod: Method OC\Lock\DBLockingProvider::assertValidLockType does not exist (see https://psalm.dev/022)

$this->markRelease($path, $type);

// we keep shared locks till the end of the request so we can re-use them
// Shared locks remain in the database until the end of the request so they
// can be reused.
if ($type === self::LOCK_EXCLUSIVE) {
$qb = $this->connection->getQueryBuilder();
$qb->update('file_locks')
->set('lock', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))
->where($qb->expr()->eq('key', $qb->createNamedParameter($path)))
->andWhere($qb->expr()->eq('lock', $qb->createNamedParameter(-1, IQueryBuilder::PARAM_INT)));
$qb->executeStatement();
$query = $this->connection->getQueryBuilder();
$query->update('file_locks')
->set('lock', $query->createNamedParameter(0, IQueryBuilder::PARAM_INT))
->where($query->expr()->eq('key', $query->createNamedParameter($path)))
->andWhere($query->expr()->eq('lock', $query->createNamedParameter(-1, IQueryBuilder::PARAM_INT)));
$query->executeStatement();
} elseif (!$this->cacheSharedLocks) {
$qb = $this->connection->getQueryBuilder();
$qb->update('file_locks')
->set('lock', $qb->func()->subtract('lock', $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT)))
->where($qb->expr()->eq('key', $qb->createNamedParameter($path)))
->andWhere($qb->expr()->gt('lock', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)));
$qb->executeStatement();
$query = $this->connection->getQueryBuilder();
$query->update('file_locks')
->set('lock', $query->func()->subtract('lock', $query->createNamedParameter(1, IQueryBuilder::PARAM_INT)))
->where($query->expr()->eq('key', $query->createNamedParameter($path)))
->andWhere($query->expr()->gt('lock', $query->createNamedParameter(0, IQueryBuilder::PARAM_INT)));
$query->executeStatement();
}
}

/** @inheritDoc */
#[\Override]
public function changeLock(string $path, int $targetType): void {
$this->assertValidLockType($targetType);

Check failure on line 193 in lib/private/Lock/DBLockingProvider.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

UndefinedMethod 

lib/private/Lock/DBLockingProvider.php:193:10: UndefinedMethod: Method OC\Lock\DBLockingProvider::assertValidLockType does not exist (see https://psalm.dev/022)

$expire = $this->getExpireTime();

if ($targetType === self::LOCK_SHARED) {
$qb = $this->connection->getQueryBuilder();
$result = $qb->update('file_locks')
->set('lock', $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT))
->set('ttl', $qb->createNamedParameter($expire, IQueryBuilder::PARAM_INT))
->where($qb->expr()->andX(
$qb->expr()->eq('key', $qb->createNamedParameter($path)),
$qb->expr()->eq('lock', $qb->createNamedParameter(-1, IQueryBuilder::PARAM_INT))
))->executeStatement();
} else {
// since we only keep one shared lock in the db we need to check if we have more than one shared lock locally manually
if (isset($this->acquiredLocks['shared'][$path]) && $this->acquiredLocks['shared'][$path] > 1) {
$query = $this->connection->getQueryBuilder();
$query->update('file_locks')
->set('lock', $query->createNamedParameter(1, IQueryBuilder::PARAM_INT))
->set('ttl', $query->createNamedParameter($expire, IQueryBuilder::PARAM_INT))
->where($query->expr()->andX(
$query->expr()->eq('key', $query->createNamedParameter($path)),
$query->expr()->eq('lock', $query->createNamedParameter(-1, IQueryBuilder::PARAM_INT))));
$result = $query->executeStatement();
} elseif ($targetType === self::LOCK_EXCLUSIVE) {
// The database retains one shared lock per request, so an upgrade is only
// possible when this request holds exactly one shared lock.
if ($this->getOwnSharedLockCount($path) > 1) {
throw new LockedException($path);
}
$qb = $this->connection->getQueryBuilder();
$result = $qb->update('file_locks')
->set('lock', $qb->createNamedParameter(-1, IQueryBuilder::PARAM_INT))
->set('ttl', $qb->createNamedParameter($expire, IQueryBuilder::PARAM_INT))
->where($qb->expr()->andX(
$qb->expr()->eq('key', $qb->createNamedParameter($path)),
$qb->expr()->eq('lock', $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT))
))->executeStatement();

$query = $this->connection->getQueryBuilder();
$query->update('file_locks')
->set('lock', $query->createNamedParameter(-1, IQueryBuilder::PARAM_INT))
->set('ttl', $query->createNamedParameter($expire, IQueryBuilder::PARAM_INT))
->where($query->expr()->andX(
$query->expr()->eq('key', $query->createNamedParameter($path)),
$query->expr()->eq('lock', $query->createNamedParameter(1, IQueryBuilder::PARAM_INT))));
$result = $query->executeStatement();
}

if ($result !== 1) {
throw new LockedException($path);
}

$this->markChange($path, $targetType);
}

/** @inheritDoc */
public function cleanExpiredLocks(): void {
$expire = $this->timeFactory->getTime();

try {
$qb = $this->connection->getQueryBuilder();
$qb->delete('file_locks')
->where($qb->expr()->lt('ttl', $qb->createNamedParameter($expire, IQueryBuilder::PARAM_INT)))
->executeStatement();
$query = $this->connection->getQueryBuilder();
$query->delete('file_locks')
->where($query->expr()->lt('ttl', $query->createNamedParameter($expire, IQueryBuilder::PARAM_INT)));
$query->executeStatement();
} catch (\Exception $e) {
// If the table is missing, the clean up was successful
// If the table is missing, cleanup was successful.
if ($this->connection->tableExists('file_locks')) {
throw $e;
}
}
}

/** @inheritDoc */
#[\Override]
public function releaseAll(): void {
parent::releaseAll();

if (!$this->cacheSharedLocks) {
return;
}
// since we keep shared locks we need to manually clean those
$lockedPaths = array_keys($this->sharedLocks);
$lockedPaths = array_filter($lockedPaths, function ($path) {
return $this->sharedLocks[$path];
});

// Shared locks remain in the database until the end of the request.
$lockedPaths = array_keys(array_filter($this->sharedLocks));
$chunkedPaths = array_chunk($lockedPaths, 100);

$qb = $this->connection->getQueryBuilder();
$qb->update('file_locks')
->set('lock', $qb->func()->subtract('lock', $qb->expr()->literal(1)))
->where($qb->expr()->in('key', $qb->createParameter('chunk')))
->andWhere($qb->expr()->gt('lock', new Literal(0)));
$query = $this->connection->getQueryBuilder();
$query->update('file_locks')
->set('lock', $query->func()->subtract('lock', $query->expr()->literal(1)))
->where($query->expr()->in('key', $query->createParameter('chunk')))
->andWhere($query->expr()->gt('lock', new Literal(0)));

foreach ($chunkedPaths as $chunk) {
$qb->setParameter('chunk', $chunk, IQueryBuilder::PARAM_STR_ARRAY);
$qb->executeStatement();
$query->setParameter('chunk', $chunk, IQueryBuilder::PARAM_STR_ARRAY);
$query->executeStatement();
}
}
}
Loading
Loading