diff --git a/system/Session/Handlers/RedisHandler.php b/system/Session/Handlers/RedisHandler.php index b429792929b4..5a81e6a4da00 100644 --- a/system/Session/Handlers/RedisHandler.php +++ b/system/Session/Handlers/RedisHandler.php @@ -115,6 +115,8 @@ protected function setSavePath(): void * * @param string $path The path where to store/retrieve the session * @param string $name The session name + * + * @throws RedisException */ public function open($path, $name): bool { @@ -124,12 +126,20 @@ public function open($path, $name): bool $redis = new Redis(); - if (! $redis->connect($this->savePath['protocol'] . '://' . $this->savePath['host'], ($this->savePath['host'][0] === '/' ? 0 : $this->savePath['port']), $this->savePath['timeout'])) { + if ( + ! $redis->connect( + $this->savePath['protocol'] . '://' . $this->savePath['host'], + ($this->savePath['host'][0] === '/' ? 0 : $this->savePath['port']), + $this->savePath['timeout'] + ) + ) { $this->logger->error('Session: Unable to connect to Redis with the configured settings.'); } elseif (isset($this->savePath['password']) && ! $redis->auth($this->savePath['password'])) { $this->logger->error('Session: Unable to authenticate to Redis instance.'); } elseif (isset($this->savePath['database']) && ! $redis->select($this->savePath['database'])) { - $this->logger->error('Session: Unable to select Redis database with index ' . $this->savePath['database']); + $this->logger->error( + 'Session: Unable to select Redis database with index ' . $this->savePath['database'] + ); } else { $this->redis = $redis; @@ -146,6 +156,8 @@ public function open($path, $name): bool * * @return false|string Returns an encoded string of the read data. * If nothing was read, it must return false. + * + * @throws RedisException */ #[ReturnTypeWillChange] public function read($id) @@ -168,7 +180,7 @@ public function read($id) return $data; } - return ''; + return false; } /** @@ -176,6 +188,8 @@ public function read($id) * * @param string $id The session ID * @param string $data The encoded session data + * + * @throws RedisException */ public function write($id, $data): bool { @@ -222,8 +236,8 @@ public function close(): bool $pingReply = $this->redis->ping(); if (($pingReply === true) || ($pingReply === '+PONG')) { - if (isset($this->lockKey)) { - $this->releaseLock(); + if (isset($this->lockKey) && ! $this->releaseLock()) { + return false; } if (! $this->redis->close()) { @@ -246,12 +260,16 @@ public function close(): bool * Destroys a session * * @param string $id The session ID being destroyed + * + * @throws RedisException */ public function destroy($id): bool { if (isset($this->redis, $this->lockKey)) { if (($result = $this->redis->del($this->keyPrefix . $id)) !== 1) { - $this->logger->debug('Session: Redis::del() expected to return 1, got ' . var_export($result, true) . ' instead.'); + $this->logger->debug( + 'Session: Redis::del() expected to return 1, got ' . var_export($result, true) . ' instead.' + ); } return $this->destroyCookie(); @@ -278,6 +296,8 @@ public function gc($max_lifetime) * Acquires an emulated lock. * * @param string $sessionID Session ID + * + * @throws RedisException */ protected function lockSession(string $sessionID): bool { @@ -287,41 +307,40 @@ protected function lockSession(string $sessionID): bool // so we need to check here if the lock key is for the // correct session ID. if ($this->lockKey === $lockKey) { + // If there is the lock, make the ttl longer. return $this->redis->expire($this->lockKey, 300); } $attempt = 0; do { - $ttl = $this->redis->ttl($lockKey); - assert(is_int($ttl)); + $result = $this->redis->set( + $lockKey, + (string) Time::now()->getTimestamp(), + // NX -- Only set the key if it does not already exist. + // EX seconds -- Set the specified expire time, in seconds. + ['nx', 'ex' => 300] + ); - if ($ttl > 0) { - sleep(1); + if (! $result) { + usleep(100000); continue; } - if (! $this->redis->setex($lockKey, 300, (string) Time::now()->getTimestamp())) { - $this->logger->error('Session: Error while trying to obtain lock for ' . $this->keyPrefix . $sessionID); - - return false; - } - $this->lockKey = $lockKey; break; - } while (++$attempt < 30); + } while (++$attempt < 300); - if ($attempt === 30) { - log_message('error', 'Session: Unable to obtain lock for ' . $this->keyPrefix . $sessionID . ' after 30 attempts, aborting.'); + if ($attempt === 300) { + $this->logger->error( + 'Session: Unable to obtain lock for ' . $this->keyPrefix . $sessionID + . ' after 300 attempts, aborting.' + ); return false; } - if ($ttl === -1) { - log_message('debug', 'Session: Lock for ' . $this->keyPrefix . $sessionID . ' had no TTL, overriding.'); - } - $this->lock = true; return true; @@ -329,6 +348,8 @@ protected function lockSession(string $sessionID): bool /** * Releases a previously acquired lock + * + * @throws RedisException */ protected function releaseLock(): bool {