Issue #2205527 by cilefen, alexpott, martin107, Xen, Gaelan, clintrandall777@gmail.com: Fixed Move configuration import lock to lock.persistent service since a lock can not exist beyond a single request.

core/core.services.yml

@@ -450,6 +450,9 @@ services:
     arguments: ['@database']
     tags:
       - { name: backend_overridable }
+  lock.persistent:
+    class: Drupal\Core\Lock\PersistentDatabaseLockBackend
+    arguments: ['@database']
   router.request_context:
     class: Drupal\Core\Routing\RequestContext
     tags:

core/lib/Drupal/Core/Config/ConfigImporter.php

@@ -44,7 +44,7 @@ class ConfigImporter {
   /**
    * The name used to identify the lock.
    */
-  const LOCK_ID = 'config_importer';
+  const LOCK_NAME = 'config_importer';
 
   /**
    * The storage comparer used to discover configuration changes.
@@ -512,9 +512,9 @@ class ConfigImporter {
     // Ensure that the changes have been validated.
     $this->validate();
 
-    if (!$this->lock->acquire(static::LOCK_ID)) {
+    if (!$this->lock->acquire(static::LOCK_NAME)) {
       // Another process is synchronizing configuration.
-      throw new ConfigImporterException(sprintf('%s is already importing', static::LOCK_ID));
+      throw new ConfigImporterException(sprintf('%s is already importing', static::LOCK_NAME));
     }
 
     $sync_steps = array();
@@ -611,7 +611,7 @@ class ConfigImporter {
   protected function finish(array &$context) {
     $this->eventDispatcher->dispatch(ConfigEvents::IMPORT, new ConfigImporterEvent($this));
     // The import is now complete.
-    $this->lock->release(static::LOCK_ID);
+    $this->lock->release(static::LOCK_NAME);
     $this->reset();
     $context['message'] = t('Finalizing configuration synchronization.');
     $context['finished'] = 1;
@@ -996,7 +996,7 @@ class ConfigImporter {
    *   TRUE if an import is already running, FALSE if not.
    */
   public function alreadyImporting() {
-    return !$this->lock->lockMayBeAvailable(static::LOCK_ID);
+    return !$this->lock->lockMayBeAvailable(static::LOCK_NAME);
   }
 
   /**
@@ -1007,13 +1007,13 @@ class ConfigImporter {
    * keep the services used by the importer in sync.
    */
   protected function reInjectMe() {
-    $this->eventDispatcher = \Drupal::service('event_dispatcher');
+    $this->_serviceIds = array();
-    $this->configManager = \Drupal::service('config.manager');
+    $vars = get_object_vars($this);
-    $this->lock = \Drupal::lock();
+    foreach ($vars as $key => $value) {
-    $this->typedConfigManager = \Drupal::service('config.typed');
+      if (is_object($value) && isset($value->_serviceId)) {
-    $this->moduleHandler = \Drupal::moduleHandler();
+        $this->$key = \Drupal::service($value->_serviceId);
-    $this->themeHandler = \Drupal::service('theme_handler');
+      }
-    $this->stringTranslation = \Drupal::service('string_translation');
+    }
   }
 
 }

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

@@ -0,0 +1,36 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Lock\PersistentDatabaseLockBackend.
+ */
+
+namespace Drupal\Core\Lock;
+
+use Drupal\Core\Database\Connection;
+
+/**
+ * Defines the persistent database lock backend. This backend is global for this
+ * Drupal installation.
+ *
+ * @ingroup lock
+ */
+class PersistentDatabaseLockBackend extends DatabaseLockBackend {
+
+  /**
+   * Constructs a new PersistentDatabaseLockBackend.
+   *
+   * @param \Drupal\Core\Database\Connection $database
+   *   The database connection.
+   */
+  public function __construct(Connection $database) {
+    // Do not call the parent constructor to avoid registering a shutdown
+    // function that releases all the locks at the end of a request.
+    $this->database = $database;
+    // Set the lockId to a fixed string to make the lock ID the same across
+    // multiple requests. The lock ID is used as a page token to relate all the
+    // locks set during a request to each other.
+    // @see \Drupal\Core\Lock\LockBackendInterface::getLockId()
+    $this->lockId = 'persistent';
+  }
+}

core/modules/config/src/Form/ConfigSync.php

