MDL-87107 core: Stop using MUC for hook manager caching

.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

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';

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);
+    }
 }

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.