Problem/Motivation
In a persistent worker such as FrankenPHP in worker mode, the theme registry service can survive request cleanup. Registry::destruct() calls the runtime collectors' cleanup method but retains those objects. The inherited CacheCollector::destruct() persists pending changes and resets their storage.
After cleanup, Registry::getRuntime() returns the retained collector instead of constructing a fresh one. Its has() and get() methods read that emptied storage directly, so previously available theme hooks are no longer found. When rendering a single hook, ThemeManager::render() logs Theme hook %hook not found. and returns FALSE, leaving the themed output missing.
Steps to reproduce
- On a bootstrapped site, obtain the runtime collector using Registry::getRuntime() and confirm that it contains a registered theme hook.
- Call Registry::destruct() once.
- Obtain the runtime collector again for the same theme. It is the same object, and checking the previously registered hook now returns false.
Proposed resolution
In Registry::destruct(), copy the runtime collectors to a local variable and clear the service's collection before destructing the copied collectors. The next call to Registry::getRuntime() then creates a fresh collector, whose constructor loads the runtime cache or initializes the registry on a cache miss. Repeated cleanup also skips collectors that have already been processed.
Issue fork drupal-3626105
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3626105-release-runtime-theme
changes, plain diff MR !17253
Comments
Comment #3
ptmkenny commentedComment #4
amitgoyal commentedReviewed the fix in MR !17253. Registry::destruct() now copies the runtime collectors to a local variable and clears $this->runtimeRegistry before destructing the copies, so a fresh collector is built on the next getRuntime() call instead of reusing the emptied one. Matches the proposed resolution.
Added test (testRuntimeRegistryAfterDestruct) confirms the theme hook is still found after destruct() and that a new collector instance is returned.
- Pipeline passes; the only red jobs are the PHPUnit Unit (Core/Component): [8.6-ubuntu] next-PHP-major lane, which is a non-blocking allow_failure job unrelated to this change.
- No open threads on the MR.
- Ran RegistryTest.php locally: OK (11 tests, 30 assertions).
- PHPCS clean on the diff; the pre-existing errors in Registry.php (lines 291, 461) also appear on unmodified main, unrelated to this change.
Moving to RTBC.
Comment #5
nicxvan commentedThis is a duplicate of #3609212: Runtime ThemeRegistry left empty after destruct(), causing "Theme hook not found" warnings.
This one is a bit more concise and rtbc so even though it's newer I think we should postpone the other.
Comment #6
nicxvan commentedI think this looks good, this is at least the third duplicate I've seen.
Test only fails as expected, I think the reset first might be better.
Ultimately we want these to be memory caches and we likely need some updates for this to really work properly for persistent workers, but let's get this in so we can stop the duplicate reports.
RTBC +1
Comment #10
catchSo yes I think for persistent workers we might want to go the other way and not clean these up at all, so that any loaded theme registry is available to any other request without re-initialization from the persistent / chained fast cache. We could clean-up and rebuild only when cache items are updated etc. But we are not there yet, so let's make the current logic correct and then we can look at that later on.
Committed/pushed to main and cherry-picked to 12.0.x and 11.x, thanks!