Issue #2160091 by alexpott, dawehner, damiankloip, swentel, sun, moshe weitzman, xjm, joelpittet, charginghawk: drupal_rebuild() rebuilds container twice, since drupal_flush_all_caches() also rebuilds it

core/includes/common.inc

@@ -12,6 +12,7 @@ use Drupal\Component\Utility\Bytes;
 use Drupal\Component\Utility\Html;
 use Drupal\Component\Utility\SortArray;
 use Drupal\Core\Cache\Cache;
+use Drupal\Core\DrupalKernel;
 use Drupal\Core\StringTranslation\TranslatableMarkup;
 
 /**
@@ -460,10 +461,11 @@ function show(&$element) {
 }
 
 /**
- * Flushes all persistent caches, resets all variables, and rebuilds all data structures.
+ * Rebuilds the container, flushes all persistent caches, resets all variables, and rebuilds all data structures.
  *
  * At times, it is necessary to re-initialize the entire system to account for
  * changed or new code. This function:
+ * - Rebuilds the container if $kernel is not passed in.
  * - Clears all persistent caches:
  *   - The bootstrap cache bin containing base system, module system, and theme
  *     system information.
@@ -483,6 +485,10 @@ function show(&$element) {
  *   - The 'active' status of fields is refreshed.
  * - Rebuilds the menu router.
  *
+ * It's discouraged to call this during a regular page request.
+ * If you call this function in tests, every code afterwards should use the new
+ * container.
+ *
  * This means the entire system is reset so all caches and static variables are
  * effectively empty. After that is guaranteed, information about the currently
  * active code is updated, and rebuild operations are successively called in
@@ -512,12 +518,19 @@ function show(&$element) {
  *   cache though.)
  * @todo Add a global lock to ensure that caches are not primed in concurrent
  *   requests.
+ *
+ * @param \Drupal\Core\DrupalKernel|array $kernel
+ *   (optional) The Drupal Kernel. It is the caller's responsibility to rebuild
+ *   the container if this is passed in. Sometimes drupal_flush_all_caches is
+ *   used as a batch operation so $kernel will be an array, in this instance it
+ *   will be treated as if it it NULL.
  */
-function drupal_flush_all_caches() {
+function drupal_flush_all_caches($kernel = NULL) {
+  // This is executed based on old/previously known information if $kernel is
+  // not passed in, which is sufficient, since new extensions cannot have any
+  // primed caches yet.
   $module_handler = \Drupal::moduleHandler();
   // Flush all persistent caches.
-  // This is executed based on old/previously known information, which is
-  // sufficient, since new extensions cannot have any primed caches yet.
   $module_handler->invokeAll('cache_flush');
   foreach (Cache::getBins() as $service_id => $cache_backend) {
     $cache_backend->deleteAll();
@@ -531,43 +544,26 @@ function drupal_flush_all_caches() {
   // Reset all static caches.
   drupal_static_reset();
 
-  // Invalidate the container.
-  \Drupal::service('kernel')->invalidateContainer();
-
   // Wipe the Twig PHP Storage cache.
   \Drupal::service('twig')->invalidate();
 
-  // Rebuild module and theme data.
-  $module_data = \Drupal::service('extension.list.module')->reset()->getList();
-  /** @var \Drupal\Core\Extension\ThemeHandlerInterface $theme_handler */
-  $theme_handler = \Drupal::service('theme_handler');
-  $theme_handler->refreshInfo();
+  // Rebuild theme data that is stored in state.
+  \Drupal::service('theme_handler')->refreshInfo();
   // In case the active theme gets requested later in the same request we need
   // to reset the theme manager.
   \Drupal::theme()->resetActiveTheme();
 
-  // Rebuild and reboot a new kernel. A simple DrupalKernel reboot is not
-  // sufficient, since the list of enabled modules might have been adjusted
-  // above due to changed code.
-  $files = [];
-  $modules = [];
-  foreach ($module_data as $name => $extension) {
-    if ($extension->status) {
-      $files[$name] = $extension;
-      $modules[$name] = $extension->weight;
-    }
+  if (!$kernel instanceof DrupalKernel) {
+    $kernel = \Drupal::service('kernel');
+    $kernel->invalidateContainer();
+    $kernel->rebuildContainer();
   }
-  $modules = module_config_sort($modules);
-  \Drupal::service('kernel')->updateModules($modules, $files);
-  // New container, new module handler.
-  $module_handler = \Drupal::moduleHandler();
 
-  // Ensure that all modules that are currently supposed to be enabled are
-  // actually loaded.
-  $module_handler->loadAll();
+  // Rebuild module data that is stored in state.
+  \Drupal::service('extension.list.module')->reset();
 
   // Rebuild all information based on new module data.
-  $module_handler->invokeAll('rebuild');
+  \Drupal::moduleHandler()->invokeAll('rebuild');
 
   // Clear all plugin caches.
   \Drupal::service('plugin.cache_clearer')->clearCachedDefinitions();

core/includes/utility.inc

@@ -5,8 +5,6 @@
  * Miscellaneous functions.
  */
 
-use Drupal\Core\PhpStorage\PhpStorageFactory;
-use Drupal\Core\Cache\Cache;
 use Drupal\Core\DrupalKernel;
 use Symfony\Component\HttpFoundation\Request;
 
@@ -28,12 +26,11 @@ function drupal_rebuild($class_loader, Request $request) {
   restore_error_handler();
   restore_exception_handler();
 
-  // Force kernel to rebuild php cache.
-  PhpStorageFactory::get('twig')->deleteAll();
-
+  // Invalidate the container.
   // Bootstrap up to where caches exist and clear them.
   $kernel = new DrupalKernel('prod', $class_loader);
   $kernel->setSitePath(DrupalKernel::findSitePath($request));
+  $kernel->invalidateContainer();
   $kernel->boot();
   $kernel->preHandle($request);
   // Ensure our request includes the session if appropriate.
@@ -41,18 +38,11 @@ function drupal_rebuild($class_loader, Request $request) {
     $request->setSession($kernel->getContainer()->get('session'));
   }
 
-  // Invalidate the container.
-  $kernel->invalidateContainer();
-
-  foreach (Cache::getBins() as $bin) {
-    $bin->deleteAll();
-  }
+  drupal_flush_all_caches($kernel);
 
   // Disable recording of cached pages.
   \Drupal::service('page_cache_kill_switch')->trigger();
 
-  drupal_flush_all_caches();
-
   // Restore Drupal's error and exception handlers.
   // @see \Drupal\Core\DrupalKernel::boot()
   set_error_handler('_drupal_error_handler');

core/modules/system/tests/modules/container_rebuild_test/container_rebuild_test.info.yml

@@ -0,0 +1,5 @@
+name: 'Service Provider test'
+type: module
+description: 'Support module for service provider testing.'
+package: Testing
+version: VERSION

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

@@ -0,0 +1,6 @@
+container_rebuild_test.module_path:
+  path: '/container_rebuild_test/{module}/{function}'
+  defaults:
+    _controller: '\Drupal\container_rebuild_test\TestController::showModuleInfo'
+  requirements:
+    _access: 'TRUE'

core/modules/system/tests/modules/container_rebuild_test/src/ContainerRebuildTestServiceProvider.php

@@ -0,0 +1,18 @@
+<?php
+
+namespace Drupal\container_rebuild_test;
+
+use Drupal\Core\DependencyInjection\ContainerBuilder;
+use Drupal\Core\DependencyInjection\ServiceModifierInterface;
+
+class ContainerRebuildTestServiceProvider implements ServiceModifierInterface {
+
+  /**
+   * {@inheritdoc}
+   */
+  public function alter(ContainerBuilder $container) {
+    $count = $container->get('state')->get('container_rebuild_test.count', 0);
+    $container->get('state')->set('container_rebuild_test.count', ++$count);
+  }
+
+}

core/modules/system/tests/modules/container_rebuild_test/src/TestController.php

@@ -0,0 +1,37 @@
+<?php
+
+namespace Drupal\container_rebuild_test;
+
+use Drupal\Core\Controller\ControllerBase;
+
+class TestController extends ControllerBase {
+
+  /**
+   * Displays the path to a module.
+   *
+   * @param string $module
+   *   The module name.
+   * @param string $function
+   *   The function to check if it exists.
+   *
+   * @return string[]
+   *   A render array.
+   */
+  public function showModuleInfo(string $module, string $function) {
+    $module_handler = \Drupal::moduleHandler();
+    $module_message = $module . ': ';
+    if ($module_handler->moduleExists($module)) {
+      $module_message .= \Drupal::moduleHandler()->getModule($module)->getPath();
+    }
+    else {
+      $module_message .= 'not installed';
+    }
+    $function_message = $function . ': ' . var_export(function_exists($function), TRUE);
+
+    return [
+      '#theme' => 'item_list',
+      '#items' => [$module_message, $function_message],
+    ];
+  }
+
+}

core/modules/system/tests/src/Functional/UpdateSystem/RebuildScriptTest.php

@@ -4,6 +4,7 @@ namespace Drupal\Tests\system\Functional\UpdateSystem;
 
 use Drupal\Core\Url;
 use Drupal\Tests\BrowserTestBase;
+use Symfony\Component\Filesystem\Filesystem;
 
 /**
  * Tests the rebuild script access and functionality.
@@ -12,6 +13,11 @@ use Drupal\Tests\BrowserTestBase;
  */
 class RebuildScriptTest extends BrowserTestBase {
 
+  /**
+   * {@inheritdoc}
+   */
+  protected static $modules = ['module_test', 'container_rebuild_test'];
+
   /**
    * {@inheritdoc}
    */
@@ -37,9 +43,60 @@ class RebuildScriptTest extends BrowserTestBase {
     $this->rebuildAll();
 
     $cache->set('rebuild_test', TRUE);
+    \Drupal::state()->set('container_rebuild_test.count', 0);
     $this->drupalGet(Url::fromUri('base:core/rebuild.php'));
     $this->assertSession()->addressEquals(new Url('<front>'));
     $this->assertFalse($cache->get('rebuild_test'));
+    $this->refreshVariables();
+    $this->assertSame(1, \Drupal::state()->get('container_rebuild_test.count', 0));
+    $this->drupalGet('/container_rebuild_test/module_test/module_test_system_info_alter');
+    $this->assertSession()->pageTextContains('module_test: core/modules/system/tests/modules/module_test');
+    $this->assertSession()->pageTextContains('module_test_system_info_alter: true');
+
+    // Move a module to ensure it does not break the rebuild.
+    $file_system = new Filesystem();
+    $file_system->mirror('core/modules/system/tests/modules/module_test', $this->siteDirectory . '/modules/module_test');
+    \Drupal::state()->set('container_rebuild_test.count', 0);
+    $this->drupalGet(Url::fromUri('base:core/rebuild.php'));
+    $this->assertSession()->addressEquals(new Url('<front>'));
+    $this->refreshVariables();
+    $this->assertSame(1, \Drupal::state()->get('container_rebuild_test.count', 0));
+    $this->drupalGet('/container_rebuild_test/module_test/module_test_system_info_alter');
+    $this->assertSession()->pageTextContains('module_test: ' . $this->siteDirectory . '/modules/module_test');
+    $this->assertSession()->pageTextContains('module_test_system_info_alter: true');
+
+    // Disable a module by writing to the core.extension list.
+    $this->config('core.extension')->clear('module.module_test')->save();
+    \Drupal::state()->set('container_rebuild_test.count', 0);
+    $this->drupalGet(Url::fromUri('base:core/rebuild.php'));
+    $this->assertSession()->addressEquals(new Url('<front>'));
+    $this->refreshVariables();
+    $this->assertSame(1, \Drupal::state()->get('container_rebuild_test.count', 0));
+    $this->drupalGet('/container_rebuild_test/module_test/module_test_system_info_alter');
+    $this->assertSession()->pageTextContains('module_test: not installed');
+    $this->assertSession()->pageTextContains('module_test_system_info_alter: false');
+
+    // Enable a module by writing to the core.extension list.
+    $modules = $this->config('core.extension')->get('module');
+    $modules['module_test'] = 0;
+    $this->config('core.extension')->set('module', module_config_sort($modules))->save();
+    \Drupal::state()->set('container_rebuild_test.count', 0);
+    $this->drupalGet(Url::fromUri('base:core/rebuild.php'));
+    $this->assertSession()->addressEquals(new Url('<front>'));
+    $this->refreshVariables();
+    $this->assertSame(1, \Drupal::state()->get('container_rebuild_test.count', 0));
+    $this->drupalGet('/container_rebuild_test/module_test/module_test_system_info_alter');
+    $this->assertSession()->pageTextContains('module_test: ' . $this->siteDirectory . '/modules/module_test');
+    $this->assertSession()->pageTextContains('module_test_system_info_alter: true');
+
+    // Test how many container rebuild occur when there is no cached container.
+    \Drupal::state()->set('container_rebuild_test.count', 0);
+    \Drupal::service('kernel')->invalidateContainer();
+    $this->drupalGet(Url::fromUri('base:core/rebuild.php'));
+    $this->assertSession()->addressEquals(new Url('<front>'));
+    $this->assertFalse($cache->get('rebuild_test'));
+    $this->refreshVariables();
+    $this->assertSame(1, \Drupal::state()->get('container_rebuild_test.count', 0));
   }
 
 }

core/tests/Drupal/KernelTests/Core/Common/DrupalFlushAllCachesTest.php

@@ -2,6 +2,7 @@
 
 namespace Drupal\KernelTests\Core\Common;
 
+use Drupal\Core\DependencyInjection\ContainerBuilder;
 use Drupal\KernelTests\KernelTestBase;
 
 /**
@@ -10,6 +11,13 @@ use Drupal\KernelTests\KernelTestBase;
  */
 class DrupalFlushAllCachesTest extends KernelTestBase {
 
+  /**
+   * Stores the number of container builds.
+   *
+   * @var int
+   */
+  protected $containerBuilds = 0;
+
   /**
    * {@inheritdoc}
    */
@@ -19,14 +27,30 @@ class DrupalFlushAllCachesTest extends KernelTestBase {
    * Tests that drupal_flush_all_caches() uses core.extension properly.
    */
   public function testDrupalFlushAllCachesModuleList() {
+    $this->assertFalse(function_exists('system_test_help'));
     $core_extension = \Drupal::configFactory()->getEditable('core.extension');
     $module = $core_extension->get('module');
     $module['system_test'] = -10;
     $core_extension->set('module', module_config_sort($module))->save();
-
+    $this->containerBuilds = 0;
     drupal_flush_all_caches();
-
     $this->assertSame(['system_test', 'system'], array_keys($this->container->getParameter('container.modules')));
+    $this->assertSame(1, $this->containerBuilds);
+    $this->assertTrue(function_exists('system_test_help'));
+
+    $core_extension->clear('module.system_test')->save();
+    $this->containerBuilds = 0;
+    drupal_flush_all_caches();
+    $this->assertSame(['system'], array_keys($this->container->getParameter('container.modules')));
+    $this->assertSame(1, $this->containerBuilds);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function register(ContainerBuilder $container) {
+    parent::register($container);
+    $this->containerBuilds++;
   }
 
 }