Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions system/Database/BaseConnection.php
Original file line number Diff line number Diff line change
Expand Up @@ -625,16 +625,44 @@ public function close()
* Keep or establish the connection if no queries have been sent for
* a length of time exceeding the server's idle timeout.
*
* A replaced socket is not in the transaction that was open on the old
* one. The server has already rolled that work back. Clear the depth and
* mark the transaction failed before opening the new connection, so a
* failed reconnect cannot still report success. OCI8 also returns its
* commit mode to autocommit.
*
* @return void
*/
public function reconnect()
{
if ($this->ping() === false) {
$lostTransaction = $this->transDepth > 0;

$this->close();

// initialize() can throw. Clear the lost transaction first so a
// failed reconnect cannot still report the old transaction.
if ($lostTransaction) {
$this->abandonLostTransaction();
}

$this->initialize();
}
}

/**
* The socket that owned the transaction is already closed.
*
* Drivers that keep transaction state in PHP, rather than only on the
* server, must override this. OCI8 commit mode is one case: leaving it
* at OCI_NO_AUTO_COMMIT makes later writes uncommitted.
*/
protected function abandonLostTransaction(): void
{
$this->transDepth = 0;
$this->transStatus = false;
}

/**
* Platform dependent way method for closing the connection.
*
Expand Down
7 changes: 7 additions & 0 deletions system/Database/OCI8/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -722,6 +722,13 @@ protected function _transRollback(): bool
return oci_rollback($this->connID);
}

protected function abandonLostTransaction(): void
{
$this->commitMode = OCI_COMMIT_ON_SUCCESS;

parent::abandonLostTransaction();
}

/**
* Returns the name of the current database being used.
*/
Expand Down
132 changes: 132 additions & 0 deletions tests/system/Database/Live/ReconnectDuringTransactionTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
<?php

declare(strict_types=1);

/**
* This file is part of CodeIgniter 4 framework.
*
* (c) CodeIgniter Foundation <admin@codeigniter.com>
*
* For the full copyright and license information, please view
* the LICENSE file that was distributed with this source code.
*/

namespace CodeIgniter\Database\Live;

use CodeIgniter\Test\CIUnitTestCase;
use CodeIgniter\Test\DatabaseTestTrait;
use Config\Database;
use PHPUnit\Framework\Attributes\Group;

/**
* @internal
*/
#[Group('DatabaseLive')]
final class ReconnectDuringTransactionTest extends CIUnitTestCase
{
use DatabaseTestTrait;

protected $migrate = false;

protected function setUp(): void
{
// Dedicated connection so a forced wait_timeout cannot drop the shared one.
$this->db = Database::connect($this->DBGroup, false);

parent::setUp();
}

public function testReconnectInsideTransactionDoesNotReportSuccess(): void
{
if ($this->db->DBDriver !== 'MySQLi') {
$this->markTestSkipped('Requires the MySQLi driver.');
}

$table = $this->db->escapeIdentifier($this->db->prefixTable('reconnect_probe'));
$this->db->query('DROP TABLE IF EXISTS ' . $table);
$this->db->query(
'CREATE TABLE ' . $table . ' ('
. 'id INT NOT NULL AUTO_INCREMENT, '
. 'note VARCHAR(32) NOT NULL, '
. 'PRIMARY KEY (id)'
. ') ENGINE=InnoDB',
);

$observer = Database::connect($this->DBGroup, false);
$observer->initialize();

try {
$this->db->transStart();
$this->db->table('reconnect_probe')->insert(['note' => 'before-timeout']);
$this->assertSame(1, $this->db->transDepth);
$this->assertTrue($this->db->transStatus());

$this->db->query('SET SESSION wait_timeout = 1');
sleep(3);

$this->assertFalse($this->db->ping(), 'Precondition: the server should have closed the idle socket.');

$this->db->reconnect();

$depthAfterReconnect = $this->db->transDepth;
$statusAfterReconnect = $this->db->transStatus();
$completed = $this->db->transComplete();
$rows = (int) $observer->table('reconnect_probe')->countAllResults();

$this->assertSame(
['depth' => 0, 'status' => false, 'completed' => false, 'rows' => 0],
[
'depth' => $depthAfterReconnect,
'status' => $statusAfterReconnect,
'completed' => $completed,
'rows' => $rows,
],
);
} finally {
if ($this->db->transDepth > 0) {
$this->db->transRollback();
}

$this->db->query('DROP TABLE IF EXISTS ' . $table);
$observer->close();
$this->db->close();
}
}

public function testReconnectKeepsAnOpenTransactionWhenTheSocketIsAlive(): void
{
if ($this->db->DBDriver !== 'MySQLi') {
$this->markTestSkipped('Requires the MySQLi driver.');
}

$table = $this->db->escapeIdentifier($this->db->prefixTable('reconnect_probe'));
$this->db->query('DROP TABLE IF EXISTS ' . $table);
$this->db->query(
'CREATE TABLE ' . $table . ' ('
. 'id INT NOT NULL AUTO_INCREMENT, '
. 'note VARCHAR(32) NOT NULL, '
. 'PRIMARY KEY (id)'
. ') ENGINE=InnoDB',
);

try {
$this->db->transStart();
$this->db->table('reconnect_probe')->insert(['note' => 'still-open']);
$this->assertTrue($this->db->ping());

$this->db->reconnect();

$this->assertSame(1, $this->db->transDepth);
$this->assertTrue($this->db->transStatus());
$this->assertTrue($this->db->transComplete());
$this->assertSame(1, $this->db->table('reconnect_probe')->countAllResults());
} finally {
if ($this->db->transDepth > 0) {
$this->db->transRollback();
}

$this->db->query('DROP TABLE IF EXISTS ' . $table);
$this->db->close();
}
}
}
75 changes: 75 additions & 0 deletions tests/system/Database/ReconnectLostTransactionTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
<?php

