fix: use dynamic lockMaxRetries limit in RedisHandler (#10295)

* fix: use dynamic lockMaxRetries limit in RedisHandler

* fix: read correct config property names in RedisHandler lockSession

* fix: add lockMaxRetries test and remove stale PHPStan baseline entries

* fix: add setLogger to RedisHandlerTest to prevent null logger error

* Update system/Session/Handlers/RedisHandler.php

Co-authored-by: Michal Sniatala <michal@sniatala.pl>

* Fix PHPStan warnings for RedisHandler and update changelog

* Update user_guide_src/source/changelogs/v4.7.4.rst

Co-authored-by: Michal Sniatala <michal@sniatala.pl>

* docs: alphabetize changelog entries and properties in v4.7.4.rst

---------

Co-authored-by: Michal Sniatala <michal@sniatala.pl>

Author: gr8man

system/Session/Handlers/RedisHandler.php

@@ -98,8 +98,8 @@ public function __construct(SessionConfig $config, string $ipAddress)
             $this->keyPrefix .= $this->ipAddress . ':';
         }
 
-        $this->lockRetryInterval = $config->lockWait ?? $this->lockRetryInterval;
-        $this->lockMaxRetries    = $config->lockAttempts ?? $this->lockMaxRetries;
+        $this->lockRetryInterval = $config->lockRetryInterval ?? $this->lockRetryInterval; // @phpstan-ignore nullCoalesce.property
+        $this->lockMaxRetries    = $config->lockMaxRetries ?? $this->lockMaxRetries; // @phpstan-ignore nullCoalesce.property
     }
 
     protected function setSavePath(): void
@@ -386,10 +386,10 @@ protected function lockSession(string $sessionID): bool
             break;
         } while (++$attempt < $this->lockMaxRetries);
 
-        if ($attempt === 300) {
+        if ($attempt === $this->lockMaxRetries) {
             $this->logger->error(
                 'Session: Unable to obtain lock for ' . $this->keyPrefix . $sessionID
-                . ' after 300 attempts, aborting.',
+                . ' after ' . $this->lockMaxRetries . ' attempts, aborting.',
             );
 
             return false;

tests/system/Session/Handlers/Database/RedisHandlerTest.php

@@ -15,6 +15,8 @@
 
 use CodeIgniter\Session\Handlers\RedisHandler;
 use CodeIgniter\Test\CIUnitTestCase;
+use CodeIgniter\Test\TestLogger;
+use Config\Logger as LoggerConfig;
 use Config\Session as SessionConfig;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Group;
@@ -54,7 +56,10 @@ protected function getInstance($options = []): RedisHandler
             $sessionConfig->{$key} = $value;
         }
 
-        return new RedisHandler($sessionConfig, $this->userIpAddress);
+        $handler = new RedisHandler($sessionConfig, $this->userIpAddress);
+        $handler->setLogger(new TestLogger(new LoggerConfig()));
+
+        return $handler;
     }
 
     protected function tearDown(): void
@@ -359,4 +364,26 @@ public function testResetPersistentConnections(): void
         $handler1->close();
         $handler2->close();
     }
+
+    public function testLockMaxRetries(): void
+    {
+        $options = [
+            'lockRetryInterval' => 10_000, // 10ms
+            'lockMaxRetries'    => 3,
+        ];
+
+        $handler1 = $this->getInstance($options);
+        $handler1->open($this->sessionSavePath, $this->sessionName);
+        $handler1->read('lock_test_session'); // Acquires lock
+
+        $handler2 = $this->getInstance($options);
+        $handler2->open($this->sessionSavePath, $this->sessionName);
+
+        // Before the fix, this would incorrectly return true (since $attempt === 3 !== 300).
+        // With the fix, it should return false after 3 attempts.
+        $this->assertFalse($handler2->read('lock_test_session'));
+
+        $handler1->close();
+        $handler2->close();
+    }
 }

user_guide_src/source/changelogs/v4.7.4.rst

@@ -36,6 +36,7 @@ Bugs Fixed
 - **Filters:** Fixed a bug in ``InvalidChars`` filter where invalid UTF-8 or control characters in array keys were not checked.
 - **HTTP:** Fixed a bug where the User Agent library reported Safari's WebKit version instead of the browser version from the ``Version`` token.
 - **Model:** Fixed a bug in ``Model::objectToRawArray()`` where the ``$recursive`` parameter was ignored.
+- **Session:** Fixed a bug in ``RedisHandler`` where the configured ``$lockMaxRetries`` and ``$lockRetryInterval`` values were not respected when acquiring session locks.
 - **Testing:** Fixed a bug where using ``MockInputOutput`` within a test that also uses ``StreamFilterTrait`` tore down the trait's stream filters, so CLI output produced after the ``MockInputOutput`` interaction (such as in ``tearDown()``) was no longer captured and leaked to the console.
 
 See the repo's

utils/phpstan-baseline/property.notFound.neon

@@ -1,4 +1,4 @@
-# total 49 errors
+# total 47 errors
 
 parameters:
     ignoreErrors:
@@ -17,16 +17,6 @@ parameters:
             count: 14
             path: ../../system/Database/SQLSRV/Forge.php
 
-        -
-            message: '#^Access to an undefined property Config\\Session\:\:\$lockAttempts\.$#'
-            count: 1
-            path: ../../system/Session/Handlers/RedisHandler.php
-
-        -
-            message: '#^Access to an undefined property Config\\Session\:\:\$lockWait\.$#'
-            count: 1
-            path: ../../system/Session/Handlers/RedisHandler.php
-
         -
             message: '#^Access to an undefined property CodeIgniter\\Database\\BaseConnection\:\:\$mysqli\.$#'
             count: 1