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

@@ -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 @@ public static function load($id, array $definition, $unused = null) {
         $requiremultipleidentifiers = false;
         $requirelockingread = false;
         $requirelockingwrite = false;
+        $requirelockingbeforewrite = false;
         $requiresearchable = ($mode === cache_store::MODE_SESSION) ? true : false;
         $maxsize = null;
         $overrideclass = null;
@@ -395,7 +402,14 @@ public static function load($id, array $definition, $unused = null) {
         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 @@ public static function load($id, array $definition, $unused = null) {
         $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 @@ public function require_locking_write() {
         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

cache/classes/helper.php

@@ -382,6 +382,7 @@ protected static function ensure_ready_for_stats($store, $storeclass, $definitio
                         'misses' => 0,
                         'sets' => 0,
                         'iobytes' => cache_store::IO_BYTES_NOT_SUPPORTED,
+                        'locks' => 0,
                     )
                 )
             );
@@ -392,6 +393,7 @@ protected static function ensure_ready_for_stats($store, $storeclass, $definitio
                 'misses' => 0,
                 'sets' => 0,
                 'iobytes' => cache_store::IO_BYTES_NOT_SUPPORTED,
+                'locks' => 0,
             );
         }
     }

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 @@ public function __construct(cache_definition $definition, cache_store $store, $l
             $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 @@ public function __clone() {
      * @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 @@ public function acquire_lock($key) {
      */
     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 @@ public function check_lock_state($key) {
     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 @@ protected function ensure_cachelock_available() {
      * @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 @@ protected function set_implementation($key, int $version, $data, bool $setparent
      *      ... 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) {

cache/disabledlib.php

@@ -200,6 +200,26 @@ public function has_any(array $keys) {
     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;
+    }
 }
 
 /**

cache/stores/file/addinstanceform.php

@@ -62,5 +62,10 @@ protected function configuration_definition() {
         $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');
     }
 }

cache/stores/file/lang/en/cachestore_file.php

@@ -32,6 +32,8 @@
 $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';

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 @@ public function __construct($name, array $configuration = array()) {
         } 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 @@ public static function config_get_configuration_array($data) {
         if (isset($data->asyncpurge)) {
             $config['asyncpurge'] = $data->asyncpurge;
         }
+        if (isset($data->lockwait)) {
+            $config['lockwait'] = $data->lockwait;
+        }
 
         return $config;
     }
@@ -702,6 +738,9 @@ public static function config_set_edit_form_data(moodleform $editform, array $co
         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 @@ public function cache_size_details(int $samplekeys = 50): stdClass {
         $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;
+    }
 }