From 3460632bf1441a16c49f48548739dce7f25dfe23 Mon Sep 17 00:00:00 2001
From: Daniel Ziegenberg <daniel@ziegenberg.at>
Date: Wed, 27 Mar 2024 13:47:21 +0100
Subject: [PATCH] MDL-69684 session: Redis session locks set with expiry
 atomically

Co-Authored-By: Jamie Chapman-Brown <jamie.chapman-brown@agilicus.com>
Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
---
 lib/classes/session/redis.php | 29 ++++++++++++++++++++++-------
 lib/upgrade.txt               |  1 +
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/lib/classes/session/redis.php b/lib/classes/session/redis.php
index c3b623d78f2..4dae1715db8 100644
--- a/lib/classes/session/redis.php
+++ b/lib/classes/session/redis.php
@@ -53,6 +53,15 @@ class redis extends handler {
      */
     const COMPRESSION_ZSTD      = 'zstd';
 
+    /**
+     * Minimum version of the Redis extension required.
+     */
+    private const REDIS_EXTENSION_MIN_VERSION = '2.2.4';
+    /**
+     * Minimum version of the Redis extension required.
+     */
+    private const REDIS_SERVER_MIN_VERSION = '2.6.12';
+
     /** @var string $host save_path string  */
     protected $host = '';
     /** @var int $port The port to connect to */
@@ -193,13 +202,14 @@ class redis extends handler {
 
         if (empty($this->host)) {
             throw new exception('sessionhandlerproblem', 'error', '', null,
-                    '$CFG->session_redis_host must be specified in config.php');
+                '$CFG->session_redis_host must be specified in config.php');
         }
 
-        // The session handler requires a version of Redis with the SETEX command (at least 2.0).
+        // The session handler requires a version of PHP Redis extension with support for SET command options (at least 2.2.4).
         $version = phpversion('Redis');
-        if (!$version or version_compare($version, '2.0') <= 0) {
-            throw new exception('sessionhandlerproblem', 'error', '', null, 'redis extension version must be at least 2.0');
+        if (!$version || version_compare($version, self::REDIS_EXTENSION_MIN_VERSION) <= 0) {
+            throw new exception('sessionhandlerproblem', 'error', '', null,
+                'redis extension version must be at least ' . self::REDIS_EXTENSION_MIN_VERSION);
         }
 
         $this->connection = new \Redis();
@@ -265,6 +275,13 @@ class redis extends handler {
                         throw new RedisException('Unable to select Redis database '.$this->database.'.');
                     }
                 }
+
+                // The session handler requires a version of Redis server with support for SET command options (at least 2.6.12).
+                $serverversion = $this->connection->info('server')['redis_version'];
+                if (version_compare($serverversion, self::REDIS_SERVER_MIN_VERSION) <= 0) {
+                    throw new exception('sessionhandlerproblem', 'error', '', null,
+                        'redis server version must be at least ' . self::REDIS_SERVER_MIN_VERSION);
+                }
                 return true;
             } catch (RedisException $e) {
                 $logstring = "Failed to connect (try {$counter} out of {$maxnumberofretries}) to redis ";
@@ -507,12 +524,10 @@ class redis extends handler {
         $haswarned = false; // Have we logged a lock warning?
 
         while (!$haslock) {
-
-            $haslock = $this->connection->setnx($lockkey, $whoami);
+            $haslock = $this->connection->set($lockkey, $whoami, ['nx', 'ex' => $this->lockexpire]);
 
             if ($haslock) {
                 $this->locks[$id] = $this->time() + $this->lockexpire;
-                $this->connection->expire($lockkey, $this->lockexpire);
                 return true;
             }
 
diff --git a/lib/upgrade.txt b/lib/upgrade.txt
index 689faaea1b5..ab783e76361 100644
--- a/lib/upgrade.txt
+++ b/lib/upgrade.txt
@@ -3,6 +3,7 @@ information provided here is intended especially for developers.
 
 === 4.3.6 ===
 
+* Redis session cache has been improved to make a single call where two were used before. The minimum redis version is now 2.6.12.
 * The `\core\dataformat::get_format_instance` method is now public, and can be used to retrieve a writer instance for
   a given dataformat
 * Added the ability for unit tests to autoload classes in the `\[component]\tests\` namespace from the `[path/to/component]/tests/classes` directory.