Commit a2cac883 authored by catch's avatar catch
Browse files

fix: #3055983 Locks on SQLite - consistent fails on PHP 8.4 and PHP 8.5

By: alexpott
By: catch
By: mondrake
parent 7b7967df
Loading
Loading
Loading
Loading
Loading
+3 −5
Original line number Diff line number Diff line
@@ -10,7 +10,6 @@
use Drupal\Core\Database\StatementInterface;
use Drupal\Core\Database\SupportsTemporaryTablesInterface;
use Drupal\Core\Database\Transaction\TransactionManagerInterface;
use Pdo\Sqlite;

/**
 * SQLite implementation of \Drupal\Core\Database\Connection.
@@ -78,8 +77,7 @@ class Connection extends DatabaseConnection implements SupportsTemporaryTablesIn
   * Constructs a \Drupal\sqlite\Driver\Database\sqlite\Connection object.
   */
  public function __construct(\PDO $connection, array $connection_options) {
    // @phpstan-ignore class.notFound
    assert(\PHP_VERSION_ID >= 80400 ? $connection instanceof Sqlite : TRUE);
    assert(\PHP_VERSION_ID >= 80400 ? $connection instanceof SqliteConnection : TRUE);
    parent::__construct($connection, $connection_options);

    // Empty prefix means query the main database -- no need to attach anything.
@@ -111,8 +109,8 @@ public static function open(array &$connection_options = []) {

    try {
      if (\PHP_VERSION_ID >= 80400) {
        // @phpstan-ignore class.notFound
        $sqlite = new Sqlite('sqlite:' . $connection_options['database'], '', '', $connection_options['pdo']);
        // @phpstan-ignore new.noConstructor
        $sqlite = new SqliteConnection('sqlite:' . $connection_options['database'], '', '', $connection_options['pdo']);
      }
      else {
        $sqlite = new PDOConnection('sqlite:' . $connection_options['database'], '', '', $connection_options['pdo']);
+55 −0
Original line number Diff line number Diff line
<?php

namespace Drupal\sqlite\Driver\Database\sqlite;

use Pdo\Sqlite;

/**
 * SQLite-specific implementation of a PDO connection.
 *
 * SQLite does not implement row locks, so when it acquires a lock, it locks
 * the entire database. To improve performance, by default SQLite tries to
 * defer acquiring a write lock until the first write operation of a
 * transaction rather than when the transaction is started. Unfortunately, this
 * seems to be incompatible with how Drupal uses transactions, and frequently
 * leads to deadlocks.
 *
 * Therefore, this class overrides \PDO to begin transactions with a
 * BEGIN IMMEDIATE TRANSACTION statement, for which SQLite acquires the write
 * lock immediately. This can incur some performance cost in a high concurrency
 * environment: it adds approximately 5% to the time it takes to execute Drupal
 * core's entire test suite on DrupalCI, and it potentially could add more in a
 * higher concurrency environment. However, under high enough concurrency of a
 * Drupal application, SQLite isn't the best choice anyway, and a database
 * engine that implements row locking, such as MySQL or PostgreSQL, is more
 * suitable.
 *
 * Because of https://bugs.php.net/42766 we have to create such a transaction
 * manually which means we must also override commit() and rollback().
 *
 * @see https://www.drupal.org/project/drupal/issues/1120020
 */
class SqliteConnection extends Sqlite {

  /**
   * {@inheritdoc}
   */
  public function beginTransaction(): bool {
    return $this->exec('BEGIN IMMEDIATE TRANSACTION') !== FALSE;
  }

  /**
   * {@inheritdoc}
   */
  public function commit(): bool {
    return $this->exec('COMMIT') !== FALSE;
  }

  /**
   * {@inheritdoc}
   */
  public function rollBack(): bool {
    return $this->exec('ROLLBACK') !== FALSE;
  }

}
+3 −5
Original line number Diff line number Diff line
@@ -5,9 +5,9 @@
namespace Drupal\Tests\sqlite\Unit;

use Drupal\sqlite\Driver\Database\sqlite\Connection;
use Drupal\sqlite\Driver\Database\sqlite\SqliteConnection;
use Drupal\Tests\Core\Database\Stub\StubPDO;
use Drupal\Tests\UnitTestCase;
use Pdo\Sqlite;
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\Attributes\Group;
@@ -32,8 +32,7 @@ class ConnectionTest extends UnitTestCase {
   */
  #[DataProvider('providerCreateConnectionOptionsFromUrl')]
  public function testCreateConnectionOptionsFromUrl(string $url, string $expected): void {
    // @phpstan-ignore class.notFound
    $sqlite_connection = new Connection($this->createMock(\PHP_VERSION_ID >= 80400 ? Sqlite::class : StubPDO::class), []);
    $sqlite_connection = new Connection($this->createMock(\PHP_VERSION_ID >= 80400 ? SqliteConnection::class : StubPDO::class), []);
    $database = $sqlite_connection->createConnectionOptionsFromUrl($url, NULL);
    $this->assertEquals('sqlite', $database['driver']);
    $this->assertEquals($expected, $database['database']);
@@ -62,8 +61,7 @@ public static function providerCreateConnectionOptionsFromUrl(): array {
  public function testDeprecationOfRootInConnectionOptionsFromUrl(): void {
    $this->expectDeprecation('Passing the $root value to Drupal\sqlite\Driver\Database\sqlite\Connection::createConnectionOptionsFromUrl() is deprecated in drupal:11.2.0 and will be removed in drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3511287');
    $root = dirname(__DIR__, 8);
    // @phpstan-ignore class.notFound
    $sqlite_connection = new Connection($this->createMock(\PHP_VERSION_ID >= 80400 ? Sqlite::class : StubPDO::class), []);
    $sqlite_connection = new Connection($this->createMock(\PHP_VERSION_ID >= 80400 ? SqliteConnection::class : StubPDO::class), []);
    $database = $sqlite_connection->createConnectionOptionsFromUrl('sqlite://localhost/tmp/test', $root);
    $this->assertEquals('sqlite', $database['driver']);
    $this->assertEquals('tmp/test', $database['database']);
+2 −0
Original line number Diff line number Diff line
@@ -44,6 +44,8 @@ parameters:
    - modules/system/tests/modules/plugin_test/src/Plugin/plugin_test/fruit/ExtendingNonInstalledClass.php
    - modules/system/tests/modules/plugin_test/src/Plugin/plugin_test/custom_annotation/UsingNonInstalledTraitClass.php
    - modules/system/tests/modules/plugin_test/src/Plugin/plugin_test/custom_annotation/ExtendingNonInstalledClass.php
    # Needs to be skipped until PHPStan recognises the existence of Pdo\Sqlite
    - modules/sqlite/src/Driver/Database/sqlite/SqliteConnection.php

  ignoreErrors:
    # new static() is a best practice in Drupal, so we cannot fix that.