diff --git a/system/Session/Handlers/RedisHandler.php b/system/Session/Handlers/RedisHandler.php index 4c8f78ba3894..54f628c4789b 100644 --- a/system/Session/Handlers/RedisHandler.php +++ b/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; diff --git a/tests/system/Session/Handlers/Database/RedisHandlerTest.php b/tests/system/Session/Handlers/Database/RedisHandlerTest.php index 2e7821446fe3..637132669aec 100644 --- a/tests/system/Session/Handlers/Database/RedisHandlerTest.php +++ b/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(); + } } diff --git a/user_guide_src/source/changelogs/v4.7.4.rst b/user_guide_src/source/changelogs/v4.7.4.rst index 87029861aaa0..284cc112183a 100644 --- a/user_guide_src/source/changelogs/v4.7.4.rst +++ b/user_guide_src/source/changelogs/v4.7.4.rst @@ -35,6 +35,7 @@ Bugs Fixed - **Database:** Fixed a bug where ``updateBatch()`` could be called after Query Builder ``where()`` conditions, even though it's not supported. In this situation, now the ``DatabaseException`` is thrown. - **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 diff --git a/utils/phpstan-baseline/property.notFound.neon b/utils/phpstan-baseline/property.notFound.neon index 9e6fc64adb15..b64acebff15b 100644 --- a/utils/phpstan-baseline/property.notFound.neon +++ b/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