fix: keep writes transactional after MySQL deadlock 1213 by KevinBlakeLopez · Pull Request #10651 · codeigniter4/CodeIgniter4

KevinBlakeLopez · 2026-10-22T15:05:50Z

Description

InnoDB errno 1213 rolls the victim transaction back. CodeIgniter left transDepth at 1, so the next write on that connection autocommitted. transComplete() returned false and did not remove that write. transCommit() called mysqli::commit(), which returns true, and reported success while the victim row was already gone.

This is only errno 1213. Errno 1205 is not handled here, because innodb_rollback_on_timeout decides whether InnoDB rolled that transaction back.

After 1213, the next statement on that connection starts a new server transaction, so it does not autocommit. transCommit() returns false without calling mysqli::commit(). transComplete() still sees the failed status and rolls the new server transaction back.

On develop (df8b96bee) the new live tests failed before this change. transCommit():

-    'visible_before' => [],
-    'finished' => false,
-    'visible_after' => [],
+    'visible_before' => [102],
+    'finished' => true,
+    'visible_after' => [102],

transComplete() already returned false, and the following row stayed visible:

-    'visible_before' => [],
+    'visible_before' => [102],
     'finished' => false,
-    'visible_after' => [],
+    'visible_after' => [102],

The victim id 101 was absent in both runs. After the change, DeadlockTransactionTest passes, and the existing TransactionTest, TransactionDBDebugTrueTest, and TransactionDBDebugFalseTest still pass (local MySQL 8.0.45, PHP 8.5.12, MySQLi).

Checklist:

Securely signed commits
Component(s) with PHPDoc blocks, only if necessary or adds value (without duplication)
Unit testing, with >80% coverage
User guide updated
Conforms to style guide

Made with Cursor

carson-codeigniter4 · 2026-10-22T15:06:07Z

Hi there, @KevinBlakeLopez! 👋

Thank you for sending this PR!

We expect the following in all Pull Requests (PRs).

PRs must be sent to the appropriate branch
All git commits must be GPG-signed
Must follow our style guide
Be commented in the PHP source file
Be documented in the user guide
Be unit tested
Pass all checks in GitHub Actions

Important

We expect all code changes or bug-fixes to be accompanied by one or more tests added to our test suite to prove the code works.

If pull requests do not comply with the above, they will likely be closed. Since we are a team of volunteers, we don't have any more time to work
on the framework than you do. Please make it as painless for your contributions to be included as possible.

See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md

Copilot

🟡 Changes recommended

Prepared statements and failed transaction restarts can still allow subsequent writes to autocommit.

3 open issues

What changed in this PR

Addresses #10649 by tracking MySQLi transactions rolled back by deadlock error 1213, preventing later writes from autocommitting and avoiding false commit success. Reviewed under the assumed develop patch-release policy.

Changes:

Adds rollback tracking, deferred transaction restart, and commit protection.
Adds live MySQLi deadlock regression tests.
Documents recovery behavior while leaving error 1205 unchanged.

Validation used targeted source inspection; live database tests and full validation remain with CI.

File	Description
user_guide_src/source/database/transactions.rst	Documents MySQL deadlock recovery behavior.
user_guide_src/source/changelogs/v4.7.6.rst	Records the errno 1213 fix.
tests/system/Database/Live/MySQLi/DeadlockTransactionTest.php	Adds deadlock and subsequent-write regression tests.
system/Database/MySQLi/Connection.php	Detects deadlocks and restarts server transactions.
system/Database/BaseConnection.php	Tracks server rollback state and prevents false commit success.

_{Review effort: Balanced}

InnoDB rolls the victim transaction back and mysqli::commit() still returns true. The next statement now starts a new server transaction, and transCommit() does not report success. Fixes codeigniter4#10649 Co-authored-by: Cursor <cursoragent@cursor.com>

mysqli_sql_exception::getCode() is already an int. Rector rejects the redundant cast. Co-authored-by: Cursor <cursoragent@cursor.com>

The restart BEGIN ran outside the query try block, after the restart flag was cleared. If the connection dropped after errno 1213, query(), ping() and reconnect() threw a raw mysqli_sql_exception, even with DBDebug false, and the next write skipped BEGIN. BEGIN now runs inside the try, and the restart stays pending until it succeeds. transCommit() returned false and left transDepth at 1 with the restarted server transaction open. A caller that retried without transRollback() nested inside it, got true from transCommit(), and nothing was committed. transCommit() now rolls that transaction back before returning false. The guide and changelog now limit the fix to query(), simpleQuery() and the Query Builder. Prepared queries are not covered. The deadlock test records the probe table before recreating it, so teardown drops it. Refs codeigniter4#10649 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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>

michalsn

Thank you!

paulbalandan · 2026-10-27T06:14:25Z

+            // Errno 1213 ends the server transaction. Begin again before this
+            // statement so it does not autocommit. A failed BEGIN goes through
+            // the normal error path below, and the restart stays pending.
+            // Errno 1205 is not handled: innodb_rollback_on_timeout decides
+            // whether InnoDB rolled that transaction back.
+            if ($this->connID instanceof mysqli && $this->serverTransactionRestartPending()) {
+                // With mysqli exception reporting off, a failed BEGIN returns
+                // false instead of throwing. Do not run the statement without
+                // the transaction.


Can you make the comments here a bit more concise?

paulbalandan · 2026-10-27T06:14:43Z

+        // InnoDB already removed the victim transaction. mysqli::commit()
+        // returns true anyway, which would publish later writes and report success.
+        // Roll back the restarted server transaction so a false return ends the
+        // transaction, as transComplete() does. Otherwise a caller that retries
+        // without transRollback() nests inside it and nothing commits.


and here, too

paulbalandan · 2026-10-27T06:58:02Z

+    /**
+     * The server has already rolled this transaction back (MySQLi errno 1213).
+     * mysqli::commit() still returns true, so transCommit() must not report success.
+     */
+    protected bool $transServerRolledBack = false;
+
+    /**
+     * The next statement must open a server transaction before it runs.
+     * InnoDB ended the previous one and the connection is back in autocommit.
+     * Errno 1205 does not set this: innodb_rollback_on_timeout decides whether
+     * that error rolled the transaction back.
+     */
+    protected bool $transNeedsServerRestart = false;
+


Shared properties (and the new shared methods) on the parent connection class that will only be used by the MySQL driver is not the flow I'm expecting (unless other drivers will eventually use them).

Would you rather consider moving these entirely inside MySQLi\Connection. BaseConnection only calls _transBegin(), _transCommit() and _transRollback() at the outermost depth, so the flags can be private there:

_transBegin() and _transRollback() clear them after success

_transCommit() rolls back, decrements transDepth and returns false when InnoDB has already rolled back

BaseConnection then stays untouched, and no protected API is added for every driver.

Done in 72aab61. The errno 1213 flags now live on MySQLi\Connection, and BaseConnection is unchanged. _transBegin() and _transRollback() clear them after success. When InnoDB has already rolled back, _transCommit() calls transRollback() and returns false, so the depth decrement stays in transRollback(). execute() still starts the next server transaction before the statement. Those comments are shorter. Errno 1205 stays out of scope.

The flags are MySQLi-only, so BaseConnection stays unchanged. _transBegin() and _transRollback() clear them after success. When InnoDB has already rolled back, _transCommit() calls transRollback() and returns false. Co-authored-by: Cursor <cursoragent@cursor.com>

carson-codeigniter4 Bot added the bug Verified issues on the current code behavior or pull requests that will fix them label Oct 22, 2026

KevinBlakeLopez force-pushed the fix/10649-innodb-deadlock-1213 branch from 4976289 to 7750791 Compare October 22, 2026 23:37

paulbalandan requested a balanced review from Copilot October 23, 2026 05:41

Copilot started reviewing on behalf of paulbalandan October 23, 2026 05:42

View session

Copilot AI reviewed Oct 23, 2026

View reviewed changes

Comment thread system/Database/MySQLi/Connection.php Outdated

Comment thread system/Database/MySQLi/Connection.php Outdated

Comment thread tests/system/Database/Live/MySQLi/DeadlockTransactionTest.php

KevinBlakeLopez and others added 3 commits October 25, 2026 15:10

fix: compare the MySQL deadlock code without an int cast

17395ee

mysqli_sql_exception::getCode() is already an int. Rector rejects the redundant cast. Co-authored-by: Cursor <cursoragent@cursor.com>

KevinBlakeLopez force-pushed the fix/10649-innodb-deadlock-1213 branch from 8089966 to 5cd609b Compare October 25, 2026 22:32

michalsn reviewed Oct 26, 2026

View reviewed changes

Comment thread system/Database/MySQLi/Connection.php

Comment thread user_guide_src/source/database/transactions.rst Outdated

Comment thread user_guide_src/source/changelogs/v4.7.6.rst Outdated

michalsn approved these changes Oct 26, 2026

View reviewed changes

paulbalandan reviewed Oct 27, 2026

View reviewed changes

Navigation Menu

Repository navigation

fix: keep writes transactional after MySQL deadlock 1213 - #10651

fix: keep writes transactional after MySQL deadlock 1213#10651
KevinBlakeLopez wants to merge 5 commits into
codeigniter4:developfrom
KevinBlakeLopez:fix/10649-innodb-deadlock-1213

KevinBlakeLopez commented Oct 22, 2026 •

edited

Loading

Uh oh!

carson-codeigniter4 Bot commented Oct 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

michalsn left a comment

Uh oh!

paulbalandan Oct 27, 2026

Uh oh!

paulbalandan Oct 27, 2026

Uh oh!

paulbalandan Oct 27, 2026

Uh oh!

KevinBlakeLopez Oct 27, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

KevinBlakeLopez commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

carson-codeigniter4 Bot commented Oct 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

michalsn left a comment

Choose a reason for hiding this comment

Uh oh!

paulbalandan Oct 27, 2026

Choose a reason for hiding this comment

Uh oh!

paulbalandan Oct 27, 2026

Choose a reason for hiding this comment

Uh oh!

paulbalandan Oct 27, 2026

Choose a reason for hiding this comment

Uh oh!

KevinBlakeLopez Oct 27, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

KevinBlakeLopez commented Oct 22, 2026 •

edited

Loading