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.

Comments

donquixote created an issue. See original summary.

donquixote’s picture

donquixote commented

The underlying problem here is that Drupal\Core\Theme\Registry cannot decide whether it manages and represents the registry of a specific theme, or that of all themes. E.g. Registry->registry contains entries for multiple themes, but there is Registry->theme which is the current theme, and Registry->get() returns the registry for the current theme.

donquixote’s picture

donquixote commented

I think I found the reason why this might never happen in practice:
Theme\Registry->themeName or Theme\Registry->theme are not changed mid request.
They can only be initialized on first use, but never modified afterwards.

What could happen, in theory, is that someone creates a Theme\Registry instance that is distinct from the "theme.registry" service, with a theme name different from the current theme, and calls ->getRuntime() on that.

So the entire thing is still weird and fragile, but in practice it might not be a problem.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave commented
Status: Active » Postponed (maintainer needs more info)
Issue tags: +Bug Smash Initiative

Came up in the bugsmash daily triage target.

Wonder if this should be closed out?

@lendude brought the logic is being refactored over on #2957440: Refactor ThemeRegistry and Theme\Registry

donquixote’s picture

donquixote commented

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.

donquixote’s picture

donquixote commented

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.

smustgrave’s picture

smustgrave commented
Category: Bug report » Task
Status: Postponed (maintainer needs more info) » Active
Issue tags: +Needs issue summary update, +Needs title update

Always think adding more test coverage is good.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.