[Doctrine Bridge] fix DBAL session handler according to PdoSessionHandler

src/Symfony/Bridge/Doctrine/HttpFoundation/DbalSessionHandler.php

@@ -12,6 +12,8 @@
 namespace Symfony\Bridge\Doctrine\HttpFoundation;
 
 use Doctrine\DBAL\Connection;
+use Doctrine\DBAL\Driver\DriverException;
+use Doctrine\DBAL\Platforms\SQLServer2008Platform;
 
 /**
  * DBAL based session storage.
@@ -146,13 +148,10 @@ class DbalSessionHandler implements \SessionHandlerInterface
      */
     public function write($sessionId, $data)
     {
-        // Session data can contain non binary safe characters so we need to encode it.
         $encoded = base64_encode($data);
 
-        // We use a MERGE SQL query when supported by the database.
-        // Otherwise we have to use a transactional DELETE followed by INSERT to prevent duplicate entries under high concurrency.
-
         try {
+            // We use a single MERGE SQL query when supported by the database.
             $mergeSql = $this->getMergeSql();
 
             if (null !== $mergeSql) {
@@ -165,28 +164,41 @@ class DbalSessionHandler implements \SessionHandlerInterface
                 return true;
             }
 
-            $this->con->beginTransaction();
+            $updateStmt = $this->con->prepare(
+                "UPDATE $this->table SET $this->dataCol = :data, $this->timeCol = :time WHERE $this->idCol = :id"
+            );
+            $updateStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR);
+            $updateStmt->bindParam(':data', $encoded, \PDO::PARAM_STR);
+            $updateStmt->bindValue(':time', time(), \PDO::PARAM_INT);
+            $updateStmt->execute();
 
-            try {
+            // When MERGE is not supported, like in Postgres, we have to use this approach that can result in
-                $deleteStmt = $this->con->prepare(
+            // duplicate key errors when the same session is written simultaneously. We can just catch such an
-                    "DELETE FROM $this->table WHERE $this->idCol = :id"
+            // error and re-execute the update. This is similar to a serializable transaction with retry logic
-                );
+            // on serialization failures but without the overhead and without possible false positives due to
-                $deleteStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR);
+            // longer gap locking.
-                $deleteStmt->execute();
+            if (!$updateStmt->rowCount()) {
-
+                try {
-                $insertStmt = $this->con->prepare(
+                    $insertStmt = $this->con->prepare(
-                    "INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time)"
+                        "INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time)"
-                );
+                    );
-                $insertStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR);
+                    $insertStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR);
-                $insertStmt->bindParam(':data', $encoded, \PDO::PARAM_STR);
+                    $insertStmt->bindParam(':data', $encoded, \PDO::PARAM_STR);
-                $insertStmt->bindValue(':time', time(), \PDO::PARAM_INT);
+                    $insertStmt->bindValue(':time', time(), \PDO::PARAM_INT);
-                $insertStmt->execute();
+                    $insertStmt->execute();
-
+                } catch (\Exception $e) {
-                $this->con->commit();
+                    $driverException = $e->getPrevious();
-            } catch (\Exception $e) {
+                    // Handle integrity violation SQLSTATE 23000 (or a subclass like 23505 in Postgres) for duplicate keys
-                $this->con->rollback();
+                    // DriverException only available since DBAL 2.5
-
+                    if (
-                throw $e;
+                        ($driverException instanceof DriverException && 0 === strpos($driverException->getSQLState(), '23')) ||
+                        ($driverException instanceof \PDOException && 0 === strpos($driverException->getCode(), '23'))
+                    ) {
+                        $updateStmt->execute();
+                    } else {
+                        throw $e;
+                    }
+                }
             }
         } catch (\Exception $e) {
             throw new \RuntimeException(sprintf('Exception was thrown when trying to write the session data: %s', $e->getMessage()), 0, $e);
@@ -212,12 +224,13 @@ class DbalSessionHandler implements \SessionHandlerInterface
                 // DUAL is Oracle specific dummy table
                 return "MERGE INTO $this->table USING DUAL ON ($this->idCol = :id) " .
                     "WHEN NOT MATCHED THEN INSERT ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time) " .
-                    "WHEN MATCHED THEN UPDATE SET $this->dataCol = :data";
+                    "WHEN MATCHED THEN UPDATE SET $this->dataCol = :data, $this->timeCol = :time";
-            case 'mssql':
+            case $this->con->getDatabasePlatform() instanceof SQLServer2008Platform:
-                // MS SQL Server requires MERGE be terminated by semicolon
+                // MERGE is only available since SQL Server 2008 and must be terminated by semicolon
-                return "MERGE INTO $this->table USING (SELECT 'x' AS dummy) AS src ON ($this->idCol = :id) " .
+                // It also requires HOLDLOCK according to http://weblogs.sqlteam.com/dang/archive/2009/01/31/UPSERT-Race-Condition-With-MERGE.aspx
+                return "MERGE INTO $this->table WITH (HOLDLOCK) USING (SELECT 1 AS dummy) AS src ON ($this->idCol = :id) " .
                     "WHEN NOT MATCHED THEN INSERT ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time) " .
-                    "WHEN MATCHED THEN UPDATE SET $this->dataCol = :data;";
+                    "WHEN MATCHED THEN UPDATE SET $this->dataCol = :data, $this->timeCol = :time;";
             case 'sqlite':
                 return "INSERT OR REPLACE INTO $this->table ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time)";
         }

