Skip to content

Commit 1c2fba7

Browse files
fix: atomically acquire missing option leases (#3082)
Co-authored-by: homeboy-ci[bot] <266378653+homeboy-ci[bot]@users.noreply.github.com>
1 parent a5f3afd commit 1c2fba7

2 files changed

Lines changed: 151 additions & 1 deletion

File tree

inc/Core/OptionLeaseStore.php

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public static function acquire(
5656
);
5757
}
5858

59-
if ( add_option( $option_name, $payload, '', false ) ) {
59+
if ( self::insertUnlocked( $option_name, $payload ) ) {
6060
return array(
6161
'acquired' => true,
6262
'status' => 'held',
@@ -75,6 +75,42 @@ public static function acquire(
7575
);
7676
}
7777

78+
/**
79+
* Atomically insert a lease only while its option row is absent.
80+
*
81+
* @param array<string,mixed> $payload Lease payload.
82+
*/
83+
private static function insertUnlocked( string $option_name, array $payload ): bool {
84+
global $wpdb;
85+
if ( isset( $wpdb->options ) && method_exists( $wpdb, 'query' ) && method_exists( $wpdb, 'prepare' ) ) {
86+
$previous_suppression = method_exists( $wpdb, 'suppress_errors' ) ? $wpdb->suppress_errors( true ) : null;
87+
// phpcs:disable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching, WordPress.DB.PreparedSQL.NotPrepared -- A plain insert and the option-name unique key are the missing-row fencing primitive.
88+
$inserted = $wpdb->query(
89+
$wpdb->prepare(
90+
'INSERT INTO %i (option_name, option_value, autoload) VALUES (%s, %s, %s)',
91+
$wpdb->options,
92+
$option_name,
93+
maybe_serialize( $payload ),
94+
'off'
95+
)
96+
);
97+
// phpcs:enable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching, WordPress.DB.PreparedSQL.NotPrepared
98+
if ( null !== $previous_suppression ) {
99+
$wpdb->suppress_errors( $previous_suppression );
100+
}
101+
102+
// Both sessions may have cached the missing row before the unique-key race resolves.
103+
wp_cache_delete( $option_name, 'options' );
104+
wp_cache_delete( 'notoptions', 'options' );
105+
wp_cache_delete( 'alloptions', 'options' );
106+
107+
return 1 === $inserted;
108+
}
109+
110+
// Lightweight test runtimes may not provide wpdb.
111+
return add_option( $option_name, $payload, '', false );
112+
}
113+
78114
/**
79115
* Atomically replace one exact stale lease payload.
80116
*

tests/Unit/Core/OptionLeaseStoreTest.php

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,102 @@ public function test_acquire_replaces_an_exact_stale_lease(): void {
7777
$this->assertSame( $fresh, get_option( $this->lease_name ) );
7878
}
7979

80+
public function test_missing_lease_acquisition_is_non_autoloaded_and_readable_after_cached_miss(): void {
81+
$lease = $this->lease();
82+
$this->assertFalse( get_option( $this->lease_name, false ) );
83+
84+
$result = OptionLeaseStore::acquire( $this->lease_name, $lease, 300 );
85+
86+
$this->assertTrue( $result['acquired'] );
87+
$this->assertSame( $lease, get_option( $this->lease_name ) );
88+
$this->assertNotContains( $this->stored_autoload_value(), wp_autoload_values_to_autoload(), true );
89+
$this->assertArrayNotHasKey( $this->lease_name, wp_load_alloptions() );
90+
$notoptions = wp_cache_get( 'notoptions', 'options' );
91+
$this->assertTrue( false === $notoptions || ! isset( $notoptions[ $this->lease_name ] ) );
92+
}
93+
94+
public function test_contended_missing_lease_acquisition_reads_the_winner_after_cached_miss(): void {
95+
global $wpdb;
96+
97+
$winner = $this->lease();
98+
$loser = $winner;
99+
$loser['token'] = 'other-owner';
100+
$this->assertFalse( get_option( $this->lease_name, false ) );
101+
$this->assertSame(
102+
1,
103+
$wpdb->insert(
104+
$wpdb->options,
105+
array(
106+
'option_name' => $this->lease_name,
107+
'option_value' => maybe_serialize( $winner ),
108+
'autoload' => 'off',
109+
),
110+
array( '%s', '%s', '%s' )
111+
)
112+
);
113+
114+
$result = OptionLeaseStore::acquire( $this->lease_name, $loser, 300 );
115+
116+
$this->assertFalse( $result['acquired'] );
117+
$this->assertSame( $winner, $result['payload'] );
118+
$this->assertSame( $winner, get_option( $this->lease_name ) );
119+
$this->assertArrayNotHasKey( $this->lease_name, wp_load_alloptions() );
120+
}
121+
122+
public function test_two_mysql_sessions_allow_only_one_missing_lease_acquisition(): void {
123+
if ( ! class_exists( '\mysqli' ) || ! defined( 'MYSQLI_ASYNC' ) ) {
124+
$this->markTestSkipped( 'MySQLi async support is unavailable.' );
125+
}
126+
127+
$first = $this->open_mysql_connection();
128+
$second = $this->open_mysql_connection();
129+
if ( ! $first instanceof \mysqli || ! $second instanceof \mysqli ) {
130+
$this->markTestSkipped( 'Two direct test database connections are unavailable.' );
131+
}
132+
133+
$winner = $this->lease();
134+
$loser = $winner;
135+
$loser['token'] = 'other-owner';
136+
137+
try {
138+
$this->assertTrue( $first->query( 'SET SESSION innodb_lock_wait_timeout = 2' ) );
139+
$this->assertTrue( $second->query( 'SET SESSION innodb_lock_wait_timeout = 2' ) );
140+
$this->assertTrue( $first->query( 'START TRANSACTION' ) );
141+
$this->assertTrue( $first->query( $this->insert_query( $first, $winner ) ) );
142+
$this->assertSame( 1, $first->affected_rows );
143+
$this->assertTrue( $second->query( $this->insert_query( $second, $loser ), MYSQLI_ASYNC ) );
144+
$read = array( $second );
145+
$error = array();
146+
$reject = array();
147+
$this->assertSame( 0, \mysqli_poll( $read, $error, $reject, 0, 100000 ), 'The competing insert must wait for the option-name unique key.' );
148+
149+
$this->assertTrue( $first->query( 'COMMIT' ) );
150+
$ready = 0;
151+
for ( $attempt = 0; $attempt < 20 && 0 === $ready; ++$attempt ) {
152+
$read = array( $second );
153+
$error = array();
154+
$reject = array();
155+
$ready = \mysqli_poll( $read, $error, $reject, 0, 100000 );
156+
}
157+
158+
$this->assertSame( 1, $ready, 'The competing insert should resume after the winner commits.' );
159+
$this->assertFalse( $second->reap_async_query() );
160+
$this->assertSame( 1062, $second->errno );
161+
$table = $this->options_table();
162+
$lease_name = $first->real_escape_string( $this->lease_name );
163+
$result = $first->query( "SELECT option_value, autoload FROM `{$table}` WHERE option_name = '{$lease_name}'" );
164+
$this->assertInstanceOf( \mysqli_result::class, $result );
165+
$row = $result->fetch_assoc();
166+
$this->assertSame( $winner, maybe_unserialize( $row['option_value'] ) );
167+
$this->assertNotContains( $row['autoload'], wp_autoload_values_to_autoload(), true );
168+
} finally {
169+
$first->query( 'ROLLBACK' );
170+
$second->query( 'ROLLBACK' );
171+
$first->close();
172+
$second->close();
173+
}
174+
}
175+
80176
public function test_two_mysql_sessions_allow_only_one_stale_lease_takeover(): void {
81177
if ( ! class_exists( '\mysqli' ) || ! defined( 'MYSQLI_ASYNC' ) ) {
82178
$this->markTestSkipped( 'MySQLi async support is unavailable.' );
@@ -246,6 +342,24 @@ private function takeover_query( \mysqli $connection, array $stale, array $repla
246342
);
247343
}
248344

345+
private function insert_query( \mysqli $connection, array $lease ): string {
346+
$table = $this->options_table();
347+
348+
return sprintf(
349+
"INSERT INTO `{$table}` (option_name, option_value, autoload) VALUES ('%s', '%s', 'off')",
350+
$connection->real_escape_string( $this->lease_name ),
351+
$connection->real_escape_string( maybe_serialize( $lease ) )
352+
);
353+
}
354+
355+
private function stored_autoload_value(): string {
356+
global $wpdb;
357+
358+
return (string) $wpdb->get_var(
359+
$wpdb->prepare( 'SELECT autoload FROM %i WHERE option_name = %s', $wpdb->options, $this->lease_name )
360+
);
361+
}
362+
249363
private function options_table(): string {
250364
global $wpdb;
251365

0 commit comments

Comments
 (0)