declare(strict_types=1);

/**
* This file is part of CodeIgniter 4 framework.
*
* (c) CodeIgniter Foundation <admin@codeigniter.com>
*
* For the full copyright and license information, please view
* the LICENSE file that was distributed with this source code.
*/

namespace CodeIgniter\Database;

use CodeIgniter\Database\Exceptions\DatabaseException;
use CodeIgniter\Database\MySQLi\Connection as MySQLiConnection;
use CodeIgniter\Database\OCI8\Connection as OCI8Connection;
use CodeIgniter\Test\CIUnitTestCase;
use PHPUnit\Framework\Attributes\Group;

/**
* @internal
*/
#[Group('Others')]
final class ReconnectLostTransactionTest extends CIUnitTestCase
{
public function testReconnectClearsTransactionStateWhenInitializeFails(): void
{
$db = new class (['DBDriver' => 'MySQLi']) extends MySQLiConnection {
public function initialize(): never
{
throw new DatabaseException('database unavailable');
}
};

$this->setPrivateProperty($db, 'transDepth', 1);

try {
$db->reconnect();
$this->fail('initialize() should have thrown.');
} catch (DatabaseException $exception) {
$this->assertSame('database unavailable', $exception->getMessage());
}

$this->assertSame(0, $this->getPrivateProperty($db, 'transDepth'));
$this->assertFalse($db->transStatus());
}

public function testReconnectRestoresOci8CommitModeWhenTransactionIsLost(): void
{
if (! defined('OCI_COMMIT_ON_SUCCESS')) {
define('OCI_COMMIT_ON_SUCCESS', 32);
}

if (! defined('OCI_NO_AUTO_COMMIT')) {
define('OCI_NO_AUTO_COMMIT', 0);
}

$db = new class (['DBDriver' => 'OCI8']) extends OCI8Connection {
public function initialize(): void
{
}
};

$db->commitMode = OCI_NO_AUTO_COMMIT;
$this->setPrivateProperty($db, 'transDepth', 1);

$db->reconnect();

$this->assertSame(OCI_COMMIT_ON_SUCCESS, $db->commitMode);
$this->assertSame(0, $this->getPrivateProperty($db, 'transDepth'));
$this->assertFalse($db->transStatus());
}
}
8 changes: 8 additions & 0 deletions user_guide_src/source/changelogs/v4.7.6.rst
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@ Deprecations
Bugs Fixed
**********

- **Database:** Fixed ``reconnect()`` so a replaced socket does not keep the
transaction that died with the old connection. ``transDepth`` is cleared and
the transaction status is failed before the new connection is opened, so a
failed reconnect cannot still report success. OCI8 commit mode returns to
autocommit. ``transComplete()`` does not report success. This applies when
``ping()`` reports the connection lost; PostgreSQL's ``pg_ping()`` reconnects
a broken connection itself, so it is not covered.
See `#10648 <https://github.com/codeigniter4/CodeIgniter4/issues/10648>`_.
- **Commands:** Fixed a bug where generator commands did not sort the imports of a template with CRLF line endings.
- **Format:** Fixed ``XMLFormatter`` so that an array key that becomes empty after
sanitization no longer triggers a ``ValueError``; the fallback tag name ``item``
Expand Down
9 changes: 8 additions & 1 deletion user_guide_src/source/database/connecting.rst
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,14 @@ which can gracefully keep the connection alive or re-establish it.

The ``reconnect()`` method pings the server to check if the connection is still
alive. If the connection has been lost, it will close and re-establish the
connection.
connection. An open transaction ends with the old connection. ``reconnect()``
clears the transaction depth and marks the transaction failed before it
opens the new connection, so ``transComplete()`` does not report success.
On OCI8, commit mode returns to autocommit.

This only applies when ``ping()`` reports the connection lost. On PostgreSQL,
``ping()`` uses ``pg_ping()``, which re-establishes a broken connection itself
and returns true, so ``reconnect()`` does not clear the transaction state.

.. literalinclude:: connecting/007.php
:lines: 2-
Expand Down
Loading