From e0cf8b94bbbae5a04ecd1c581e03127bf79f60b9 Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Thu, 27 Jun 2019 19:59:04 -0700
Subject: [PATCH] rdbms: make DatabaseSqlite try harder to make named locks
 work

Also make unlock() calls to non-existing locks return false

Change-Id: I9f0cd63e85392d1fab1868638896492bbd8e6531
---
 .../libs/rdbms/database/DatabaseSqlite.php    | 35 ++++++++++++-------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/includes/libs/rdbms/database/DatabaseSqlite.php b/includes/libs/rdbms/database/DatabaseSqlite.php
index 17f12d327f..c1a3ab5829 100644
--- a/includes/libs/rdbms/database/DatabaseSqlite.php
+++ b/includes/libs/rdbms/database/DatabaseSqlite.php
@@ -23,6 +23,7 @@
  */
 namespace Wikimedia\Rdbms;
 
+use NullLockManager;
 use PDO;
 use PDOException;
 use Exception;
@@ -39,7 +40,7 @@ class DatabaseSqlite extends Database {
 	/** @var bool Whether full text is enabled */
 	private static $fulltextEnabled = null;
 
-	/** @var string Directory */
+	/** @var string|null Directory */
 	protected $dbDir;
 	/** @var string File name for SQLite database file */
 	protected $dbPath;
@@ -91,10 +92,16 @@ class DatabaseSqlite extends Database {
 			$this->queryLogger->warning( "Invalid SQLite transaction mode provided." );
 		}
 
-		$this->lockMgr = new FSLockManager( [
-			'domain' => $lockDomain,
-			'lockDirectory' => "{$this->dbDir}/locks"
-		] );
+		if ( $this->hasProcessMemoryPath() ) {
+			$this->lockMgr = new NullLockManager( [ 'domain' => $lockDomain ] );
+		} else {
+			$this->lockMgr = new FSLockManager( [
+				'domain' => $lockDomain,
+				'lockDirectory' => is_string( $this->dbDir )
+					? "{$this->dbDir}/locks"
+					: dirname( $this->dbPath ) . "/locks"
+			] );
+		}
 
 		parent::__construct( $p );
 	}
@@ -186,7 +193,7 @@ class DatabaseSqlite extends Database {
 	 * @throws DBConnectionError
 	 */
 	protected function openFile( $fileName, $dbName, $tablePrefix ) {
-		if ( !$this->hasMemoryPath() && !is_readable( $fileName ) ) {
+		if ( !$this->hasProcessMemoryPath() && !is_readable( $fileName ) ) {
 			$error = "SQLite database file not readable";
 			$this->connLogger->error(
 				"Error connecting to {db_server}: {error}",
@@ -772,13 +779,13 @@ class DatabaseSqlite extends Database {
 	}
 
 	public function serverIsReadOnly() {
-		return ( !$this->hasMemoryPath() && !is_writable( $this->dbPath ) );
+		return ( !$this->hasProcessMemoryPath() && !is_writable( $this->dbPath ) );
 	}
 
 	/**
 	 * @return bool
 	 */
-	private function hasMemoryPath() {
+	private function hasProcessMemoryPath() {
 		return ( strpos( $this->dbPath, ':memory:' ) === 0 );
 	}
 
@@ -974,17 +981,19 @@ class DatabaseSqlite extends Database {
 	}
 
 	public function lock( $lockName, $method, $timeout = 5 ) {
-		if ( !is_dir( "{$this->dbDir}/locks" ) ) { // create dir as needed
-			if ( !is_writable( $this->dbDir ) || !mkdir( "{$this->dbDir}/locks" ) ) {
-				throw new DBError( $this, "Cannot create directory \"{$this->dbDir}/locks\"." );
-			}
+		// Give better error message for permission problems than just returning false
+		if (
+			!is_dir( "{$this->dbDir}/locks" ) &&
+			( !is_writable( $this->dbDir ) || !mkdir( "{$this->dbDir}/locks" ) )
+		) {
+			throw new DBError( $this, "Cannot create directory \"{$this->dbDir}/locks\"." );
 		}
 
 		return $this->lockMgr->lock( [ $lockName ], LockManager::LOCK_EX, $timeout )->isOK();
 	}
 
 	public function unlock( $lockName, $method ) {
-		return $this->lockMgr->unlock( [ $lockName ], LockManager::LOCK_EX )->isOK();
+		return $this->lockMgr->unlock( [ $lockName ], LockManager::LOCK_EX )->isGood();
 	}
 
 	/**
-- 
2.20.1