Skip to content

Commit 84385a2

Browse files
fix: do not run the statement when the restart BEGIN fails
With mysqli exception reporting off, begin_transaction() returns false instead of throwing. The restart flag was then cleared and the statement ran without a transaction. execute() now returns false and keeps the restart pending. The guide now says the rollback happens when the outermost transaction finishes, and that a nested transCommit() still returns true. Drop the issue link from the changelog entry to match the changelog style. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent d64b06a commit 84385a2

4 files changed

Lines changed: 40 additions & 5 deletions

File tree

‎system/Database/MySQLi/Connection.php‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,13 @@ protected function execute(string $sql)
297297
// Errno 1205 is not handled: innodb_rollback_on_timeout decides
298298
// whether InnoDB rolled that transaction back.
299299
if ($this->connID instanceof mysqli && $this->serverTransactionRestartPending()) {
300-
$this->connID->begin_transaction();
300+
// With mysqli exception reporting off, a failed BEGIN returns
301+
// false instead of throwing. Do not run the statement without
302+
// the transaction.
303+
if (! $this->connID->begin_transaction()) {
304+
return false;
305+
}
306+
301307
$this->serverTransactionRestarted();
302308
}
303309

‎tests/system/Database/Live/MySQLi/DeadlockTransactionTest.php‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use CodeIgniter\Test\DatabaseTestTrait;
1919
use Config\Database;
2020
use mysqli;
21+
use mysqli_driver;
2122
use PHPUnit\Framework\Attributes\Group;
2223
use Throwable;
2324

@@ -115,6 +116,32 @@ public function testLostConnectionAfterDeadlockDoesNotEscapeAsDriverException():
115116
);
116117
}
117118

119+
public function testRestartThatCannotBeginDoesNotRunTheStatement(): void
120+
{
121+
$table = $this->db->escapeIdentifier($this->db->prefixTable('deadlock_probe'));
122+
$this->probeTable = $table;
123+
$this->forceDeadlock($table);
124+
125+
// An active XA transaction makes MySQL refuse START TRANSACTION on a live
126+
// connection. With mysqli exception reporting off, begin_transaction()
127+
// then returns false instead of throwing.
128+
$mysqli = $this->db->getConnection();
129+
$previous = (new mysqli_driver())->report_mode;
130+
$mysqli->query("XA START 'deadlock_probe'");
131+
mysqli_report(MYSQLI_REPORT_OFF);
132+
133+
try {
134+
$insert = $this->db->query('INSERT INTO ' . $table . " (id, note) VALUES (102, 'after')");
135+
$inserted = (int) $mysqli->query('SELECT COUNT(*) FROM ' . $table . ' WHERE id = 102')->fetch_row()[0];
136+
} finally {
137+
$mysqli->query("XA END 'deadlock_probe'");
138+
$mysqli->query("XA ROLLBACK 'deadlock_probe'");
139+
mysqli_report($previous);
140+
}
141+
142+
$this->assertSame(['insert' => false, 'inserted' => 0], ['insert' => $insert, 'inserted' => $inserted]);
143+
}
144+
118145
private function killConnection(BaseConnection $db): void
119146
{
120147
$threadId = $db->getConnection()->thread_id;

‎user_guide_src/source/changelogs/v4.7.6.rst‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ Bugs Fixed
3939
- **CLI:** Fixed a bug where a prompt asked again forever when its input was piped or redirected and ran out before a valid answer: ``CLI::prompt()`` and ``CLI::promptByKey()`` whose default fails the validation rules, and ``CLI::promptByMultipleKeys()`` whose options have string keys. They now throw a ``CLIException``. The new ``InputOutput::isAtEnd()`` tells that end of the input from an empty line. Pressing Ctrl+D at a terminal still asks again.
4040
- **Commands:** Fixed a bug where generator commands did not sort the imports of a template with CRLF line endings.
4141
- **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.
42-
- **Database:** Fixed MySQLi errno 1213 (``ER_LOCK_DEADLOCK``) so a following write through ``query()``, ``simpleQuery()`` or the Query Builder on the victim connection does not autocommit, and ``transCommit()`` rolls back and returns false after InnoDB has rolled the victim transaction back. Prepared queries and errno 1205 are unchanged. See `#10649 <https://github.com/codeigniter4/CodeIgniter4/issues/10649>`_.
42+
- **Database:** Fixed MySQLi errno 1213 (``ER_LOCK_DEADLOCK``) so a following write through ``query()``, ``simpleQuery()`` or the Query Builder on the victim connection does not autocommit, and ``transCommit()`` rolls back and returns false after InnoDB has rolled the victim transaction back. Prepared queries and errno 1205 are unchanged.
4343
- **Format:** Fixed ``XMLFormatter`` so that an array key that becomes empty after sanitization no longer triggers a ``ValueError``; the fallback tag name ``item`` is used instead. Values that cannot be converted to a string, such as objects not implementing ``Stringable``, now throw a ``FormatException`` instead of a fatal ``Error``.
4444
- **Helpers:** Fixed a regression in v4.7.5 where ``set_value()`` threw a ``TypeError`` when the default value was not a string or an array, such as an integer, a float, ``null``, or a ``Stringable`` object. Such values are now converted to a string.
4545
- **HTTP:** Fixed a bug in ``ContentSecurityPolicy`` where a numeric source such as ``'0'`` threw a ``TypeError``, and a space-separated directive string in ``Config\\ContentSecurityPolicy`` was emitted as one unquoted source.

‎user_guide_src/source/database/transactions.rst‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,9 +151,11 @@ MySQL Deadlocks
151151
InnoDB error 1213 rolls the victim transaction back. CodeIgniter keeps that
152152
transaction open. The next statement run through ``query()``, ``simpleQuery()``
153153
or the Query Builder on the same connection starts a new server transaction,
154-
so the write does not autocommit. ``transCommit()`` rolls that server
155-
transaction back and returns false, so the transaction has ended when it
156-
returns. ``transComplete()`` also rolls it back.
154+
so the write does not autocommit. The rollback happens when the outermost
155+
transaction finishes: ``transCommit()`` rolls that server transaction back and
156+
returns false, so the transaction has ended when it returns, and
157+
``transComplete()`` also rolls it back. At a nested depth, ``transCommit()``
158+
still returns true and only decrements the depth.
157159

158160
Prepared queries are not covered. A deadlock raised by a prepared query is not
159161
detected, and a prepared query run after a deadlock does not start a new server

0 commit comments

Comments
 (0)