diff --git a/src/Symfony/Bridge/Doctrine/HttpFoundation/DbalSessionHandler.php b/src/Symfony/Bridge/Doctrine/HttpFoundation/DbalSessionHandler.php index 3248313717..d0c0577bec 100644 --- a/src/Symfony/Bridge/Doctrine/HttpFoundation/DbalSessionHandler.php +++ b/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 { - $deleteStmt = $this->con->prepare( - "DELETE FROM $this->table WHERE $this->idCol = :id" - ); - $deleteStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR); - $deleteStmt->execute(); - - $insertStmt = $this->con->prepare( - "INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time)" - ); - $insertStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR); - $insertStmt->bindParam(':data', $encoded, \PDO::PARAM_STR); - $insertStmt->bindValue(':time', time(), \PDO::PARAM_INT); - $insertStmt->execute(); - - $this->con->commit(); - } catch (\Exception $e) { - $this->con->rollback(); - - throw $e; + // When MERGE is not supported, like in Postgres, we have to use this approach that can result in + // duplicate key errors when the same session is written simultaneously. We can just catch such an + // 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 + // longer gap locking. + if (!$updateStmt->rowCount()) { + try { + $insertStmt = $this->con->prepare( + "INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time)" + ); + $insertStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR); + $insertStmt->bindParam(':data', $encoded, \PDO::PARAM_STR); + $insertStmt->bindValue(':time', time(), \PDO::PARAM_INT); + $insertStmt->execute(); + } catch (\Exception $e) { + $driverException = $e->getPrevious(); + // Handle integrity violation SQLSTATE 23000 (or a subclass like 23505 in Postgres) for duplicate keys + // DriverException only available since DBAL 2.5 + if ( + ($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"; - case 'mssql': - // MS SQL Server requires MERGE be terminated by semicolon - return "MERGE INTO $this->table USING (SELECT 'x' AS dummy) AS src ON ($this->idCol = :id) " . + "WHEN MATCHED THEN UPDATE SET $this->dataCol = :data, $this->timeCol = :time"; + case $this->con->getDatabasePlatform() instanceof SQLServer2008Platform: + // MERGE is only available since SQL Server 2008 and must be terminated by semicolon + // 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)"; } diff --git a/src/Symfony/Bridge/Doctrine/HttpFoundation/DbalSessionHandlerSchema.php b/src/Symfony/Bridge/Doctrine/HttpFoundation/DbalSessionHandlerSchema.php index b7b4b91a7a..978373da0c 100644 --- a/src/Symfony/Bridge/Doctrine/HttpFoundation/DbalSessionHandlerSchema.php +++ b/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(); + $this->addSessionTable($tableName); } 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); diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php b/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php index dfe6258e0c..cc43479072 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php +++ b/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 - // that can result in duplicate key errors when the same session is written simultaneously. We can just - // ignore such an error because either the data did not change anyway or which data is written does not - // matter as proper locking to serialize access to a session is not implemented. + // When MERGE is not supported, like in Postgres, we have to use this approach that can result in + // duplicate key errors when the same session is written simultaneously. We can just catch such an + // 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 + // 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 - if ('23505' !== $e->getCode()) { + // Handle integrity violation SQLSTATE 23000 (or a subclass like 23505 in Postgres) for duplicate keys + 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':