LockManager: distinguish conflicts from other kinds of lock errors

With a user-friendly error message which uses both technical and
non-technical language, without the irrelevant detail of filebackend
URLs.

Bug: T283045
Change-Id: I8704f405d38acdffae7bd4ea5b91f3f591fc2ebc
This commit is contained in:
Tim Starling 2021-11-01 13:40:54 +11:00
parent 2b9cabf5b3
commit 12cac7cca9
8 changed files with 15 additions and 24 deletions

View file

@ -105,9 +105,7 @@ class MySqlLockManager extends DBLockManager {
} }
if ( $blocked ) { if ( $blocked ) {
foreach ( $paths as $path ) { $status->fatal( 'lockmanager-fail-conflict' );
$status->fatal( 'lockmanager-fail-acquirelock', $path );
}
} }
return $status; return $status;

View file

@ -143,7 +143,7 @@ class FSLockManager extends LockManager {
$this->handles[$path] = $handle; $this->handles[$path] = $handle;
} else { } else {
fclose( $handle ); fclose( $handle );
$status->fatal( 'lockmanager-fail-acquirelock', $path ); $status->fatal( 'lockmanager-fail-conflict' );
} }
} else { } else {
$status->fatal( 'lockmanager-fail-openlock', $path ); $status->fatal( 'lockmanager-fail-openlock', $path );

View file

@ -101,10 +101,7 @@ class MemcLockManager extends QuorumLockManager {
// Lock all of the active lock record keys... // Lock all of the active lock record keys...
if ( !$this->acquireMutexes( $memc, $keys ) ) { if ( !$this->acquireMutexes( $memc, $keys ) ) {
foreach ( $paths as $path ) { $status->fatal( 'lockmanager-fail-conflict' );
$status->fatal( 'lockmanager-fail-acquirelock', $path );
}
return $status; return $status;
} }
@ -123,7 +120,7 @@ class MemcLockManager extends QuorumLockManager {
if ( $expiry < $now ) { // stale? if ( $expiry < $now ) { // stale?
unset( $locksHeld[self::LOCK_EX][$session] ); unset( $locksHeld[self::LOCK_EX][$session] );
} elseif ( $session !== $this->session ) { } elseif ( $session !== $this->session ) {
$status->fatal( 'lockmanager-fail-acquirelock', $path ); $status->fatal( 'lockmanager-fail-conflict' );
} }
} }
if ( $type === self::LOCK_EX ) { if ( $type === self::LOCK_EX ) {
@ -131,7 +128,7 @@ class MemcLockManager extends QuorumLockManager {
if ( $expiry < $now ) { // stale? if ( $expiry < $now ) { // stale?
unset( $locksHeld[self::LOCK_SH][$session] ); unset( $locksHeld[self::LOCK_SH][$session] );
} elseif ( $session !== $this->session ) { } elseif ( $session !== $this->session ) {
$status->fatal( 'lockmanager-fail-acquirelock', $path ); $status->fatal( 'lockmanager-fail-conflict' );
} }
} }
} }

View file

@ -54,9 +54,7 @@ class PostgreSqlLockManager extends DBLockManager {
if ( count( $fields ) ) { if ( count( $fields ) ) {
$db->query( 'SELECT ' . implode( ', ', $fields ), __METHOD__ ); $db->query( 'SELECT ' . implode( ', ', $fields ), __METHOD__ );
} }
foreach ( $paths as $path ) { $status->fatal( 'lockmanager-fail-conflict' );
$status->fatal( 'lockmanager-fail-acquirelock', $path );
}
} }
return $status; return $status;

View file

@ -164,10 +164,8 @@ LUA;
foreach ( $pathList as $path ) { foreach ( $pathList as $path ) {
$status->fatal( 'lockmanager-fail-acquirelock', $path ); $status->fatal( 'lockmanager-fail-acquirelock', $path );
} }
} else { } elseif ( count( $res ) ) {
foreach ( $res as $key ) { $status->fatal( 'lockmanager-fail-conflict' );
$status->fatal( 'lockmanager-fail-acquirelock', $pathsByKey[$key] );
}
} }
return $status; return $status;