src/Symfony/Bridge/Doctrine/HttpFoundation/DbalSessionHandlerSchema.php

@@ -20,14 +20,11 @@ use Doctrine\DBAL\Schema\Schema;
  */
 final class DbalSessionHandlerSchema extends Schema
 {
-    private $tableName;
-
     public function __construct($tableName = 'sessions')
     {
         parent::__construct();
 
-        $this->tableName = $tableName;
+        $this->addSessionTable($tableName);
-        $this->addSessionTable();
     }
 
     public function addToSchema(Schema $schema)
@@ -37,9 +34,9 @@ final class DbalSessionHandlerSchema extends Schema
         }
     }
 
-    private function addSessionTable()
+    private function addSessionTable($tableName)
     {
-        $table = $this->createTable($this->tableName);
+        $table = $this->createTable($tableName);
         $table->addColumn('sess_id', 'string');
         $table->addColumn('sess_data', 'text')->setNotNull(true);
         $table->addColumn('sess_time', 'integer')->setNotNull(true)->setUnsigned(true);

src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php

@@ -173,7 +173,7 @@ class PdoSessionHandler implements \SessionHandlerInterface
         $encoded = base64_encode($data);
 
         try {
-            // We use a MERGE SQL query when supported by the database.
+            // We use a single MERGE SQL query when supported by the database.
             $mergeSql = $this->getMergeSql();
 
             if (null !== $mergeSql) {
@@ -194,10 +194,11 @@ class PdoSessionHandler implements \SessionHandlerInterface
             $updateStmt->bindValue(':time', time(), \PDO::PARAM_INT);
             $updateStmt->execute();
 
-            // Since Postgres does not support MERGE (without custom stored procedure), we have to use this approach
+            // When MERGE is not supported, like in Postgres, we have to use this approach that can result in
-            // that can result in duplicate key errors when the same session is written simultaneously. We can just
+            // duplicate key errors when the same session is written simultaneously. We can just catch such an
-            // ignore such an error because either the data did not change anyway or which data is written does not
+            // error and re-execute the update. This is similar to a serializable transaction with retry logic
-            // matter as proper locking to serialize access to a session is not implemented.
+            // on serialization failures but without the overhead and without possible false positives due to
+            // longer gap locking.
             if (!$updateStmt->rowCount()) {
                 try {
                     $insertStmt = $this->pdo->prepare(
@@ -208,8 +209,10 @@ class PdoSessionHandler implements \SessionHandlerInterface
                     $insertStmt->bindValue(':time', time(), \PDO::PARAM_INT);
                     $insertStmt->execute();
                 } catch (\PDOException $e) {
-                    // ignore unique violation SQLSTATE
+                    // Handle integrity violation SQLSTATE 23000 (or a subclass like 23505 in Postgres) for duplicate keys
-                    if ('23505' !== $e->getCode()) {
+                    if (0 === strpos($e->getCode(), '23')) {
+                        $updateStmt->execute();
+                    } else {
                         throw $e;
                     }
                 }
@@ -241,7 +244,8 @@ class PdoSessionHandler implements \SessionHandlerInterface
                     "WHEN MATCHED THEN UPDATE SET $this->dataCol = :data, $this->timeCol = :time";
             case 'sqlsrv' && version_compare($this->pdo->getAttribute(\PDO::ATTR_SERVER_VERSION), '10', '>='):
                 // MERGE is only available since SQL Server 2008 and must be terminated by semicolon
-                return "MERGE INTO $this->table USING (SELECT 'x' AS dummy) AS src ON ($this->idCol = :id) " .
+                // It also requires HOLDLOCK according to http://weblogs.sqlteam.com/dang/archive/2009/01/31/UPSERT-Race-Condition-With-MERGE.aspx
+                return "MERGE INTO $this->table WITH (HOLDLOCK) USING (SELECT 1 AS dummy) AS src ON ($this->idCol = :id) " .
                     "WHEN NOT MATCHED THEN INSERT ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time) " .
                     "WHEN MATCHED THEN UPDATE SET $this->dataCol = :data, $this->timeCol = :time;";
             case 'sqlite':