@@ -134,7 +134,7 @@ class ConfigSync extends FormBase {
       $container->get('config.storage.staging'),
       $container->get('config.storage'),
       $container->get('config.storage.snapshot'),
-      $container->get('lock'),
+      $container->get('lock.persistent'),
       $container->get('event_dispatcher'),
       $container->get('config.manager'),
       $container->get('config.typed'),

core/modules/config/src/Tests/ConfigImportRenameValidationTest.php

@@ -55,7 +55,7 @@ class ConfigImportRenameValidationTest extends DrupalUnitTestBase {
       $storage_comparer->createChangelist(),
       $this->container->get('event_dispatcher'),
       $this->container->get('config.manager'),
-      $this->container->get('lock'),
+      $this->container->get('lock.persistent'),
       $this->container->get('config.typed'),
       $this->container->get('module_handler'),
       $this->container->get('theme_handler'),

core/modules/config/src/Tests/ConfigImportUITest.php

@@ -228,14 +228,14 @@ class ConfigImportUITest extends WebTestBase {
 
     // Acquire a fake-lock on the import mechanism.
     $config_importer = $this->configImporter();
-    $this->container->get('lock')->acquire($config_importer::LOCK_ID);
+    $this->container->get('lock.persistent')->acquire($config_importer::LOCK_NAME);
 
     // Attempt to import configuration and verify that an error message appears.
     $this->drupalPostForm(NULL, array(), t('Import all'));
     $this->assertText(t('Another request may be synchronizing configuration already.'));
 
     // Release the lock, just to keep testing sane.
-    $this->container->get('lock')->release($config_importer::LOCK_ID);
+    $this->container->get('lock.persistent')->release($config_importer::LOCK_NAME);
 
     // Verify site name has not changed.
     $this->assertNotEqual($new_site_name, \Drupal::config('system.site')->get('name'));

core/modules/config/src/Tests/ConfigImporterTest.php

@@ -527,4 +527,3 @@ class ConfigImporterTest extends DrupalUnitTestBase {
     $this->assertEqual(count($logs), 0);
   }
 }
-

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

@@ -59,4 +59,29 @@ class LockFunctionalTest extends WebTestBase {
     $this->assertText($lock_acquired_exit, 'Lock acquired by the other request before exit.', 'Lock');
     $this->assertTrue($lock->acquire('system_test_lock_exit'), 'Lock acquired by this request after the other request exits.', 'Lock');
   }
+
+  /**
+   * Tests that the persistent lock is persisted between requests.
+   */
+  public function testPersistentLock() {
+    $persistent_lock = $this->container->get('lock.persistent');
+    // Get a persistent lock.
+    $this->drupalGet('system-test/lock-persist/lock1');
+    $this->assertText('TRUE: Lock successfully acquired in SystemTestController::lockPersist()');
+    // Ensure that a shutdown function has not released the lock.
+    $this->assertFalse($persistent_lock->lockMayBeAvailable('lock1'));
+    $this->drupalGet('system-test/lock-persist/lock1');
+    $this->assertText('FALSE: Lock not acquired in SystemTestController::lockPersist()');
+
+    // Get another persistent lock.
+    $this->drupalGet('system-test/lock-persist/lock2');
+    $this->assertText('TRUE: Lock successfully acquired in SystemTestController::lockPersist()');
+    $this->assertFalse($persistent_lock->lockMayBeAvailable('lock2'));
+
+    // Release the first lock and try getting it again.
+    $persistent_lock->release('lock1');
+    $this->drupalGet('system-test/lock-persist/lock1');
+    $this->assertText('TRUE: Lock successfully acquired in SystemTestController::lockPersist()');
+  }
+
 }

core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php

@@ -10,12 +10,38 @@ namespace Drupal\system_test\Controller;
 use Drupal\Core\Controller\ControllerBase;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
+use Drupal\Core\Lock\LockBackendInterface;
+use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
  * Controller routines for system_test routes.
  */
 class SystemTestController extends ControllerBase {
 
+  /**
+   * The persistent lock service.
+   *
+   * @var \Drupal\Core\Lock\LockBackendInterface
+   */
+  protected $persistentLock;
+
+  /**
+   * Constructs the SystemTestController.
+   *
+   * @param \Drupal\Core\Lock\LockBackendInterface $persistent_lock
+   *   The persistent lock service.
+   */
+  public function __construct(LockBackendInterface $persistent_lock) {
+    $this->persistentLock = $persistent_lock;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function create(ContainerInterface $container) {
+    return new static($container->get('lock.persistent'));
+  }
+
   /**
    * Tests main content fallback.
    *
@@ -56,6 +82,24 @@ class SystemTestController extends ControllerBase {
     return system_test_lock_exit();
   }
 
+  /**
+   * Creates a lock that will persist across requests.
+   *
+   * @param string $lock_name
+   *   The name of the persistent lock to acquire.
+   *
+   * @return string
+   *   The text to display.
+   */
+  public function lockPersist($lock_name) {
+    if ($this->persistentLock->acquire($lock_name)) {
+      return 'TRUE: Lock successfully acquired in SystemTestController::lockPersist()';
+    }
+    else {
+      return 'FALSE: Lock not acquired in SystemTestController::lockPersist()';
+    }
+  }
+
   /**
    * Set cache tag on on the returned render array.
    */

core/modules/system/tests/modules/system_test/system_test.routing.yml

@@ -53,6 +53,14 @@ system_test.lock_exit:
   requirements:
     _access: 'TRUE'
 
+system_test.lock_persist:
+  path: '/system-test/lock-persist/{lock_name}'
+  defaults:
+    _title: 'Persistent lock acquire'
+    _content: '\Drupal\system_test\Controller\SystemTestController::lockPersist'
+  requirements:
+    _access: 'TRUE'
+
 system_test.cache_tags_page:
   path: '/system-test/cache_tags_page'
   defaults: