Issue #2695109 by dpi, willzyx, Sam152, dawehner: Cache bins are not deleted when the module that declares them is uninstalled

core/lib/Drupal/Core/Extension/ModuleInstaller.php

@@ -505,34 +505,30 @@ class ModuleInstaller implements ModuleInstallerInterface {
    *   The name of the module for which to remove all registered cache bins.
    */
   protected function removeCacheBins($module) {
-    // Remove any cache bins defined by a module.
     $service_yaml_file = drupal_get_path('module', $module) . "/$module.services.yml";
-    if (file_exists($service_yaml_file)) {
+    if (!file_exists($service_yaml_file)) {
-      $definitions = Yaml::decode(file_get_contents($service_yaml_file));
+      return;
-      if (isset($definitions['services'])) {
+    }
-        foreach ($definitions['services'] as $id => $definition) {
+
-          if (isset($definition['tags'])) {
+    $definitions = Yaml::decode(file_get_contents($service_yaml_file));
-            foreach ($definition['tags'] as $tag) {
+
-              // This works for the default cache registration and even in some
+    $cache_bin_services = array_filter(
-              // cases when a non-default "super" factory is used. That should
+      isset($definitions['services']) ? $definitions['services'] : [],
-              // be extremely rare.
+      function ($definition) {
-              if ($tag['name'] == 'cache.bin' && isset($definition['factory_service']) && isset($definition['factory_method']) && !empty($definition['arguments'])) {
+        $tags = isset($definition['tags']) ? $definition['tags'] : [];
-                try {
+        foreach ($tags as $tag) {
-                  $factory = \Drupal::service($definition['factory_service']);
+          if (isset($tag['name']) && ($tag['name'] == 'cache.bin')) {
-                  if (method_exists($factory, $definition['factory_method'])) {
+            return TRUE;
-                    $backend = call_user_func_array([$factory, $definition['factory_method']], $definition['arguments']);
-                    if ($backend instanceof CacheBackendInterface) {
-                      $backend->removeBin();
-                    }
-                  }
-                }
-                catch (\Exception $e) {
-                  watchdog_exception('system', $e, 'Failed to remove cache bin defined by the service %id.', ['%id' => $id]);
-                }
-              }
-            }
           }
         }
+        return FALSE;
+      }
+    );
+
+    foreach (array_keys($cache_bin_services) as $service_id) {
+      $backend = $this->kernel->getContainer()->get($service_id);
+      if ($backend instanceof CacheBackendInterface) {
+        $backend->removeBin();
       }
     }
   }

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

@@ -0,0 +1,7 @@
+name: module cache bin tests
+type: module
+description: Test cache bins defined by modules.
+package: Testing
+version: VERSION
+core: 8.x
+hidden: true

core/modules/system/tests/modules/module_cachebin/module_cachebin.services.yml

@@ -0,0 +1,7 @@
+services:
+  module_cachebin.cache_bin:
+    class: Drupal\Core\Cache\CacheBackendInterface
+    tags:
+      - { name: cache.bin }
+    factory: cache.backend.database:get
+    arguments: [module_cachebin]

core/tests/Drupal/KernelTests/Core/Extension/ModuleInstallerTest.php

@@ -60,4 +60,29 @@ class ModuleInstallerTest extends KernelTestBase {
     $this->assertEquals(1, $modules['module_handler_test_multiple_child'], 'Weight of module_handler_test_multiple_child is set.');
   }
 
+  /**
+   * Tests cache bins defined by modules are removed when uninstalled.
+   *
+   * @covers ::removeCacheBins
+   */
+  public function testCacheBinCleanup() {
+    $schema = $this->container->get('database')->schema();
+    $table = 'cache_module_cachebin';
+
+    $module_installer = $this->container->get('module_installer');
+    $module_installer->install(['module_cachebin']);
+
+    // Prime the bin.
+    /** @var \Drupal\Core\Cache\CacheBackendInterface $cache_bin */
+    $cache_bin = $this->container->get('module_cachebin.cache_bin');
+    $cache_bin->set('foo', 'bar');
+
+    // A database backend is used so there is a convenient way check whether the
+    // backend is uninstalled.
+    $this->assertTrue($schema->tableExists($table));
+
+    $module_installer->uninstall(['module_cachebin']);
+    $this->assertFalse($schema->tableExists($table));
+  }
+
 }