Uninstalling modules containing cache contexts or #post_render_cache callbacks may break Drupal if they are cached somewhere

Bumping to critical for now, we can always bump it down later but this feels like unrecoverable site-breakage if it happens (at least not without drush).

There's also the possibility that a module code update renames or removes one of these as well as uninstall. Suggested trigger_error() in the other issue - we may need to make this a non-fatal.

Yes we can't detect that, that's why I suggested non-fatal.

So:

1. Validation - trigger_error() instead of throwing an exception.
2. Before calling the service check if it exists, if it doesn't, trigger_error() and treat the cache item as invalid.

Something like this.

I like #6 and using trigger_error, once we have an assertion and not do the check at run-time at all, this will be a similar "fault".

Note that uninstalling modules explicitly clears all caches I don't think you will be able to write tests for that scenario, it shouldn't happen.

Yes, where it could happen is when modules are updated/changed, but then you're supposed to run a cache clear/update.php anyway, anything could be broken then.

Not sure this is critical, I'd say demote unless we have a test case that it can happen on an easy to reproduce scenario?

Pretty sure :)

#2359369: Render cache is not cleared when module is uninstalled

    // Flush all persistent caches.
    // Any cache entry might implicitly depend on the uninstalled modules,
    // so clear all of them explicitly.
    $this->moduleHandler->invokeAll('cache_flush');
    foreach (Cache::getBins() as $service_id => $cache_backend) {
      $cache_backend->deleteAll();
    }

at the end of uninstall().

ModuleInstaller::uninstall() has this at the end

    // Flush all persistent caches.
    // Any cache entry might implicitly depend on the uninstalled modules,
    // so clear all of them explicitly.
    $this->moduleHandler->invokeAll('cache_flush');
    foreach (Cache::getBins() as $service_id => $cache_backend) {
      $cache_backend->deleteAll();
    }

I think #2359369: Render cache is not cleared when module is uninstalled restored this.

Haha, that code is broken :) But the same exists in drupal_flush_all_caches(), so that is broken too?
This should use Cache::getBins().

I'm stupid, that should work fine. Being able to read code would be a huge advantage.

#12: Bad copy paste searching for that issue. Also edited it before I saw your comment. Gotta love multiple cross posts...

So it this an issue or not? Seems to be consensus that uninstall == cache clear == problem solved?