Contrib.social

ThemeRegistry::resolveCacheMiss() might look at wrong theme in some cases

Open on Drupal.org →
Created on 30 March 2018, over 6 years ago
Updated 18 July 2023, about 1 year ago

I found this while looking for refactoring opportunities in ThemeRegistry.

A Drupal\Core\Utility\ThemeRegistry object is created for a specific theme.
This theme determines the cache id ($cid) that ThemeRegistry will use to read from and write to the cache.

ThemeRegistry::resolveCacheMiss() gets a "complete registry" from \Drupal::service('theme.registry')->get().
This provides the theme registry array for whichever is the current theme at the time this is called.

Depending on things happening between ThemeRegistry object construction and the first ThemeRegistry::get() with a cache miss, a different theme might have become active then.

This would cause ThemeRegistry to write entries from theme B into the registry cache of theme A.

I don't know if this bug manifests anywhere. I suspect it would manifest as a miraculous heisenbug.
It would require that something changes the active theme mid-request, or that something requests a theme registry for a different theme than the active theme.

📌 Task
Status

Active

Version

11.0 🔥

Component
Theme  →

Last updated about 6 hours ago

Created by

🇩🇪Germany donquixote

Live updates comments and jobs are added and updated live.
  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

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.

  • Comment about 1 year ago →
    🇺🇸United States smustgrave

    Came up in the bugsmash daily triage target.

    Wonder if this should be closed out?

    @lendude brought the logic is being refactored over on 📌 Refactor ThemeRegistry and Theme\Registry Active

  • Comment about 1 year ago →
    🇩🇪Germany donquixote

    We could create a test case, to demonstrate that this was a problem in the past, and is now fixed.

    So, a test with an artificial scenario that would replicate the problem described here.
    We create one branch from an earlier version of Drupal where this still happens.
    And another branch with the same test, but in a later version where it is fixed.

    I don't have time atm to write this test, but I still think it can be a good exercise.

  • Comment about 1 year ago →
    🇩🇪Germany donquixote

    Also I don't see any code yet in the refactoring issue (you can blame me for it).

    I think the Hux POC MR has some interesting patterns that could be useful for refactoring other parts like the theme registry.

  • Status changed to Active about 1 year ago
  • Comment about 1 year ago →
    🇺🇸United States smustgrave

    Always think adding more test coverage is good.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024