Issue #2664302 by alexpott: Semaphore table is only used by a core service but it depends on system install

core/lib/Drupal/Core/Lock/DatabaseLockBackend.php

@@ -9,6 +9,7 @@ namespace Drupal\Core\Lock;
 
 use Drupal\Core\Database\Connection;
 use Drupal\Core\Database\IntegrityConstraintViolationException;
+use Drupal\Core\Database\SchemaObjectExistsException;
 
 /**
  * Defines the database lock backend. This is the default backend in Drupal.
@@ -17,6 +18,11 @@ use Drupal\Core\Database\IntegrityConstraintViolationException;
  */
 class DatabaseLockBackend extends LockBackendAbstract {
 
+  /**
+   * The database table name.
+   */
+  const TABLE_NAME = 'semaphore';
+
   /**
    * The database connection.
    *
@@ -84,6 +90,16 @@ class DatabaseLockBackend extends LockBackendAbstract {
           // the offending row from the database table in case it has expired.
           $retry = $retry ? FALSE : $this->lockMayBeAvailable($name);
         }
+        catch (\Exception $e) {
+          // Create the semaphore table if it does not exist and retry.
+          if ($this->ensureTableExists()) {
+            // Retry only once.
+            $retry = !$retry;
+          }
+          else {
+            throw $e;
+          }
+        }
         // We only retry in case the first attempt failed, but we then broke
         // an expired lock.
       } while ($retry);
@@ -95,7 +111,14 @@ class DatabaseLockBackend extends LockBackendAbstract {
    * {@inheritdoc}
    */
   public function lockMayBeAvailable($name) {
-    $lock = $this->database->query('SELECT expire, value FROM {semaphore} WHERE name = :name', array(':name' => $name))->fetchAssoc();
+    try {
+      $lock = $this->database->query('SELECT expire, value FROM {semaphore} WHERE name = :name', array(':name' => $name))->fetchAssoc();
+    }
+    catch (\Exception $e) {
+      $this->catchException($e);
+      // If the table does not exist yet then the lock may be available.
+      $lock = FALSE;
+    }
     if (!$lock) {
       return TRUE;
     }
@@ -119,10 +142,15 @@ class DatabaseLockBackend extends LockBackendAbstract {
    */
   public function release($name) {
     unset($this->locks[$name]);
-    $this->database->delete('semaphore')
-      ->condition('name', $name)
-      ->condition('value', $this->getLockId())
-      ->execute();
+    try {
+      $this->database->delete('semaphore')
+        ->condition('name', $name)
+        ->condition('value', $this->getLockId())
+        ->execute();
+    }
+    catch (\Exception $e) {
+      $this->catchException($e);
+    }
   }
 
   /**
@@ -140,4 +168,80 @@ class DatabaseLockBackend extends LockBackendAbstract {
         ->execute();
     }
   }
+
+  /**
+   * Check if the semaphore table exists and create it if not.
+   */
+  protected function ensureTableExists() {
+    try {
+      $database_schema = $this->database->schema();
+      if (!$database_schema->tableExists(static::TABLE_NAME)) {
+        $schema_definition = $this->schemaDefinition();
+        $database_schema->createTable(static::TABLE_NAME, $schema_definition);
+        return TRUE;
+      }
+    }
+    // If another process has already created the semaphore table, attempting to
+    // recreate it will throw an exception. In this case just catch the
+    // exception and do nothing.
+    catch (SchemaObjectExistsException $e) {
+      return TRUE;
+    }
+    return FALSE;
+  }
+
+  /**
+   * Act on an exception when semaphore might be stale.
+   *
+   * If the table does not yet exist, that's fine, but if the table exists and
+   * yet the query failed, then the semaphore is stale and the exception needs
+   * to propagate.
+   *
+   * @param $e
+   *   The exception.
+   *
+   * @throws \Exception
+   */
+  protected function catchException(\Exception $e) {
+    if ($this->database->schema()->tableExists(static::TABLE_NAME)) {
+      throw $e;
+    }
+  }
+
+  /**
+   * Defines the schema for the semaphore table.
+   */
+  public function schemaDefinition() {
+    return [
+      'description' => 'Table for holding semaphores, locks, flags, etc. that cannot be stored as state since they must not be cached.',
+      'fields' => [
+        'name' => [
+          'description' => 'Primary Key: Unique name.',
+          'type' => 'varchar_ascii',
+          'length' => 255,
+          'not null' => TRUE,
+          'default' => ''
+        ],
+        'value' => [
+          'description' => 'A value for the semaphore.',
+          'type' => 'varchar_ascii',
+          'length' => 255,
+          'not null' => TRUE,
+          'default' => ''
+        ],
+        'expire' => [
+          'description' => 'A Unix timestamp with microseconds indicating when the semaphore should expire.',
+          'type' => 'float',
+          'size' => 'big',
+          'not null' => TRUE
+        ],
+      ],
+      'indexes' => [
+        'value' => ['value'],
+        'expire' => ['expire'],
+      ],
+      'primary key' => ['name'],
+    ];
+  }
+
 }

core/modules/system/src/Tests/Lock/LockUnitTest.php

@@ -24,17 +24,9 @@ class LockUnitTest extends KernelTestBase {
    */
   protected $lock;
 
-  /**
-   * Modules to enable.
-   *
-   * @var array
-   */
-  public static $modules = array('system');
-
   protected function setUp() {
     parent::setUp();
     $this->lock = new DatabaseLockBackend($this->container->get('database'));
-    $this->installSchema('system', 'semaphore');
   }
 
   /**

core/modules/system/src/Tests/Update/DbDumpTest.php

@@ -90,7 +90,6 @@ class DbDumpTest extends KernelTestBase {
     // Create some schemas so our export contains tables.
     $this->installSchema('system', [
       'key_value_expire',
-      'semaphore',
       'sessions',
       'url_alias',
     ]);
@@ -131,7 +130,6 @@ class DbDumpTest extends KernelTestBase {
       'key_value_expire',
       'menu_link_content',
       'menu_link_content_data',
-      'semaphore',
       'sequences',
       'sessions',
       'url_alias',

core/modules/system/system.install

@@ -968,37 +968,6 @@ function system_schema() {
     'primary key' => array('name'),
   );
 
-  $schema['semaphore'] = array(
-    'description' => 'Table for holding semaphores, locks, flags, etc. that cannot be stored as state since they must not be cached.',
-    'fields' => array(
-      'name' => array(
-        'description' => 'Primary Key: Unique name.',
-        'type' => 'varchar_ascii',
-        'length' => 255,
-        'not null' => TRUE,
-        'default' => ''
-      ),
-      'value' => array(
-        'description' => 'A value for the semaphore.',
-        'type' => 'varchar_ascii',
-        'length' => 255,
-        'not null' => TRUE,
-        'default' => ''
-      ),
-      'expire' => array(
-        'description' => 'A Unix timestamp with microseconds indicating when the semaphore should expire.',
-        'type' => 'float',
-        'size' => 'big',
-        'not null' => TRUE
-      ),
-    ),
-    'indexes' => array(
-      'value' => array('value'),
-      'expire' => array('expire'),
-    ),
-    'primary key' => array('name'),
-  );
-
   $schema['sequences'] = array(
     'description' => 'Stores IDs.',
     'fields' => array(

core/modules/system/tests/src/Kernel/Scripts/DbImportCommandTest.php

@@ -42,7 +42,6 @@ class DbImportCommandTest extends KernelTestBase {
     'key_value_expire',
     'menu_link_content',
     'menu_link_content_data',
-    'semaphore',
     'sessions',
     'url_alias',
     'user__roles',

core/modules/user/src/Tests/TempStoreDatabaseTest.php

@@ -61,7 +61,7 @@ class TempStoreDatabaseTest extends KernelTestBase {
 
     // Install system tables to test the key/value storage without installing a
     // full Drupal environment.
-    $this->installSchema('system', array('semaphore', 'key_value_expire'));
+    $this->installSchema('system', array('key_value_expire'));
 
     // Create several objects for testing.
     for ($i = 0; $i <= 3; $i++) {

core/tests/Drupal/KernelTests/Core/Cache/CacheCollectorTest.php

@@ -17,19 +17,6 @@ use Symfony\Component\DependencyInjection\Reference;
  */
 class CacheCollectorTest extends KernelTestBase {
 
-  /**
-   * {@inheritdoc}
-   */
-  public static $modules = ['system'];
-
-  /**
-   * {@inheritdoc}
-   */
-  protected function setUp() {
-    parent::setUp();
-    $this->installSchema('system', ['semaphore']);
-  }
-
   /**
    * {@inheritdoc}
    */