Commit a24f042963b for woocommerce
commit a24f042963b451ff5d33798cd8a4d526fcdc2923
Author: Christopher Hertel <mail@christopher-hertel.de>
Date: Tue Nov 10 19:42:48 2026 +0100
Restore autocommit when an ability transaction fails to start (#70814)
* Restore autocommit when a transaction fails to start
DatabaseTransaction::run() turned autocommit off and started the
transaction before its try block. When a query filter threw on
START TRANSACTION, the finally that turns autocommit back on never ran,
so later writes in the request stayed uncommitted.
Opening the transaction now happens inside the try. The rollback runs
only when the start succeeded, because rolling back to a savepoint that
was never set is a database error of its own.
* Cover the joined and autocommit-off paths of a failed transaction start
A savepoint that fails inside a transaction a caller started used to
leave run() owning a level, so is_open() stayed true. A throw on
SET autocommit = 0 now reaches the finally too. Test both.
diff --git a/plugins/woocommerce/changelog/wooairr-743-autocommit-restore b/plugins/woocommerce/changelog/wooairr-743-autocommit-restore
new file mode 100644
index 00000000000..a300eaac4a2
--- /dev/null
+++ b/plugins/woocommerce/changelog/wooairr-743-autocommit-restore
@@ -0,0 +1,4 @@
+Significance: patch
+Type: dev
+Comment: Restore autocommit when unreleased ability transaction code fails to start; nothing shipped is affected.
+
diff --git a/plugins/woocommerce/src/Internal/Utilities/DatabaseTransaction.php b/plugins/woocommerce/src/Internal/Utilities/DatabaseTransaction.php
index 262d2c4ee6b..3bd166d70ad 100644
--- a/plugins/woocommerce/src/Internal/Utilities/DatabaseTransaction.php
+++ b/plugins/woocommerce/src/Internal/Utilities/DatabaseTransaction.php
@@ -89,21 +89,26 @@ final class DatabaseTransaction {
$outermost = ! self::$watching;
$autocommit = 0 === $level && self::is_autocommit_on();
self::$depth = $level;
- if ( $autocommit ) {
- self::query( 'SET autocommit = 0' );
- }
- self::start();
- if ( $outermost ) {
- self::$lost = false;
- self::$watching = true;
- add_filter( 'query', array( self::class, 'watch' ) );
- }
+ $started = false;
try {
+ if ( $autocommit ) {
+ self::query( 'SET autocommit = 0' );
+ }
+ self::start();
+ $started = true;
+ if ( $outermost ) {
+ self::$lost = false;
+ self::$watching = true;
+ add_filter( 'query', array( self::class, 'watch' ) );
+ }
$result = $callback();
self::end( $level, ! is_wp_error( $result ) );
$lost = self::$lost;
} catch ( \Throwable $e ) {
- self::end( $level, false );
+ // Rolling back to a savepoint that was never set is itself a database error.
+ if ( $started ) {
+ self::end( $level, false );
+ }
throw $e;
} finally {
if ( $outermost ) {
diff --git a/plugins/woocommerce/tests/php/src/Internal/Utilities/DatabaseTransactionTest.php b/plugins/woocommerce/tests/php/src/Internal/Utilities/DatabaseTransactionTest.php
index a63082e3e1a..a6e530609b7 100644
--- a/plugins/woocommerce/tests/php/src/Internal/Utilities/DatabaseTransactionTest.php
+++ b/plugins/woocommerce/tests/php/src/Internal/Utilities/DatabaseTransactionTest.php
@@ -400,6 +400,127 @@ class DatabaseTransactionTest extends WC_Unit_Test_Case {
$this->assertSame( array( 'SELECT @@autocommit', 'SET autocommit = 0', 'START TRANSACTION', 'COMMIT', 'SET autocommit = 1' ), $this->statements );
}
+ /**
+ * @testdox run() should turn autocommit back on and own no transaction after a query filter throws on its start.
+ */
+ public function test_run_restores_autocommit_after_its_start_throws(): void {
+ $throw_on_start = static function ( $query ) {
+ if ( 'START TRANSACTION' === $query ) {
+ throw new \RuntimeException( 'Start refused.' );
+ }
+ return $query;
+ };
+ add_filter( 'query', $throw_on_start, 0 );
+ $called = false;
+ try {
+ DatabaseTransaction::run(
+ static function () use ( &$called ) {
+ $called = true;
+ }
+ );
+ $this->fail( 'The exception should reach the caller.' );
+ } catch ( \RuntimeException $e ) {
+ $this->assertSame( 'Start refused.', $e->getMessage() );
+ } finally {
+ remove_filter( 'query', $throw_on_start, 0 );
+ }
+
+ $this->assertFalse( $called, 'The callback should not run without a transaction.' );
+ $this->assertSame( array( 'SELECT @@autocommit', 'SET autocommit = 0', 'SET autocommit = 1' ), $this->statements );
+ $this->assertFalse( DatabaseTransaction::is_open() );
+ $this->assertFalse( has_filter( 'query', array( DatabaseTransaction::class, 'watch' ) ) );
+ }
+
+ /**
+ * @testdox run() should roll back no savepoint after a query filter throws on setting it.
+ */
+ public function test_run_rolls_back_no_savepoint_after_setting_it_throws(): void {
+ $throw_on_savepoint = static function ( $query ) {
+ if ( 0 === strpos( $query, 'SAVEPOINT' ) ) {
+ throw new \RuntimeException( 'Savepoint refused.' );
+ }
+ return $query;
+ };
+
+ $inner_error = null;
+ DatabaseTransaction::run(
+ static function () use ( $throw_on_savepoint, &$inner_error ) {
+ add_filter( 'query', $throw_on_savepoint, 0 );
+ try {
+ DatabaseTransaction::run( static fn() => null );
+ } catch ( \RuntimeException $e ) {
+ $inner_error = $e->getMessage();
+ } finally {
+ remove_filter( 'query', $throw_on_savepoint, 0 );
+ }
+ return null;
+ }
+ );
+
+ $this->assertSame( 'Savepoint refused.', $inner_error );
+ $this->assertSame( array( 'SELECT @@autocommit', 'SET autocommit = 0', 'START TRANSACTION', 'COMMIT', 'SET autocommit = 1' ), $this->statements );
+ $this->assertFalse( DatabaseTransaction::is_open() );
+ }
+
+ /**
+ * @testdox run() should own no transaction after a query filter throws on its savepoint inside a transaction that a caller started.
+ */
+ public function test_run_owns_no_transaction_after_joining_a_caller_transaction_throws(): void {
+ $throw_on_savepoint = static function ( $query ) {
+ if ( 0 === strpos( $query, 'SAVEPOINT' ) ) {
+ throw new \RuntimeException( 'Savepoint refused.' );
+ }
+ return $query;
+ };
+ wc_transaction_query( 'start' );
+ add_filter( 'query', $throw_on_savepoint, 0 );
+ try {
+ DatabaseTransaction::run( static fn() => null );
+ $this->fail( 'The exception should reach the caller.' );
+ } catch ( \RuntimeException $e ) {
+ $this->assertSame( 'Savepoint refused.', $e->getMessage() );
+ } finally {
+ remove_filter( 'query', $throw_on_savepoint, 0 );
+ }
+
+ $this->assertFalse( DatabaseTransaction::is_open(), 'The caller should still own its transaction.' );
+ wc_transaction_query( 'rollback' );
+ DatabaseTransaction::run( static fn() => null );
+
+ $this->assertSame( array( 'START TRANSACTION', 'ROLLBACK', 'SELECT @@autocommit', 'SET autocommit = 0', 'START TRANSACTION', 'COMMIT', 'SET autocommit = 1' ), $this->statements );
+ }
+
+ /**
+ * @testdox run() should turn autocommit back on and not run the callback after a query filter throws on turning it off.
+ */
+ public function test_run_restores_autocommit_after_turning_it_off_throws(): void {
+ $throw_on_autocommit_off = static function ( $query ) {
+ if ( 'SET autocommit = 0' === $query ) {
+ throw new \RuntimeException( 'Autocommit change refused.' );
+ }
+ return $query;
+ };
+ add_filter( 'query', $throw_on_autocommit_off, 0 );
+ $called = false;
+ try {
+ DatabaseTransaction::run(
+ static function () use ( &$called ) {
+ $called = true;
+ }
+ );
+ $this->fail( 'The exception should reach the caller.' );
+ } catch ( \RuntimeException $e ) {
+ $this->assertSame( 'Autocommit change refused.', $e->getMessage() );
+ } finally {
+ remove_filter( 'query', $throw_on_autocommit_off, 0 );
+ }
+
+ $this->assertFalse( $called, 'The callback should not run without a transaction.' );
+ $this->assertSame( array( 'SELECT @@autocommit', 'SET autocommit = 1' ), $this->statements );
+ $this->assertFalse( DatabaseTransaction::is_open() );
+ $this->assertFalse( has_filter( 'query', array( DatabaseTransaction::class, 'watch' ) ) );
+ }
+
/**
* @testdox run() should leave autocommit alone when the connection already has it off.
*/