Derived discovery can result in rebuilding of out of date data (e.g. Block)

Created on 23 May 2017, about 7 years ago
Updated 27 October 2023, 8 months ago

Problem/Motivation

There is a design pattern that derivers statically cache the resulting derivatives (e.g. \Drupal\Component\Plugin\Derivative\DeriverBase::$derivatives). This can result in out of date derivatives data to be rebuild and cached, despite other things correctly calling clearing the plugin manager caches. The example we have in the Contacts module is:

  1. Install Contacts module
  2. Some stuff happens which involves getting definitions from the block manager (possibly importing page manager pages?). This primes the static caches.
  3. Facet config entities are imported. Facets (with patch in #2880411-6: Facets added via config don't clear the block cache ) clear the block plugin cached definitions expecting those blocks now to be found next time the definitions are rebuild.
  4. Some stuff happens which gets the definitions from the block manager (possible other page manager pages?). This results in using the existing static cache and returning the old, now stale, derivatives.
  5. Because the cache has been rebuilt, future requests use this already stale version of the cache.

This can be worked around in contrib by clearing the plugin.manager.block service from the container in addition to clearing the cached definitions.

Proposed resolution

Not sure what should actually be done, initial thoughts on possible solutions:

  • Stop statically caching... If we attempt to get derivatives twice in a request that will be because the cache has been cleared and we need to get them fresh. I don't know if this is just a documentation/change record and then fix in contrib or if there are BC issues (e.g. is \Drupal\Component\Plugin\Derivative\DeriverBase public API or internal)...
  • Don't store the derivers as part of \Drupal\Component\Plugin\Discovery\DerivativeDiscoveryDecorator, which effectively renders the static caches irrelevant. This could have a performance overhead (though again a questionable benefit in the first place?), but fix all contrib even if it continues using the static pattern. Probably worth doing above in addition...
  • Make \Drupal\Core\Plugin\DefaultPluginManager::clearCachedDefinitions clear itself off the container or clear the cached derivers...

Remaining tasks

Decide how to solve this...

User interface changes

None.

API changes

Depends on the decision...

Data model changes

None.

🐛 Bug report
Status

Needs work

Version

10.1

Component
Plugin 

Last updated about 17 hours ago

Created by

🇬🇧United Kingdom andrewbelcher

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

Production build 0.69.0 2024