Skip to content

Patch for colinmollenhour/php-redis-session-abstract 1.5.5 #1

Description

@peterjaap

Which is the version Magento 2.4.7 runs.

Untested, I just manually worked through the diff.

From 9dd3a4a145f69ddf823824e1d7ba06b97d17f1d8 Mon Sep 17 00:00:00 2001
From: peterjaap <peterjaap@elgentos.nl>
Date: Tue, 9 Sep 2025 14:14:53 +0200
Subject: [PATCH] Implement lock write patch


diff --git src/Cm/RedisSession/Handler.php src/Cm/RedisSession/Handler.php
index beccd8e..fed7b28 100644
--- src/Cm/RedisSession/Handler.php
+++ src/Cm/RedisSession/Handler.php
@@ -255,6 +255,11 @@ class Handler implements \SessionHandlerInterface
      */
     private $_readOnly;
 
+    /**
+     * @var array
+     */
+    private $_lastReadSessionDataById = [];
+
     /**
      * @param ConfigInterface $config
      * @param LoggerInterface $logger
@@ -263,6 +268,12 @@ class Handler implements \SessionHandlerInterface
      */
     public function __construct(ConfigInterface $config, LoggerInterface $logger, $readOnly = false)
     {
+        // we must be able to manipulate session data easily using native serialize/unserialize functions
+        // and as long as the default php session serializer doesn't handle object references correctly, we have no choice forcing this option
+        if (ini_get('session.serialize_handler') !== 'php_serialize') {
+            throw new \Exception('You must set session.serialize_handler ini config to "php_serialize" value');
+        }
+
         $this->config = $config;
         $this->logger = $logger;
 
@@ -316,14 +327,14 @@ class Handler implements \SessionHandlerInterface
                         } catch (\CredisException $e) {
                             // Prevent throwing exception if Sentinel has no password set (error messages are different between redis 5 and redis 6)
                             if ($e->getCode() !== 0 || (
-                                strpos($e->getMessage(), 'ERR Client sent AUTH, but no password is set') === false && 
+                                strpos($e->getMessage(), 'ERR Client sent AUTH, but no password is set') === false &&
                                 strpos($e->getMessage(), 'ERR AUTH <password> called without any password configured for the default user. Are you sure your configuration is correct?') === false)
                             ) {
                                 throw $e;
                             }
                         }
                     }
-                   
+
                     $sentinel = new \Credis_Sentinel($sentinelClient);
                     $sentinel
                         ->setClientTimeout($timeout)
@@ -434,17 +445,17 @@ class Handler implements \SessionHandlerInterface
     }
 
     /**
-     * Fetch session data
+     * Acquire lock
      *
      * @param string $sessionId
      * @return string
      * @throws ConcurrentConnectionsExceededException
      */
     #[\ReturnTypeWillChange]
