From f3702d06e01c83f6e655697a34a0cf24f09bcacc Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Wed, 5 Nov 2025 13:34:48 +0800 Subject: [PATCH] MDL-87107 core: Stop using MUC for hook manager caching --- .upgradenotes/MDL-87107-2025110505340589.yml | 5 + lang/en/cache.php | 1 - lib/classes/hook/manager.php | 126 ++++++++++++++++--- lib/db/caches.php | 13 -- 4 files changed, 117 insertions(+), 28 deletions(-) create mode 100644 .upgradenotes/MDL-87107-2025110505340589.yml diff --git a/.upgradenotes/MDL-87107-2025110505340589.yml b/.upgradenotes/MDL-87107-2025110505340589.yml new file mode 100644 index 00000000000..f67543a0e2d --- /dev/null +++ b/.upgradenotes/MDL-87107-2025110505340589.yml @@ -0,0 +1,5 @@ +issueNumber: MDL-87107 +notes: + core: + - message: The Hook Manager now uses localcache instead of caching via MUC. + type: changed diff --git a/lang/en/cache.php b/lang/en/cache.php index 0c788c0a7e3..aa2561887e5 100644 --- a/lang/en/cache.php +++ b/lang/en/cache.php @@ -71,7 +71,6 @@ $string['cachedef_groupdata'] = 'Course group information'; $string['cachedef_h5p_content_type_translations'] = 'H5P content-type libraries translations'; $string['cachedef_h5p_libraries'] = 'H5P libraries'; $string['cachedef_h5p_library_files'] = 'H5P library files'; -$string['cachedef_hookcallbacks'] = 'Hook callbacks'; $string['cachedef_htmlpurifier'] = 'HTML Purifier - cleaned content'; $string['cachedef_langmenu'] = 'List of available languages'; $string['cachedef_license'] = 'List of licences'; diff --git a/lib/classes/hook/manager.php b/lib/classes/hook/manager.php index 4850103d732..87691164195 100644 --- a/lib/classes/hook/manager.php +++ b/lib/classes/hook/manager.php @@ -44,7 +44,8 @@ use Psr\EventDispatcher\StoppableEventInterface; */ final class manager implements EventDispatcherInterface, - ListenerProviderInterface { + ListenerProviderInterface +{ /** @var ?manager the one instance of listener provider and dispatcher */ private static $instance = null; @@ -278,9 +279,13 @@ final class manager implements * @return object The Event that was passed, now modified by listeners. */ public function dispatch(object $event): object { - // We can dispatch only after the lib/setup.php includes, - // that is right before the database connection is made, - // the MUC caches need to be working already. + // It is only safe to dispatch hooks after early setup is complete. + // This includes, but is not limited to: + // - configuring exception handlers + // - configuring autoloaders + // - configuring date/time. + // The database connection is not required. It is up to individual hooks to understand their context. + // At this time there is no way to check that setup is complete, but we can check that 'setup.php' has been included. if (!function_exists('setup_DB')) { debugging('Hooks cannot be dispatched yet', DEBUG_DEVELOPER); return $event; @@ -331,13 +336,14 @@ final class manager implements $this->allcallbacks = []; $this->alldeprecations = []; - $cache = null; // @codeCoverageIgnoreStart - if (!PHPUNIT_TEST && !CACHE_DISABLE_ALL) { - $cache = \cache::make('core', 'hookcallbacks'); - $callbacks = $cache->get('callbacks'); - $deprecations = $cache->get('deprecations'); - $overrideshash = $cache->get('overrideshash'); + $shouldcache = $this->should_cache(); + if ($shouldcache) { + $cache = $this->get_cache(); + + $callbacks = $cache['callbacks'] ?? null; + $deprecations = $cache['deprecations'] ?? null; + $overrideshash = $cache['overrideshash'] ?? null; $usecache = is_array($callbacks); $usecache = $usecache && is_array($deprecations); @@ -367,11 +373,15 @@ final class manager implements // Load the callbacks and apply overrides. $this->load_callbacks($components); - if ($cache) { - $cache->set('callbacks', $this->allcallbacks); - $cache->set('deprecations', $this->alldeprecations); - $cache->set('overrideshash', $this->calculate_overrides_hash()); + // @codeCoverageIgnoreStart + if ($shouldcache) { + $this->set_cache( + $this->allcallbacks, + $this->alldeprecations, + $this->calculate_overrides_hash(), + ); } + // @codeCoverageIgnoreEnd } /** @@ -669,4 +679,92 @@ final class manager implements return $hooks; } + + /** + * Get the path to the hook cache. + * + * @return string + */ + protected function get_cache_path(): string { + global $CFG; + + return $CFG->localcachedir . '/hookcallbacks.json'; + } + + /** + * Whether we should enable caching of hook data. + * + * The cache is disabled during unit tests, when CACHE_DISABLE_ALL is set, and during upgrades. + * + * @return bool + */ + protected function should_cache(): bool { + if (PHPUNIT_TEST) { + return false; + } + + if (CACHE_DISABLE_ALL) { + return false; + } + + if ($this->is_upgrade_running()) { + // Do not use the cache during upgrade. + return false; + } + + return true; + } + + /** + * Fetch and decode the hook cache. + * + * @return array|null + */ + protected function get_cache(): ?array { + $cachepath = $this->get_cache_path(); + if (!file_exists($cachepath)) { + return null; + } + return json_decode(file_get_contents($cachepath), true) ?? []; + } + + /** + * Store all relevant data in the cache. + * + * @param array $callbacks + * @param array $deprecations + * @param string|null $hash + */ + protected function set_cache( + array $callbacks, + array $deprecations, + ?string $hash, + ): void { + $cachedata = [ + 'callbacks' => $callbacks, + 'deprecations' => $deprecations, + 'overrideshash' => $hash, + ]; + + // Write to a temp file and rename it to ensure atomicity of reads. + // If we write directly to the cache file, another process may read it during the write and get corrupted data. + $cachepath = $this->get_cache_path(); + $tmppath = "{$cachepath}." . uniqid('tmp', true); + + file_put_contents($tmppath, json_encode($cachedata)); + rename($tmppath, $cachepath); + clearstatcache(true, $cachepath); + } + + /** + * Check whether upgrade is currently running. + * + * @return bool + */ + protected function is_upgrade_running(): bool { + global $CFG; + + // Note: This mimics the test in lib/setuplib.php during upgrade. + return !empty($CFG->upgraderunning); + } } diff --git a/lib/db/caches.php b/lib/db/caches.php index b3ad892d16e..ad9bb3019ff 100644 --- a/lib/db/caches.php +++ b/lib/db/caches.php @@ -79,19 +79,6 @@ $definitions = array( 'simpledata' => true, ), - // Hook callbacks cache. - // There is a static cache in hook manager, data is fetched once per page on first hook execution. - // This cache needs to be invalidated during upgrades when code changes and when callbacks - // overrides are updated. - 'hookcallbacks' => array( - 'mode' => cache_store::MODE_APPLICATION, - 'simplekeys' => true, - 'simpledata' => true, - 'staticacceleration' => false, - // WARNING: Manual cache purge may be required when overriding hook callbacks. - 'canuselocalstore' => true, - ), - // Cache for question definitions. This is used by the question_bank class. // Users probably do not need to know about this cache. They will just call // question_bank::load_question.