From 9e7d9a0e9b76bcc76f27761b73c0ab4d4c1a36e2 Mon Sep 17 00:00:00 2001
From: Mark Johnson <mark.johnson@catalyst-eu.net>
Date: Thu, 11 Aug 2022 08:48:48 +0100
Subject: [PATCH 1/6] MDL-67020 Cache: Prevent unnecessary coursemodinfo
 rebuild

---
 course/format/classes/base.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/course/format/classes/base.php b/course/format/classes/base.php
index 9523522a5f9..b1fe4b486e8 100644
--- a/course/format/classes/base.php
+++ b/course/format/classes/base.php
@@ -1068,7 +1068,7 @@ abstract class base {
         $changed = $needrebuild = false;
         foreach ($defaultoptions as $key => $value) {
             if (isset($records[$key])) {
-                if (array_key_exists($key, $data) && $records[$key]->value !== $data[$key]) {
+                if (array_key_exists($key, $data) && $records[$key]->value != $data[$key]) {
                     $DB->set_field('course_format_options', 'value',
                             $data[$key], array('id' => $records[$key]->id));
                     $changed = true;

From c06fc0648d72a0cbc7407d6d33d898211f93d202 Mon Sep 17 00:00:00 2001
From: Mark Johnson <mark.johnson@catalyst-eu.net>
Date: Thu, 11 Aug 2022 08:50:25 +0100
Subject: [PATCH 2/6] MDL-67020 Cache: Release locks in
 cache_application::get_many()

---
 cache/classes/loaders.php | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/cache/classes/loaders.php b/cache/classes/loaders.php
index c8a92729072..c1f6d8ee1e0 100644
--- a/cache/classes/loaders.php
+++ b/cache/classes/loaders.php
@@ -1809,10 +1809,11 @@ class cache_application extends cache implements cache_loader_with_locking {
      * @throws coding_exception
      */
     public function get_many(array $keys, $strictness = IGNORE_MISSING) {
+        $locks = [];
         if ($this->requirelockingread) {
             foreach ($keys as $id => $key) {
-                $lock =$this->acquire_lock($key);
-                if (!$lock) {
+                $locks[$key] = $this->acquire_lock($key);
+                if (!$locks[$key]) {
                     if ($strictness === MUST_EXIST) {
                         throw new coding_exception('Could not acquire read locks for all of the items being requested.');
                     } else {
@@ -1823,7 +1824,13 @@ class cache_application extends cache implements cache_loader_with_locking {
 
             }
         }
-        return parent::get_many($keys, $strictness);
+        $result = parent::get_many($keys, $strictness);
+        if ($this->requirelockingread) {
+            foreach ($locks as $key => $lock) {
+                $this->release_lock($key);
+            }
+        }
+        return $result;
     }
 
     /**

From dcc7d7b8df087fff6f228b1a5af14e496ce0ba6e Mon Sep 17 00:00:00 2001
From: Mark Johnson <mark.johnson@catalyst-eu.net>
Date: Thu, 11 Aug 2022 09:07:20 +0100
Subject: [PATCH 3/6] MDL-67020 Cache: Increment cacherev when purging course
 cache

---
 lib/modinfolib.php | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/modinfolib.php b/lib/modinfolib.php
index 765aeba500e..fca0b7071d0 100644
--- a/lib/modinfolib.php
+++ b/lib/modinfolib.php
@@ -970,6 +970,7 @@ class course_modinfo {
      * @param int $courseid Course id
      */
     public static function purge_course_cache(int $courseid): void {
+        increment_revision_number('course', 'cacherev', 'id = :id', array('id' => $courseid));
         $cachemodinfo = cache::make('core', 'coursemodinfo');
         $cachemodinfo->delete($courseid);
     }

From 045ee091daf71f151c3fbddd7fe1fa5264900f8a Mon Sep 17 00:00:00 2001
From: Mark Johnson <mark.johnson@catalyst-eu.net>
Date: Thu, 4 Aug 2022 16:27:27 +0100
Subject: [PATCH 4/6] MDL-67020 Cache: Make locking work for local file caches

* Add new requirelockingbeforewrite flag for cache definitions
* Implement native locking in cachestore_file (leveraging
file_lock_factory).
---
 cache/classes/definition.php                  |  25 +++-
 cache/classes/helper.php                      |   2 +
 cache/classes/loaders.php                     |  55 ++++++++-
 cache/disabledlib.php                         |  20 ++++
 cache/stores/file/addinstanceform.php         |   5 +
 cache/stores/file/lang/en/cachestore_file.php |   2 +
 cache/stores/file/lib.php                     | 107 +++++++++++++++++-
 cache/stores/file/tests/store_test.php        |  12 ++
 cache/tests/cache_test.php                    |  49 ++++++++
 cache/upgrade.txt                             |   8 ++
 lib/classes/lock/file_lock_factory.php        |   9 +-
 .../lock/timing_wrapper_lock_factory.php      |  61 +++++++---
 12 files changed, 328 insertions(+), 27 deletions(-)

diff --git a/cache/classes/definition.php b/cache/classes/definition.php
index e1e673c4893..52ce88a5641 100644
--- a/cache/classes/definition.php
+++ b/cache/classes/definition.php
@@ -208,6 +208,12 @@ class cache_definition {
      */
     protected $requirelockingwrite = false;
 
+    /**
+     * Gets set to true if this definition requires a lock to be acquired before a write is attempted.
+     * @var bool
+     */
+    protected $requirelockingbeforewrite = false;
+
     /**
      * Gets set to true if this definition requires searchable stores.
      * @since Moodle 2.4.4
@@ -357,6 +363,7 @@ class cache_definition {
         $requiremultipleidentifiers = false;
         $requirelockingread = false;
         $requirelockingwrite = false;
+        $requirelockingbeforewrite = false;
         $requiresearchable = ($mode === cache_store::MODE_SESSION) ? true : false;
         $maxsize = null;
         $overrideclass = null;
@@ -395,7 +402,14 @@ class cache_definition {
         if (array_key_exists('requirelockingwrite', $definition)) {
             $requirelockingwrite = (bool)$definition['requirelockingwrite'];
         }
-        $requirelocking = $requirelockingwrite || $requirelockingread;
+        if (array_key_exists('requirelockingbeforewrite', $definition)) {
+            $requirelockingbeforewrite = (bool)$definition['requirelockingbeforewrite'];
+        }
+        if ($requirelockingbeforewrite && ($requirelockingwrite || $requirelockingread)) {
+            throw new coding_exception('requirelockingbeforewrite cannot be set with requirelockingread or requirelockingwrite
+                    in a cache definition, as this will result in conflicting locks.');
+        }
+        $requirelocking = $requirelockingwrite || $requirelockingbeforewrite || $requirelockingread;
 
         if (array_key_exists('requiresearchable', $definition)) {
             $requiresearchable = (bool)$definition['requiresearchable'];
@@ -523,6 +537,7 @@ class cache_definition {
         $cachedefinition->requirelocking = $requirelocking;
         $cachedefinition->requirelockingread = $requirelockingread;
         $cachedefinition->requirelockingwrite = $requirelockingwrite;
+        $cachedefinition->requirelockingbeforewrite = $requirelockingbeforewrite;
         $cachedefinition->requiresearchable = $requiresearchable;
         $cachedefinition->maxsize = $maxsize;
         $cachedefinition->overrideclass = $overrideclass;
@@ -741,6 +756,14 @@ class cache_definition {
         return $this->requirelockingwrite;
     }
 
+    /**
+     * Returns true if this definition requires a lock to be aquired before a write is attempted.
+     * @return bool
+     */
+    public function require_locking_before_write() {
+        return $this->requirelockingbeforewrite;
+    }
+
     /**
      * Returns true if this definition allows local storage to be used for caching.
      * @since Moodle 3.1.0
diff --git a/cache/classes/helper.php b/cache/classes/helper.php
index db78844626c..cf9364e30ea 100644
--- a/cache/classes/helper.php
+++ b/cache/classes/helper.php
@@ -382,6 +382,7 @@ class cache_helper {
                         'misses' => 0,
                         'sets' => 0,
                         'iobytes' => cache_store::IO_BYTES_NOT_SUPPORTED,
+                        'locks' => 0,
                     )
                 )
             );
@@ -392,6 +393,7 @@ class cache_helper {
                 'misses' => 0,
                 'sets' => 0,
                 'iobytes' => cache_store::IO_BYTES_NOT_SUPPORTED,
+                'locks' => 0,
             );
         }
     }
diff --git a/cache/classes/loaders.php b/cache/classes/loaders.php
index c1f6d8ee1e0..d60122be400 100644
--- a/cache/classes/loaders.php
+++ b/cache/classes/loaders.php
@@ -1581,12 +1581,24 @@ class cache_application extends cache implements cache_loader_with_locking {
      */
     protected $requirelockingwrite = false;
 
+    /**
+     * Gets set to true if the cache writes (set|delete) must have a manual lock created first
+     * @var bool
+     */
+    protected $requirelockingbeforewrite = false;
+
     /**
      * Gets set to a cache_store to use for locking if the caches primary store doesn't support locking natively.
      * @var cache_lock_interface
      */
     protected $cachelockinstance;
 
+    /**
+     * Store a list of locks acquired by this process.
+     * @var array
+     */
+    protected $locks;
+
     /**
      * Overrides the cache construct method.
      *
@@ -1603,6 +1615,7 @@ class cache_application extends cache implements cache_loader_with_locking {
             $this->requirelocking = true;
             $this->requirelockingread = $definition->require_locking_read();
             $this->requirelockingwrite = $definition->require_locking_write();
+            $this->requirelockingbeforewrite = $definition->require_locking_before_write();
         }
 
         $this->handle_invalidation_events();
@@ -1648,13 +1661,24 @@ class cache_application extends cache implements cache_loader_with_locking {
      * @return bool Returns true if the lock could be acquired, false otherwise.
      */
     public function acquire_lock($key) {
+        global $CFG;
         $key = $this->parse_key($key);
+        $before = microtime(true);
         if ($this->nativelocking) {
-            return $this->get_store()->acquire_lock($key, $this->get_identifier());
+            $lock = $this->get_store()->acquire_lock($key, $this->get_identifier());
         } else {
             $this->ensure_cachelock_available();
-            return $this->cachelockinstance->lock($key, $this->get_identifier());
+            $lock = $this->cachelockinstance->lock($key, $this->get_identifier());
         }
+        $after = microtime(true);
+        if ($lock) {
+            $this->locks[$key] = $lock;
+            if (defined('MDL_PERF') || !empty($CFG->perfdebug)) {
+                \core\lock\timing_wrapper_lock_factory::record_lock_data($after, $before,
+                        $this->get_definition()->get_id(), $key, $lock, $this->get_identifier() . $key);
+            }
+        }
+        return $lock;
     }
 
     /**
@@ -1666,6 +1690,9 @@ class cache_application extends cache implements cache_loader_with_locking {
      */
     public function check_lock_state($key) {
         $key = $this->parse_key($key);
+        if (!empty($this->locks[$key])) {
+            return true; // Shortcut to save having to make a call to the cache store if the lock is held by this process.
+        }
         if ($this->nativelocking) {
             return $this->get_store()->check_lock_state($key, $this->get_identifier());
         } else {
@@ -1683,11 +1710,18 @@ class cache_application extends cache implements cache_loader_with_locking {
     public function release_lock($key) {
         $key = $this->parse_key($key);
         if ($this->nativelocking) {
-            return $this->get_store()->release_lock($key, $this->get_identifier());
+            $released = $this->get_store()->release_lock($key, $this->get_identifier());
         } else {
             $this->ensure_cachelock_available();
-            return $this->cachelockinstance->unlock($key, $this->get_identifier());
+            $released = $this->cachelockinstance->unlock($key, $this->get_identifier());
         }
+        if ($released && array_key_exists($key, $this->locks)) {
+            unset($this->locks[$key]);
+            if (defined('MDL_PERF') || !empty($CFG->perfdebug)) {
+                \core\lock\timing_wrapper_lock_factory::record_lock_released_data($this->get_identifier() . $key);
+            }
+        }
+        return $released;
     }
 
     /**
@@ -1719,6 +1753,10 @@ class cache_application extends cache implements cache_loader_with_locking {
      * @return bool True on success, false otherwise.
      */
     protected function set_implementation($key, int $version, $data, bool $setparents = true): bool {
+        if ($this->requirelockingbeforewrite && !$this->check_lock_state($key)) {
+            throw new coding_exception('Attempted to set cache key "' . $key . '" without a lock. '
+                . 'Locking before writes is required for ' . $this->get_definition()->get_id());
+        }
         if ($this->requirelockingwrite && !$this->acquire_lock($key)) {
             return false;
         }
@@ -1753,6 +1791,15 @@ class cache_application extends cache implements cache_loader_with_locking {
      *      ... if they care that is.
      */
     public function set_many(array $keyvaluearray) {
+        if ($this->requirelockingbeforewrite) {
+            foreach ($keyvaluearray as $id => $pair) {
+                if (!$this->check_lock_state($pair['key'])) {
+                    debugging('Attempted to set cache key "' . $pair['key'] . '" without a lock. '
+                            . 'Locking before writes is required for ' . $this->get_definition()->get_name(), DEBUG_DEVELOPER);
+                    unset($keyvaluearray[$id]);
+                }
+            }
+        }
         if ($this->requirelockingwrite) {
             $locks = array();
             foreach ($keyvaluearray as $id => $pair) {
diff --git a/cache/disabledlib.php b/cache/disabledlib.php
index e27318dead6..c7cde7607db 100644
--- a/cache/disabledlib.php
+++ b/cache/disabledlib.php
@@ -200,6 +200,26 @@ class cache_disabled extends cache {
     public function purge() {
         return true;
     }
+
+    /**
+     * Pretend that we got a lock to avoid errors.
+     *
+     * @param string $key
+     * @return bool
+     */
+    public function acquire_lock(string $key) : bool {
+        return true;
+    }
+
+    /**
+     * Pretend that we released a lock to avoid errors.
+     *
+     * @param string $key
+     * @return void
+     */
+    public function release_lock(string $key) : bool {
+        return true;
+    }
 }
 
 /**
diff --git a/cache/stores/file/addinstanceform.php b/cache/stores/file/addinstanceform.php
index a2f3c3a9d46..4240d7f5952 100644
--- a/cache/stores/file/addinstanceform.php
+++ b/cache/stores/file/addinstanceform.php
@@ -62,5 +62,10 @@ class cachestore_file_addinstance_form extends cachestore_addinstance_form {
         $form->addElement('checkbox', 'asyncpurge', get_string('asyncpurge', 'cachestore_file'));
         $form->setType('asyncpurge', PARAM_BOOL);
         $form->addHelpButton('asyncpurge', 'asyncpurge', 'cachestore_file');
+
+        $form->addElement('text', 'lockwait', get_string('lockwait', 'cachestore_file'));
+        $form->setDefault('lockwait', 60);
+        $form->setType('lockwait', PARAM_INT);
+        $form->addHelpButton('lockwait', 'lockwait', 'cachestore_file');
     }
 }
\ No newline at end of file
diff --git a/cache/stores/file/lang/en/cachestore_file.php b/cache/stores/file/lang/en/cachestore_file.php
index ceb1bfe9ae8..1562d752698 100644
--- a/cache/stores/file/lang/en/cachestore_file.php
+++ b/cache/stores/file/lang/en/cachestore_file.php
@@ -32,6 +32,8 @@ $string['asyncpurge'] = 'Asynchronously purge directory';
 $string['asyncpurge_help'] = 'If enabled, the new directory is created with cache revision and the old directory will be deleted asynchronously via a scheduled task.';
 $string['autocreate'] = 'Auto create directory';
 $string['autocreate_help'] = 'If enabled the directory specified in path will be automatically created if it does not already exist.';
+$string['lockwait'] = 'Maximum lock wait time';
+$string['lockwait_help'] = 'The maximum amount of time in seconds to wait for an exclusive lock before reading or writing a cache key. This is only used for cache definitions that have read or write locking required.';
 $string['path'] = 'Cache path';
 $string['path_help'] = 'The directory that should be used to store files for this cache store. If left blank (default) a directory will be automatically created in the moodledata directory. This can be used to point a file store towards a directory on a better performing drive (such as one in memory).';
 $string['pluginname'] = 'File cache';
diff --git a/cache/stores/file/lib.php b/cache/stores/file/lib.php
index a6e026b02a0..12bb8b09fea 100644
--- a/cache/stores/file/lib.php
+++ b/cache/stores/file/lib.php
@@ -37,7 +37,8 @@
  * @copyright  2012 Sam Hemelryk
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
-class cachestore_file extends cache_store implements cache_is_key_aware, cache_is_configurable, cache_is_searchable  {
+class cachestore_file extends cache_store implements cache_is_key_aware, cache_is_configurable, cache_is_searchable,
+        cache_is_lockable {
 
     /**
      * The name of the store.
@@ -128,6 +129,23 @@ class cachestore_file extends cache_store implements cache_is_key_aware, cache_i
      */
     private $cfg = null;
 
+    /** @var int Maximum number of seconds to wait for a lock before giving up. */
+    protected $lockwait = 60;
+
+    /**
+     * Instance of file_lock_factory configured to create locks in the cache directory.
+     *
+     * @var \core\lock\file_lock_factory $lockfactory
+     */
+    protected $lockfactory = null;
+
+    /**
+     * List of current locks.
+     *
+     * @var array $locks
+     */
+    protected $locks = [];
+
     /**
      * Constructs the store instance.
      *
@@ -193,6 +211,21 @@ class cachestore_file extends cache_store implements cache_is_key_aware, cache_i
         } else {
             $this->asyncpurge = false;
         }
+
+        // Leverage cachelock_file to provide native locking, to avoid duplicating logic.
+        // This will store locks alongside the cache, so local cache uses local locks.
+        $lockdir = $path . '/filelocks';
+        if (!file_exists($lockdir)) {
+            make_writable_directory($lockdir);
+        }
+        if (array_key_exists('lockwait', $configuration)) {
+            $this->lockwait = (int)$configuration['lockwait'];
+        }
+        $this->lockfactory = new \core\lock\file_lock_factory('cachestore_file', $lockdir);
+        if (!$this->lockfactory->is_available()) {
+            // File locking is disabled in config, fall back to default lock factory.
+            $this->lockfactory = \core\lock\lock_config::get_lock_factory('cachestore_file');
+        }
     }
 
     /**
@@ -675,6 +708,9 @@ class cachestore_file extends cache_store implements cache_is_key_aware, cache_i
         if (isset($data->asyncpurge)) {
             $config['asyncpurge'] = $data->asyncpurge;
         }
+        if (isset($data->lockwait)) {
+            $config['lockwait'] = $data->lockwait;
+        }
 
         return $config;
     }
@@ -702,6 +738,9 @@ class cachestore_file extends cache_store implements cache_is_key_aware, cache_i
         if (isset($config['asyncpurge'])) {
             $data['asyncpurge'] = (bool)$config['asyncpurge'];
         }
+        if (isset($config['lockwait'])) {
+            $data['lockwait'] = (int)$config['lockwait'];
+        }
         $editform->set_data($data);
     }
 
@@ -924,4 +963,70 @@ class cachestore_file extends cache_store implements cache_is_key_aware, cache_i
         $result->sd = sqrt($squarediff);
         return $result;
     }
+
+    /**
+     * Use lock factory to determine the lock state.
+     *
+     * @param string $key Lock identifier
+     * @param string $ownerid Cache identifier
+     * @return bool|null
+     */
+    public function check_lock_state($key, $ownerid) : ?bool {
+        if (!array_key_exists($key, $this->locks)) {
+            return null; // Lock does not exist.
+        }
+        if (!array_key_exists($ownerid, $this->locks[$key])) {
+            return false; // Lock exists, but belongs to someone else.
+        }
+        if ($this->locks[$key][$ownerid] instanceof \core\lock\lock) {
+            return true; // Lock exists, and we own it.
+        }
+        // Try to get the lock with an immediate timeout. If this succeeds, the lock does not currently exist.
+        $lock = $this->lockfactory->get_lock($key, 0);
+        if ($lock) {
+            // Lock was not already held.
+            $lock->release();
+            return null;
+        } else {
+            // Lock is held by someone else.
+            return false;
+        }
+    }
+
+    /**
+     * Use lock factory to acquire a lock.
+     *
+     * @param string $key Lock identifier
+     * @param string $ownerid Cache identifier
+     * @return bool
+     * @throws cache_exception
+     */
+    public function acquire_lock($key, $ownerid) : bool {
+        $lock = $this->lockfactory->get_lock($key, $this->lockwait);
+        if ($lock) {
+            $this->locks[$key][$ownerid] = $lock;
+        }
+        return (bool)$lock;
+    }
+
+    /**
+     * Use lock factory to release a lock.
+     *
+     * @param string $key Lock identifier
+     * @param string $ownerid Cache identifier
+     * @return bool
+     */
+    public function release_lock($key, $ownerid) : bool {
+        if (!array_key_exists($key, $this->locks)) {
+            return false; // No lock to release.
+        }
+        if (!array_key_exists($ownerid, $this->locks[$key])) {
+            return false; // Tried to release someone else's lock.
+        }
+        $unlocked = $this->locks[$key][$ownerid]->release();
+        if ($unlocked) {
+            unset($this->locks[$key]);
+        }
+        return $unlocked;
+    }
 }
diff --git a/cache/stores/file/tests/store_test.php b/cache/stores/file/tests/store_test.php
index 364593bec99..d39725bd2c7 100644
--- a/cache/stores/file/tests/store_test.php
+++ b/cache/stores/file/tests/store_test.php
@@ -33,6 +33,7 @@ require_once($CFG->dirroot.'/cache/stores/file/lib.php');
  * @package    cachestore_file
  * @copyright  2013 Sam Hemelryk
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ * @covers \cachestore_file
  */
 class store_test extends \cachestore_tests {
     /**
@@ -99,4 +100,15 @@ class store_test extends \cachestore_tests {
         ]);
         $this->assertEquals(21, $store->get_last_io_bytes());
     }
+
+    public function test_lock() {
+        $store = new \cachestore_file('Test');
+
+        $this->assertTrue($store->acquire_lock('lock', '123'));
+        $this->assertTrue($store->check_lock_state('lock', '123'));
+        $this->assertFalse($store->check_lock_state('lock', '321'));
+        $this->assertNull($store->check_lock_state('notalock', '123'));
+        $this->assertFalse($store->release_lock('lock', '321'));
+        $this->assertTrue($store->release_lock('lock', '123'));
+    }
 }
diff --git a/cache/tests/cache_test.php b/cache/tests/cache_test.php
index d64e921e883..f5b874236b7 100644
--- a/cache/tests/cache_test.php
+++ b/cache/tests/cache_test.php
@@ -2111,6 +2111,55 @@ class cache_test extends \advanced_testcase {
         $this->assertFalse($cache->has('a'));
     }
 
+    /**
+     * Test requiring a lock before attempting to set a key.
+     *
+     * @covers ::set_implementation
+     */
+    public function test_application_locking_before_write() {
+        $instance = cache_config_testing::instance(true);
+        $instance->phpunit_add_definition('phpunit/test_application_locking', array(
+            'mode' => cache_store::MODE_APPLICATION,
+            'component' => 'phpunit',
+            'area' => 'test_application_locking',
+            'staticacceleration' => true,
+            'staticaccelerationsize' => 1,
+            'requirelockingbeforewrite' => true
+        ));
+        $cache = cache::make('phpunit', 'test_application_locking');
+        $this->assertInstanceOf(cache_application::class, $cache);
+
+        $cache->acquire_lock('a');
+        $this->assertTrue($cache->set('a', 'A'));
+        $cache->release_lock('a');
+
+        $this->expectExceptionMessage('Attempted to set cache key "b" without a lock. '
+                . 'Locking before writes is required for phpunit/test_application_locking');
+        $this->assertFalse($cache->set('b', 'B'));
+    }
+
+
+    /**
+     * Test that invalid lock setting combinations are caught.
+     *
+     * @covers ::make
+     */
+    public function test_application_conflicting_locks() {
+        $instance = cache_config_testing::instance(true);
+        $instance->phpunit_add_definition('phpunit/test_application_locking', array(
+                'mode' => cache_store::MODE_APPLICATION,
+                'component' => 'phpunit',
+                'area' => 'test_application_locking',
+                'staticacceleration' => true,
+                'staticaccelerationsize' => 1,
+                'requirelockingwrite' => true,
+                'requirelockingbeforewrite' => true,
+        ));
+
+        $this->expectException('coding_exception');
+        cache::make('phpunit', 'test_application_locking');
+    }
+
     /**
      * Test the static cache_helper method purge_stores_used_by_definition.
      */
diff --git a/cache/upgrade.txt b/cache/upgrade.txt
index b42024bff76..079de19a94a 100644
--- a/cache/upgrade.txt
+++ b/cache/upgrade.txt
@@ -1,6 +1,14 @@
 This files describes API changes in /cache/stores/* - cache store plugins.
 Information provided here is intended especially for developers.
 
+=== 4.1 ===
+* Added new `requirelockingbeforewrite` option for cache definitions. This will check that a lock for a given cache key already
+  exists before it will perform a `set()` on that key. A `coding_exception` is thrown if the lock has not been acquired.
+* Added native locking to cachestore_file. This will use an instance of file_lock_factory pointing at a subdirectory in the same
+  location as the cache instance, meaning a local file cache will have its locks stored locally. If file locks are disabled
+  globally, it will fall back to use the default lock factory, which may not be in the same location as the cache. cachestore_file
+  includes an additional setting to control how long it will wait for a lock before giving up, default is 60 seconds.
+
 === 4.0 ===
 * Cache stores may implement new optional function cache_store::get_last_io_bytes() to provide
   information about the size of data transferred (shown in footer if performance info enabled).
diff --git a/lib/classes/lock/file_lock_factory.php b/lib/classes/lock/file_lock_factory.php
index 87b93803571..ff60aeb10a0 100644
--- a/lib/classes/lock/file_lock_factory.php
+++ b/lib/classes/lock/file_lock_factory.php
@@ -58,12 +58,15 @@ class file_lock_factory implements lock_factory {
      * Create this lock factory.
      *
      * @param string $type - The type, e.g. cron, cache, session
+     * @param string|null $lockdirectory - Optional path to the lock directory, to override defaults.
      */
-    public function __construct($type) {
+    public function __construct($type, ?string $lockdirectory = null) {
         global $CFG;
 
         $this->type = $type;
-        if (!isset($CFG->file_lock_root)) {
+        if (!is_null($lockdirectory)) {
+            $this->lockdirectory = $lockdirectory;
+        } else if (!isset($CFG->file_lock_root)) {
             $this->lockdirectory = $CFG->dataroot . '/lock';
         } else {
             $this->lockdirectory = $CFG->file_lock_root;
@@ -100,7 +103,7 @@ class file_lock_factory implements lock_factory {
         global $CFG;
         $preventfilelocking = !empty($CFG->preventfilelocking);
         $lockdirisdataroot = true;
-        if (!empty($CFG->file_lock_root) && strpos($CFG->file_lock_root, $CFG->dataroot) !== 0) {
+        if (strpos($this->lockdirectory, $CFG->dataroot) !== 0) {
             $lockdirisdataroot = false;
         }
         return !$preventfilelocking || !$lockdirisdataroot;
diff --git a/lib/classes/lock/timing_wrapper_lock_factory.php b/lib/classes/lock/timing_wrapper_lock_factory.php
index 8563664e1d4..0a8a6c15513 100644
--- a/lib/classes/lock/timing_wrapper_lock_factory.php
+++ b/lib/classes/lock/timing_wrapper_lock_factory.php
@@ -70,40 +70,67 @@ class timing_wrapper_lock_factory implements lock_factory {
      * @return \core\lock\lock|boolean - An instance of \core\lock\lock if the lock was obtained, or false.
      */
     public function get_lock($resource, $timeout, $maxlifetime = 86400) {
-        global $PERF;
-
         $before = microtime(true);
 
         $result = $this->factory->get_lock($resource, $timeout, $maxlifetime);
 
         $after = microtime(true);
-        $duration = $after - $before;
-        if (empty($PERF->locks)) {
-            $PERF->locks = [];
-        }
-        $lockdata = (object) [
-            'type' => $this->type,
-            'resource' => $resource,
-            'wait' => $duration,
-            'success' => (bool)$result
-        ];
+        self::record_lock_data($after, $before, $this->type, $resource, (bool)$result, $result);
         if ($result) {
-            $lockdata->lock = $result;
-            $lockdata->timestart = $after;
             $result->init_factory($this);
         }
-        $PERF->locks[] = $lockdata;
 
         return $result;
     }
 
     /**
-     * Release a lock that was previously obtained with @lock.
+     * Records statistics about a lock to the performance data.
+     *
+     * @param float $after The time after the lock was achieved.
+     * @param float $before The time before the lock was requested.
+     * @param string $type The type of lock.
+     * @param string $resource The resource being locked.
+     * @param bool $result Whether the lock was successful.
+     * @param lock|string $lock A value uniquely identifying the lock.
+     * @return void
+     */
+    public static function record_lock_data(float $after, float $before, string $type, string $resource, bool $result, $lock) {
+        global $PERF;
+        $duration = $after - $before;
+        if (empty($PERF->locks)) {
+            $PERF->locks = [];
+        }
+        $lockdata = (object) [
+            'type' => $type,
+            'resource' => $resource,
+            'wait' => $duration,
+            'success' => $result
+        ];
+        if ($result) {
+            $lockdata->lock = $lock;
+            $lockdata->timestart = $after;
+        }
+        $PERF->locks[] = $lockdata;
+    }
+
+    /**
+     * Release a lock that was previously obtained with {@see get_lock}.
      *
      * @param lock $lock - The lock to release.
      * @return boolean - True if the lock is no longer held (including if it was never held).
      */
     public function release_lock(lock $lock) {
+        self::record_lock_released_data($lock);
+        return $this->factory->release_lock($lock);
+    }
+
+    /**
+     * Find the lock in the performance info and update it with the time held.
+     *
+     * @param lock|string $lock A value uniquely identifying the lock.
+     * @return void
+     */
+    public static function record_lock_released_data($lock) {
         global $PERF;
 
         // Find this lock in the list of locks we got, looking backwards since it is probably
@@ -117,8 +144,6 @@ class timing_wrapper_lock_factory implements lock_factory {
                 break;
             }
         }
-
-        return $this->factory->release_lock($lock);
     }
 
     /**

From ea01fb24273de4011694d91e34f86016d319686e Mon Sep 17 00:00:00 2001
From: Mark Johnson <mark.johnson@catalyst-eu.net>
Date: Thu, 11 Aug 2022 09:13:41 +0100
Subject: [PATCH 5/6] MDL-67020 Cache: Make local caching work for
 coursemodinfo

---
 lib/db/caches.php             |  1 +
 lib/modinfolib.php            | 84 +++++++++++++----------------------
 lib/tests/modinfolib_test.php |  2 +
 lib/upgrade.txt               |  3 ++
 version.php                   |  2 +-
 5 files changed, 39 insertions(+), 53 deletions(-)

diff --git a/lib/db/caches.php b/lib/db/caches.php
index 084350647fd..ab449d750db 100644
--- a/lib/db/caches.php
+++ b/lib/db/caches.php
@@ -251,6 +251,7 @@ $definitions = array(
         'mode' => cache_store::MODE_APPLICATION,
         'simplekeys' => true,
         'canuselocalstore' => true,
+        'requirelockingbeforewrite' => true
     ),
     // This is the session user selections cache.
     // It's a special cache that is used to record user selections that should persist for the lifetime of the session.
diff --git a/lib/modinfolib.php b/lib/modinfolib.php
index fca0b7071d0..94f2dfb4452 100644
--- a/lib/modinfolib.php
+++ b/lib/modinfolib.php
@@ -475,7 +475,8 @@ class course_modinfo {
         // partial rebuild logic sometimes sets the $coursemodinfo->cacherev to -1 which is an
         // indicator that it needs rebuilding.
         if ($coursemodinfo === false || ($course->cacherev > $coursemodinfo->cacherev)) {
-            $lock = self::get_course_cache_lock($course->id);
+            $cachekey = $course->id;
+            $lock = $cachecoursemodinfo->acquire_lock($cachekey);
             try {
                 // Only actually do the build if it's still needed after getting the lock (not if
                 // somebody else, who might have been holding the lock, built it already).
@@ -484,7 +485,7 @@ class course_modinfo {
                     $coursemodinfo = self::inner_build_course_cache($course, $lock);
                 }
             } finally {
-                $lock->release();
+                $cachecoursemodinfo->release_lock($cachekey);
             }
         }
 
@@ -631,34 +632,6 @@ class course_modinfo {
         return $compressedsections;
     }
 
-    /**
-     * Gets a lock for rebuilding the cache of a single course.
-     *
-     * Caller must release the returned lock.
-     *
-     * This is used to ensure that the cache rebuild doesn't happen multiple times in parallel.
-     * This function will wait up to 1 minute for the lock to be obtained. If the lock cannot
-     * be obtained, it throws an exception.
-     *
-     * @param int $courseid Course id
-     * @return \core\lock\lock Lock (must be released!)
-     * @throws moodle_exception If the lock cannot be obtained
-     */
-    protected static function get_course_cache_lock($courseid) {
-        // Get database lock to ensure this doesn't happen multiple times in parallel. Wait a
-        // reasonable time for the lock to be released, so we can give a suitable error message.
-        // In case the system crashes while building the course cache, the lock will automatically
-        // expire after a (slightly longer) period.
-        $lockfactory = \core\lock\lock_config::get_lock_factory('core_modinfo');
-        $lock = $lockfactory->get_lock('build_course_cache_' . $courseid,
-                self::COURSE_CACHE_LOCK_WAIT, self::COURSE_CACHE_LOCK_EXPIRY);
-        if (!$lock) {
-            throw new moodle_exception('locktimeout', '', '', null,
-                    'core_modinfo/build_course_cache_' . $courseid);
-        }
-        return $lock;
-    }
-
     /**
      * Builds and stores in MUC object containing information about course
      * modules and sections together with cached fields from table course.
@@ -676,11 +649,13 @@ class course_modinfo {
             throw new coding_exception('Object $course is missing required property \id\'');
         }
 
-        $lock = self::get_course_cache_lock($course->id);
+        $cachecoursemodinfo = cache::make('core', 'coursemodinfo');
+        $cachekey = $course->id;
+        $cachecoursemodinfo->acquire_lock($cachekey);
         try {
-            return self::inner_build_course_cache($course, $lock, $partialrebuild);
+            return self::inner_build_course_cache($course, $partialrebuild);
         } finally {
-            $lock->release();
+            $cachecoursemodinfo->release_lock($cachekey);
         }
     }
 
@@ -688,20 +663,23 @@ class course_modinfo {
      * Called to build course cache when there is already a lock obtained.
      *
      * @param stdClass $course object from DB table course
-     * @param \core\lock\lock $lock Lock object - not actually used, just there to indicate you have a lock
      * @param bool $partialrebuild Indicate if it's partial course cache rebuild or not
      * @return stdClass Course object that has been stored in MUC
      */
-    protected static function inner_build_course_cache(\stdClass $course, \core\lock\lock $lock,
-            bool $partialrebuild = false): \stdClass {
+    protected static function inner_build_course_cache(\stdClass $course, bool $partialrebuild = false): \stdClass {
         global $DB, $CFG;
         require_once("{$CFG->dirroot}/course/lib.php");
 
+        $cachekey = $course->id;
+        $cachecoursemodinfo = cache::make('core', 'coursemodinfo');
+        if (!$cachecoursemodinfo->check_lock_state($cachekey)) {
+            throw new coding_exception('You must acquire a lock on the course ID before calling inner_build_course_cache');
+        }
+
         // Always reload the course object from database to ensure we have the latest possible
         // value for cacherev.
         $course = $DB->get_record('course', ['id' => $course->id],
                 implode(',', array_merge(['id'], self::$cachedfields)), MUST_EXIST);
-
         // Retrieve all information about activities and sections.
         $coursemodinfo = new stdClass();
         $coursemodinfo->modinfo = self::get_array_of_activities($course, $partialrebuild);
@@ -710,8 +688,7 @@ class course_modinfo {
             $coursemodinfo->$key = $course->$key;
         }
         // Set the accumulated activities and sections information in cache, together with cacherev.
-        $cachecoursemodinfo = cache::make('core', 'coursemodinfo');
-        $cachecoursemodinfo->set_versioned($course->id, $course->cacherev, $coursemodinfo);
+        $cachecoursemodinfo->set_versioned($cachekey, $course->cacherev, $coursemodinfo);
         return $coursemodinfo;
     }
 
@@ -724,19 +701,20 @@ class course_modinfo {
     public static function purge_course_section_cache_by_id(int $courseid, int $sectionid): void {
         $course = get_course($courseid);
         $cache = cache::make('core', 'coursemodinfo');
-        $cache->acquire_lock($course->id);
-        $coursemodinfo = $cache->get_versioned($course->id, $course->cacherev);
+        $cachekey = $course->id;
+        $cache->acquire_lock($cachekey);
+        $coursemodinfo = $cache->get_versioned($cachekey, $course->cacherev);
         if ($coursemodinfo !== false) {
             foreach ($coursemodinfo->sectioncache as $sectionno => $sectioncache) {
                 if ($sectioncache->id == $sectionid) {
                     $coursemodinfo->cacherev = -1;
                     unset($coursemodinfo->sectioncache[$sectionno]);
-                    $cache->set_versioned($course->id, $course->cacherev, $coursemodinfo);
+                    $cache->set_versioned($cachekey, $course->cacherev, $coursemodinfo);
                     break;
                 }
             }
         }
-        $cache->release_lock($course->id);
+        $cache->release_lock($cachekey);
     }
 
     /**
@@ -748,14 +726,15 @@ class course_modinfo {
     public static function purge_course_section_cache_by_number(int $courseid, int $sectionno): void {
         $course = get_course($courseid);
         $cache = cache::make('core', 'coursemodinfo');
-        $cache->acquire_lock($course->id);
-        $coursemodinfo = $cache->get_versioned($course->id, $course->cacherev);
+        $cachekey = $course->id;
+        $cache->acquire_lock($cachekey);
+        $coursemodinfo = $cache->get_versioned($cachekey, $course->cacherev);
         if ($coursemodinfo !== false && array_key_exists($sectionno, $coursemodinfo->sectioncache)) {
             $coursemodinfo->cacherev = -1;
             unset($coursemodinfo->sectioncache[$sectionno]);
-            $cache->set_versioned($course->id, $course->cacherev, $coursemodinfo);
+            $cache->set_versioned($cachekey, $course->cacherev, $coursemodinfo);
         }
-        $cache->release_lock($course->id);
+        $cache->release_lock($cachekey);
     }
 
     /**
@@ -767,16 +746,17 @@ class course_modinfo {
     public static function purge_course_module_cache(int $courseid, int $cmid): void {
         $course = get_course($courseid);
         $cache = cache::make('core', 'coursemodinfo');
-        $cache->acquire_lock($course->id);
-        $coursemodinfo = $cache->get_versioned($course->id, $course->cacherev);
+        $cachekey = $course->id;
+        $cache->acquire_lock($cachekey);
+        $coursemodinfo = $cache->get_versioned($cachekey, $course->cacherev);
         $hascache = ($coursemodinfo !== false) && array_key_exists($cmid, $coursemodinfo->modinfo);
         if ($hascache) {
             $coursemodinfo->cacherev = -1;
             unset($coursemodinfo->modinfo[$cmid]);
-            $cache->set_versioned($course->id, $course->cacherev, $coursemodinfo);
-            $coursemodinfo = $cache->get_versioned($course->id, $course->cacherev);
+            $cache->set_versioned($cachekey, $course->cacherev, $coursemodinfo);
+            $coursemodinfo = $cache->get_versioned($cachekey, $course->cacherev);
         }
-        $cache->release_lock($course->id);
+        $cache->release_lock($cachekey);
     }
 
     /**
diff --git a/lib/tests/modinfolib_test.php b/lib/tests/modinfolib_test.php
index 54a423c4ce8..0f8ffd3a584 100644
--- a/lib/tests/modinfolib_test.php
+++ b/lib/tests/modinfolib_test.php
@@ -274,7 +274,9 @@ class modinfolib_test extends advanced_testcase {
         $prevcacherev = $cacherev;
 
         // Little trick to check that cache is not rebuilt druing the next step - substitute the value in MUC and later check that it is still there.
+        $cache->acquire_lock($course->id);
         $cache->set_versioned($course->id, $cacherev, (object)array_merge((array)$cachedvalue, array('secretfield' => 1)));
+        $cache->release_lock($course->id);
 
         // Clear static cache and call get_fast_modinfo() again (pretend we are in another request). Cache should not be rebuilt.
         course_modinfo::clear_instance_cache();
diff --git a/lib/upgrade.txt b/lib/upgrade.txt
index 3f69a5e0cee..82f607527d6 100644
--- a/lib/upgrade.txt
+++ b/lib/upgrade.txt
@@ -9,6 +9,9 @@ Declaration is as follow:
     $deprecatedcapabilities = [
         'fake/access:fakecapability' => ['replacement' => '', 'message' => 'This capability should not be used anymore.']
     ];
+* coursemodinfo cache uses the new `requirelockingbeforewrite` option, and rebuilding the cache now uses the cache lock API, rather
+  than using the core lock factory directly. This allows the locks to be stored locally if the cache is stored locally, and
+  avoids the risk of delays and timeouts when multiple nodes need to rebuild the cache locally, but are waiting for a central lock.
 
 === 4.1 ===
 
diff --git a/version.php b/version.php
index 7c4ce7c3ac9..ef537007bf0 100644
--- a/version.php
+++ b/version.php
@@ -29,7 +29,7 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$version  = 2022101800.00;              // YYYYMMDD      = weekly release date of this DEV branch.
+$version  = 2022101800.01;              // YYYYMMDD      = weekly release date of this DEV branch.
                                         //         RR    = release increments - 00 in DEV branches.
                                         //           .XX = incremental changes.
 $release  = '4.1dev+ (Build: 20221018)'; // Human-friendly version name

From 2f1e7982df1b44246ed33e1f7d2fff90fa5a8d58 Mon Sep 17 00:00:00 2001
From: Mark Johnson <mark.johnson@catalyst-eu.net>
Date: Mon, 15 Aug 2022 15:01:58 +0100
Subject: [PATCH 6/6] MDL-67020 Cache: Refactor build_course_cache

---
 lib/modinfolib.php | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/lib/modinfolib.php b/lib/modinfolib.php
index 94f2dfb4452..d0eb3e29b8b 100644
--- a/lib/modinfolib.php
+++ b/lib/modinfolib.php
@@ -475,18 +475,7 @@ class course_modinfo {
         // partial rebuild logic sometimes sets the $coursemodinfo->cacherev to -1 which is an
         // indicator that it needs rebuilding.
         if ($coursemodinfo === false || ($course->cacherev > $coursemodinfo->cacherev)) {
-            $cachekey = $course->id;
-            $lock = $cachecoursemodinfo->acquire_lock($cachekey);
-            try {
-                // Only actually do the build if it's still needed after getting the lock (not if
-                // somebody else, who might have been holding the lock, built it already).
-                $coursemodinfo = $cachecoursemodinfo->get_versioned($course->id, $course->cacherev);
-                if ($coursemodinfo === false || ($course->cacherev > $coursemodinfo->cacherev)) {
-                    $coursemodinfo = self::inner_build_course_cache($course, $lock);
-                }
-            } finally {
-                $cachecoursemodinfo->release_lock($cachekey);
-            }
+            $coursemodinfo = self::build_course_cache($course);
         }
 
         // Set initial values
@@ -653,10 +642,16 @@ class course_modinfo {
         $cachekey = $course->id;
         $cachecoursemodinfo->acquire_lock($cachekey);
         try {
-            return self::inner_build_course_cache($course, $partialrebuild);
+            // Only actually do the build if it's still needed after getting the lock (not if
+            // somebody else, who might have been holding the lock, built it already).
+            $coursemodinfo = $cachecoursemodinfo->get_versioned($course->id, $course->cacherev);
+            if ($coursemodinfo === false || ($course->cacherev > $coursemodinfo->cacherev)) {
+                $coursemodinfo = self::inner_build_course_cache($course);
+            }
         } finally {
             $cachecoursemodinfo->release_lock($cachekey);
         }
+        return $coursemodinfo;
     }
 
     /**