View file

@ -1870,6 +1870,7 @@
"lockmanager-fail-db-release": "Could not release locks on database $1.", "lockmanager-fail-db-release": "Could not release locks on database $1.",
"lockmanager-fail-svr-acquire": "Could not acquire locks on server $1.", "lockmanager-fail-svr-acquire": "Could not acquire locks on server $1.",
"lockmanager-fail-svr-release": "Could not release locks on server $1.", "lockmanager-fail-svr-release": "Could not release locks on server $1.",
"lockmanager-fail-conflict": "Could not acquire lock. Somebody else is doing something to this file.",
"zip-file-open-error": "An error was encountered when opening the file for ZIP checks.", "zip-file-open-error": "An error was encountered when opening the file for ZIP checks.",
"zip-wrong-format": "The specified file was not a ZIP file.", "zip-wrong-format": "The specified file was not a ZIP file.",
"zip-bad": "The file is a corrupt or otherwise unreadable ZIP file.\nIt cannot be properly checked for security.", "zip-bad": "The file is a corrupt or otherwise unreadable ZIP file.\nIt cannot be properly checked for security.",

View file

@ -2097,6 +2097,7 @@
"lockmanager-fail-db-release": "Parameters:\n* $1 is a database name.", "lockmanager-fail-db-release": "Parameters:\n* $1 is a database name.",
"lockmanager-fail-svr-acquire": "Parameters:\n* $1 - server", "lockmanager-fail-svr-acquire": "Parameters:\n* $1 - server",
"lockmanager-fail-svr-release": "Parameters:\n* $1 is a server name.", "lockmanager-fail-svr-release": "Parameters:\n* $1 is a server name.",
"lockmanager-fail-conflict": "Used as an error message when the lock is held by another process.",
"zip-file-open-error": "Used as ZIP error message.\n\nSee also:\n* {{msg-mw|Zip-wrong-format}}\n* {{msg-mw|Zip-bad}}\n* {{msg-mw|Zip-unsupported}}", "zip-file-open-error": "Used as ZIP error message.\n\nSee also:\n* {{msg-mw|Zip-wrong-format}}\n* {{msg-mw|Zip-bad}}\n* {{msg-mw|Zip-unsupported}}",
"zip-wrong-format": "Used as ZIP error message.\n\nSee also:\n* {{msg-mw|Zip-file-open-error}}\n* {{msg-mw|Zip-bad}}\n* {{msg-mw|Zip-unsupported}}", "zip-wrong-format": "Used as ZIP error message.\n\nSee also:\n* {{msg-mw|Zip-file-open-error}}\n* {{msg-mw|Zip-bad}}\n* {{msg-mw|Zip-unsupported}}",
"zip-bad": "Used as ZIP error message.\n\nSee also:\n* {{msg-mw|Zip-file-open-error}}\n* {{msg-mw|Zip-wrong-format}}\n* {{msg-mw|Zip-unsupported}}", "zip-bad": "Used as ZIP error message.\n\nSee also:\n* {{msg-mw|Zip-file-open-error}}\n* {{msg-mw|Zip-wrong-format}}\n* {{msg-mw|Zip-unsupported}}",

View file

@ -139,13 +139,11 @@ class MySqlLockManagerTest extends MediaWikiUnitTestCase {
$expectedErrors = []; $expectedErrors = [];
if ( !( $params['expectedOK'] ?? true ) ) { if ( !( $params['expectedOK'] ?? true ) ) {
foreach ( $params['lockArgs'][0] as $path ) { $expectedErrors[] = [
$expectedErrors[] = [ 'type' => 'error',
'type' => 'error', 'message' => 'lockmanager-fail-conflict',
'message' => 'lockmanager-fail-acquirelock', 'params' => [],
'params' => [ $path ], ];
];
}
} }
$this->assertSame( $expectedErrors, $status->getErrors() ); $this->assertSame( $expectedErrors, $status->getErrors() );
$this->assertSame( !$expectedErrors, $status->isOK() ); $this->assertSame( !$expectedErrors, $status->isOK() );