-    public function read($sessionId)
+    protected function _lock($sessionId)
     {
         // Get lock on session. Increment the "lock" field and if the new value is 1, we have the lock.
-        $sessionId = self::SESSION_PREFIX.$sessionId;
+        $sessionId = self::SESSION_PREFIX . $sessionId;
         $tries = $waiting = $lock = 0;
         $lockPid = $oldLockPid = null; // Restart waiting for lock when current lock holder changes
         $detectZombies = false;
@@ -453,11 +464,10 @@ class Handler implements \SessionHandlerInterface
         $this->_log(sprintf("Attempting to take lock on ID %s", $sessionId));
 
         $this->_redis->select($this->_dbNum);
-        while ($this->_useLocking && !$this->_readOnly)
-        {
+        while (!$this->_readOnly) {
             // Increment lock value for this session and retrieve the new value
             $oldLock = $lock;
-            $lock = $this->_redis->hIncrBy($sessionId, 'lock', 1);
+            $lock    = $this->_redis->hIncrBy($sessionId, 'lock', 1);
 
             // Get the pid of the process that has the lock
             if ($lock != 1 && $tries + 1 >= $breakAfter) {
@@ -465,25 +475,54 @@ class Handler implements \SessionHandlerInterface
             }
 
             // If we got the lock, update with our pid and reset lock and expiration
-            if (   $lock == 1                          // We actually do have the lock
+            if ($lock == 1                          // We actually do have the lock
                 || (
                     $tries >= $breakAfter   // We are done waiting and want to start trying to break it
                     && $oldLockPid == $lockPid        // Nobody else got the lock while we were waiting
                 )
             ) {
                 $this->_hasLock = true;
-                break;
-            }
 
-            // Otherwise, add to "wait" counter and continue
-            else if ( ! $waiting) {
-                $i = 0;
-                do {
-                    $waiting = $this->_redis->hIncrBy($sessionId, 'wait', 1);
-                } while (++$i < $this->_maxConcurrency && $waiting < 1);
-            }
+                $setData = [
+                    'pid' => $this->_getPid(),
+                    'lock' => 1,
+                ];
+
+                // Save request data in session so if a lock is broken we can know which page it was for debugging
+                if (empty($_SERVER['REQUEST_METHOD'])) {
+                    $setData['req'] = @$_SERVER['SCRIPT_NAME'];
+                } else {
+                    $setData['req'] = $_SERVER['REQUEST_METHOD'] . " " . @$_SERVER['SERVER_NAME'] . @$_SERVER['REQUEST_URI'];
+                }
+                if ($lock != 1) {
+                    $this->_log(
+                        sprintf(
+                            "Successfully broke lock for ID %s after %.5f seconds (%d attempts). Lock: %d\nLast request of broken lock: %s",
+                            $sessionId,
+                            (microtime(true) - $timeStart),
+                            $tries,
+                            $lock,
+                            $this->_redis->hGet($sessionId, 'req')
+                        ),
+                        LoggerInterface::INFO
+                    );
+                }
+                // Set session data and expiration
+                $this->_redis->pipeline();
+                if (!empty($setData)) {
+                    $this->_redis->hMSet($sessionId, $setData);
+                }
+                $this->_redis->expire($sessionId,
+                    3600 * 6); // Expiration will be set to correct value when session is written
+
+                // This process is no longer waiting for a lock
+                if ($tries > 0) {
+                    $this->_redis->hIncrBy($sessionId, 'wait', -1);
+                }
+                $this->_redis->exec();
 
-            // Handle overloaded sessions
+                break;
+            } // Otherwise, add to "wait" counter and continue
             else {
                 // Detect broken sessions (e.g. caused by fatal errors)
                 if ($detectZombies) {
@@ -502,7 +541,8 @@ class Handler implements \SessionHandlerInterface
                             ),
                             LoggerInterface::INFO
                         );
-                        $waiting = $this->_redis->hIncrBy($sessionId, 'wait', -1);
+                        $waiting = $this->_redis->hIncrBy($sessionId,
+                            'wait', -1);
                         continue;
                     }
                 }
@@ -510,21 +550,14 @@ class Handler implements \SessionHandlerInterface
                 // Limit concurrent lock waiters to prevent server resource hogging
                 if ($waiting >= $this->_maxConcurrency) {
                     // Overloaded sessions get 503 errors
-                    try {
-                        $this->_redis->hIncrBy($sessionId, 'wait', -1);
-                        $this->_sessionWritten = true; // Prevent session from getting written
-                        $sessionInfo = $this->_redis->hMGet($sessionId, ['writes','req']);
-                    } catch (Exception $e) {
-                        $this->_log("$e", LoggerInterface::WARNING);
-                    }
+                    $this->_redis->hIncrBy($sessionId, 'wait', -1);
+                    $this->_sessionWritten = true; // Prevent session from getting written
+                    $writes                = $this->_redis->hGet($sessionId,
+                        'writes');
                     $this->_log(
                         sprintf(
-                            'Session concurrency exceeded for ID %s; displaying HTTP 503 (%s waiting, %s total '
-                            . 'requests) - Locked URL: %s',
-                            $sessionId,
-                            $waiting,
-                            isset($sessionInfo['writes']) ? $sessionInfo['writes'] : '-',
-                            isset($sessionInfo['req']) ? $sessionInfo['req'] : '-'
+                            'Session concurrency exceeded for ID %s; displaying HTTP 503 (%s waiting, %s total requests)',
+                            $sessionId, $waiting, $writes
                         ),
                         LoggerInterface::WARNING
                     );
@@ -534,23 +567,24 @@ class Handler implements \SessionHandlerInterface
 
             $tries++;
             $oldLockPid = $lockPid;
-            $sleepTime = self::SLEEP_TIME;
+            $sleepTime  = self::SLEEP_TIME;
 
             // Detect dead lock waiters
             if ($tries % self::DETECT_ZOMBIES == 1) {
                 $detectZombies = true;
-                $sleepTime += 10000; // sleep + 0.01 seconds
+                $sleepTime     += 10000; // sleep + 0.01 seconds
             }
             // Detect dead lock holder every 10 seconds (only works on same node as lock holder)
             if ($tries % self::DETECT_ZOMBIES == 0) {
                 $this->_log(
                     sprintf(
-                        "Checking for zombies after %.5f seconds of waiting...", (microtime(true) - $timeStart)
+                        "Checking for zombies after %.5f seconds of waiting...",
+                        (microtime(true) - $timeStart)
                     )
                 );
 
                 $pid = $this->_redis->hGet($sessionId, 'pid');
-                if ($pid && ! $this->_pidExists($pid)) {
+                if ($pid && !$this->_pidExists($pid)) {
                     // Allow a live process to get the lock
                     $this->_redis->hSet($sessionId, 'lock', 0);
                     $this->_log(
@@ -576,8 +610,7 @@ class Handler implements \SessionHandlerInterface
                     LoggerInterface::NOTICE
                 );
                 break;
-            }
-            else {
+            } else {
                 $this->_log(
                     sprintf(
                         "Waiting %.2f seconds for lock on ID %s (%d tries, lock pid is %s, %.5f seconds elapsed)",
@@ -592,59 +625,32 @@ class Handler implements \SessionHandlerInterface
             }
         }
         $this->failedLockAttempts = $tries;
+    }
 
-        // Session can be read even if it was not locked by this pid!
+    /**
+     * Fetch session data
+     *
+     * @param string $sessionId
+     * @return string
+     * @throws ConcurrentConnectionsExceededException
+     */
+    #[\ReturnTypeWillChange]
+    public function read($sessionId)
+    {
+        $sessionId = self::SESSION_PREFIX . $sessionId;
         $timeStart2 = microtime(true);
-        list($sessionData, $sessionWrites) = array_values($this->_redis->hMGet($sessionId, array('data','writes')));
+        $this->_redis->select($this->_dbNum);
+        [$sessionData, $sessionWrites] = array_values($this->_redis->hMGet($sessionId, array('data','writes')));
         $this->_log(sprintf("Data read for ID %s in %.5f seconds", $sessionId, (microtime(true) - $timeStart2)));
         $this->_sessionWrites = (int) $sessionWrites;
 
-        // This process is no longer waiting for a lock
-        if ($tries > 0) {
-            $this->_redis->hIncrBy($sessionId, 'wait', -1);
-        }
-
-        // This process has the lock, save the pid
-        if ($this->_hasLock) {
-            $setData = array(
-                'pid' => $this->_getPid(),
-                'lock' => 1,
-            );
-
-            // Save request data in session so if a lock is broken we can know which page it was for debugging
-            if (empty($_SERVER['REQUEST_METHOD'])) {
-                $setData['req'] = @$_SERVER['SCRIPT_NAME'];
-            } else {
-                $setData['req'] = $_SERVER['REQUEST_METHOD']." ".@$_SERVER['SERVER_NAME'].@$_SERVER['REQUEST_URI'];
-            }
-            if ($lock != 1) {
-                $this->_log(
-                    sprintf(
-                        "Successfully broke lock for ID %s after %.5f seconds (%d attempts). Lock: %d\nLast request of "
-                            . "broken lock: %s",
-                        $sessionId,
-                        (microtime(true) - $timeStart),
-                        $tries,
-                        $lock,
-                        $this->_redis->hGet($sessionId, 'req')
-                    ),
-                    LoggerInterface::INFO
-                );
-            }
-        }
-
-        // Set session data and expiration
-        $this->_redis->pipeline();
-        if ( ! empty($setData)) {
-            $this->_redis->hMSet($sessionId, $setData);
-        }
-        $this->_redis->expire($sessionId, 3600*6); // Expiration will be set to correct value when session is written
-        $this->_redis->exec();
+        $sessionData = $sessionData ? (string) $this->_decodeData($sessionData) : '';
+        $this->_lastReadSessionDataById[$sessionId] = $sessionData;
 
         // Reset flag in case of multiple session read/write operations
         $this->_sessionWritten = false;
 
-        return $sessionData ? (string) $this->_decodeData($sessionData) : '';
+        return $sessionData;
     }
 
     /**
@@ -661,6 +667,11 @@ class Handler implements \SessionHandlerInterface
             $this->_log(sprintf(($this->_sessionWritten ? "Repeated" : "Read-only") . " session write detected; skipping for ID %s", $sessionId));
             return true;
         }
+
+        if ($this->_useLocking) {
+            $this->_lock($sessionId);
+        }
+
         $this->_sessionWritten = true;
         $timeStart = microtime(true);
 
@@ -671,6 +682,8 @@ class Handler implements \SessionHandlerInterface
             if ( ! $this->_useLocking
                 || ( ! ($pid = $this->_redis->hGet('sess_'.$sessionId, 'pid')) || $pid == $this->_getPid())
             ) {
+                $sessionData = $this->_getRefreshedSessionDataToWrite($sessionId);
+
                 $this->_writeRawSession($sessionId, $sessionData, $this->getLifeTime());
                 $this->_log(sprintf("Data written to ID %s in %.5f seconds", $sessionId, (microtime(true) - $timeStart)));
 
@@ -902,7 +915,7 @@ class Handler implements \SessionHandlerInterface
      */
     protected function _pidExists($pid)
     {
-        list($host,$pid) = explode('|', $pid);
+        [$host,$pid] = explode('|', $pid);
         if (PHP_OS != 'Linux' || $host != gethostname()) {
             return true;
         }
@@ -926,4 +939,98 @@ class Handler implements \SessionHandlerInterface
 
         return $this->_breakAfter;
     }
+
+    /**
+     * Get fresh session data, calculate a diff from last read session and session to write, and apply it onto the fresh
+     *
+     * @param string $sessionId
+     * @return string
+     */
+    protected function _getRefreshedSessionDataToWrite(string $sessionId): string
+    {
+        $currentSessionData = $this->_decodeData($this->_redis->hGet('sess_' . $sessionId, 'data'));
+        $lastReadSessionData = $this->_lastReadSessionDataById['sess_' . $sessionId];
+
+        $freshSession = unserialize($currentSessionData) ?: [];
+        $lastReadSession = unserialize($lastReadSessionData) ?: [];
+
+        $diffToUnset = $this->_arrayDiffRecursive($lastReadSession, $_SESSION);
+        if (!empty($diffToUnset)) {
+            $freshSession = $this->_arrayRemoveRecursive($freshSession, $diffToUnset);
+        }
+
+        $diffToSet = $this->_arrayDiffRecursive($_SESSION, $lastReadSession);
+        if (!empty($diffToSet)) {
+            $freshSession = array_replace_recursive($freshSession, $diffToSet);
+        }
+
+        return serialize($freshSession);
+    }
+
+    /**
+     * Remove array elements by key recursively (deep down first and removing empty arrays)
+     *
+     * @param array $arr1
+     * @param array $arr2
+     * @return array
+     */
+    protected function _arrayRemoveRecursive($arr1, $arr2)
+    {
+        foreach ($arr2 as $key => $item) {
+            if (is_array($arr2[$key])) {
+                $arr1[$key] = $this->_arrayRemoveRecursive($arr1[$key], $arr2[$key]);
+
+                if (count($arr1[$key]) === 0) {
+                    unset($arr1[$key]);
+                }
+            } else {
+                unset($arr1[$key]);
+            }
+        }
+
+        return $arr1;
+    }
+
+    /**
+     * Array diff recursively
+     *
+     * @param array $arr1
+     * @param array $arr2
+     * @return array
+     */
+    protected function _arrayDiffRecursive($arr1, $arr2)
+    {
+        $outputDiff = [];
+
+        foreach ($arr1 as $key => $value) {
+            if (!array_key_exists($key, $arr2)) {
+                $outputDiff[$key] = $value;
+                continue;
+            }
+
+            if ((is_array($value) && !is_array($arr2[$key]))
+                || (is_object($value) && !is_object($arr2[$key]))) {
+                $outputDiff[$key] = $value;
+                continue;
+            }
+
+            if (is_array($value)) {
+                $recursiveDiff = $this->_arrayDiffRecursive($value, $arr2[$key]);
+
+                if (count($recursiveDiff)) {
+                    $outputDiff[$key] = $recursiveDiff;
+                }
+            } elseif (is_object($value)) {
+                if (serialize($value) !== serialize($arr2[$key])) {
+                    $outputDiff[$key] = $value;
+                }
+            } else {
+                if ($value !== $arr2[$key]) {
+                    $outputDiff[$key] = $value;
+                }
+            }
+        }
+
+        return $outputDiff;
+    }
 }

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions