fix: end the transaction when transCommit() returns false after 1213

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 #10649

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Author: KevinBlakeLopez

system/Database/BaseConnection.php

@@ -1086,7 +1086,12 @@ public function transCommit(): bool
 
         // 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.
         if ($this->transServerRolledBack) {
+            $this->transRollback();
+
             return false;
         }
 
@@ -1154,17 +1159,19 @@ protected function markServerRolledBackTransaction(): void
     }
 
     /**
-     * True once, when the next statement must BEGIN before it runs.
+     * True while the next statement must BEGIN before it runs.
      */
-    protected function consumeServerTransactionRestart(): bool
+    protected function serverTransactionRestartPending(): bool
     {
-        if (! $this->transNeedsServerRestart || $this->transDepth < 1) {
-            return false;
-        }
+        return $this->transNeedsServerRestart && $this->transDepth > 0;
+    }
 
+    /**
+     * BEGIN succeeded. Clear the restart only now, so a failed BEGIN keeps it pending.
+     */
+    protected function serverTransactionRestarted(): void
+    {
         $this->transNeedsServerRestart = false;
-
-        return true;
     }
 
     private function clearServerRolledBackTransaction(): void

system/Database/MySQLi/Connection.php

@@ -283,14 +283,6 @@ public function getVersion(): string
      */
     protected function execute(string $sql)
     {
-        // Errno 1213 ends the server transaction. Begin again before this
-        // statement so it does not autocommit. Errno 1205 is not handled:
-        // innodb_rollback_on_timeout decides whether InnoDB rolled that
-        // transaction back.
-        if ($this->connID instanceof mysqli && $this->consumeServerTransactionRestart()) {
-            $this->connID->begin_transaction();
-        }
-
         while ($this->connID->more_results()) {
             $this->connID->next_result();
             if ($res = $this->connID->store_result()) {
@@ -299,6 +291,16 @@ protected function execute(string $sql)
         }
 
         try {
+            // 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()) {
+                $this->connID->begin_transaction();
+                $this->serverTransactionRestarted();
+            }
+
             return $this->connID->query($this->prepQuery($sql), $this->resultMode);
         } catch (mysqli_sql_exception $e) {
             log_message('error', "{message}\nin {exFile} on line {exLine}.\n{trace}", [

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

@@ -69,6 +69,72 @@ public function testFollowingWriteDoesNotAutocommitAndTransCompleteRollsItBack()
         $this->assertDeadlockVictimDoesNotPublishTheNextWrite(true);
     }
 
+    public function testFalseTransCommitEndsTheTransactionSoARetryCommits(): void
+    {
+        $table            = $this->db->escapeIdentifier($this->db->prefixTable('deadlock_probe'));
+        $this->probeTable = $table;
+        $this->forceDeadlock($table);
+
+        $this->db->query('INSERT INTO ' . $table . " (id, note) VALUES (102, 'after')");
+
+        $firstCommit = $this->db->transCommit();
+        $depthAfter  = $this->db->transDepth;
+
+        // A caller that retries after the false commit must get a real transaction,
+        // not a nested one inside the restarted server transaction.
+        $this->db->transBegin();
+        $this->db->query('INSERT INTO ' . $table . " (id, note) VALUES (103, 'retry')");
+        $retryCommit = $this->db->transCommit();
+
+        $this->assertSame(
+            ['first_commit' => false, 'depth_after' => 0, 'retry_commit' => true, 'visible' => [103]],
+            [
+                'first_commit' => $firstCommit,
+                'depth_after'  => $depthAfter,
+                'retry_commit' => $retryCommit,
+                'visible'      => $this->committedIds($table),
+            ],
+        );
+    }
+
+    public function testLostConnectionAfterDeadlockDoesNotEscapeAsDriverException(): void
+    {
+        $table            = $this->db->escapeIdentifier($this->db->prefixTable('deadlock_probe'));
+        $this->probeTable = $table;
+        $this->forceDeadlock($table);
+        $this->killConnection($this->db);
+
+        // The restart BEGIN fails on the dead socket. That must surface through the
+        // normal query error path, not as a raw mysqli_sql_exception.
+        $insert = $this->db->query('INSERT INTO ' . $table . " (id, note) VALUES (102, 'after')");
+        $this->db->reconnect();
+
+        $this->assertSame(
+            ['insert' => false, 'complete' => false],
+            ['insert' => $insert, 'complete' => $this->db->transComplete()],
+        );
+    }
+
+    private function killConnection(BaseConnection $db): void
+    {
+        $threadId = $db->getConnection()->thread_id;
+        $observer = Database::connect($this->DBGroup, false);
+        $observer->initialize();
+        $observer->query('KILL ' . $threadId);
+
+        for ($i = 0; $i < 50; $i++) {
+            $alive = $observer->query('SELECT COUNT(*) AS n FROM information_schema.processlist WHERE id = ' . $threadId)->getRow()->n;
+
+            if ((int) $alive === 0) {
+                break;
+            }
+
+            usleep(20000);
+        }
+
+        $observer->close();
+    }
+
     private function assertDeadlockVictimDoesNotPublishTheNextWrite(bool $useComplete): void
     {
         $table            = $this->db->escapeIdentifier($this->db->prefixTable('deadlock_probe'));
@@ -140,6 +206,10 @@ private function forceDeadlock(string $table): void
 
     private function resetProbe(string $table): void
     {
+        // releaseDeadlock() clears this marker after it drops the table.
+        // Record the name again before recreating it, including when setup fails.
+        $this->probeTable = $table;
+
         if ($this->db->transDepth > 0) {
             $this->db->transRollback();
         }
@@ -162,7 +232,7 @@ private function committedIds(string $table): array
     {
         $observer = Database::connect($this->DBGroup, false);
         $observer->initialize();
-        $rows = $observer->query('SELECT id FROM ' . $table . ' WHERE id IN (101, 102) ORDER BY id')->getResultArray();
+        $rows = $observer->query('SELECT id FROM ' . $table . ' WHERE id > 100 ORDER BY id')->getResultArray();
         $observer->close();
 
         $ids = [];

user_guide_src/source/changelogs/v4.7.6.rst

@@ -39,7 +39,7 @@ Bugs Fixed
 - **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.
 - **Commands:** Fixed a bug where generator commands did not sort the imports of a template with CRLF line endings.
 - **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.
-- **Database:** Fixed MySQLi errno 1213 (``ER_LOCK_DEADLOCK``) so a following write on the victim connection does not autocommit, and ``transCommit()`` does not return true after InnoDB has rolled the victim transaction back. Errno 1205 is unchanged. See `#10649 <https://github.com/codeigniter4/CodeIgniter4/issues/10649>`_.
+- **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>`_.
 - **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``.
 - **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.
 - **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

@@ -149,9 +149,15 @@ MySQL Deadlocks
 ===============
 
 InnoDB error 1213 rolls the victim transaction back. CodeIgniter keeps that
-transaction open. The next statement on the same connection starts a new
-server transaction, so the write does not autocommit, and ``transCommit()``
-returns false. ``transComplete()`` rolls the new server transaction back.
+transaction open. The next statement run through ``query()``, ``simpleQuery()``
+or the Query Builder on the same connection starts a new server transaction,
+so the write does not autocommit. ``transCommit()`` rolls that server
+transaction back and returns false, so the transaction has ended when it
+returns. ``transComplete()`` also rolls it back.
+
+Prepared queries are not covered. A deadlock raised by a prepared query is not
+detected, and a prepared query run after a deadlock does not start a new server
+transaction.
 
 Error 1205 is not handled this way. ``innodb_rollback_on_timeout`` decides
 whether InnoDB rolled that transaction back, and the message is the same