fix: atomically acquire missing option leases - #3082
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
OptionLeaseStorepath's unsafe WordPress 7add_option()upsert with a prepared plainINSERTowned by the lease store.Atomicity and return semantics
WordPress 7.0.3 implements
add_option()withINSERT ... ON DUPLICATE KEY UPDATE, so a caller that observed a missing row can overwrite a concurrent winner and still receive a truthy result. The replacement uses a plain preparedINSERTagainst the options table. The options table's uniqueoption_namekey serializes competing inserts: the winner receives one affected row, while the loser receives duplicate-key failure and is not reported as acquired. Expected duplicate errors are suppressed at thewpdbdisplay layer, then the prior suppression state is restored.The exact stale-payload
UPDATE ... WHERE option_name = ? AND option_value = ?path introduced for #3072 remains unchanged. Missing-row acquisition and exact stale-row takeover therefore retain separate fencing semantics.Cache behavior
The inserted row uses
autoload = off, matching WordPress's current explicit non-autoload representation. After either insert success or contention, the store invalidates the option's object-cache entry plusnotoptionsandalloptions. This removes each session's cached missing-row snapshot and forces the winner and loser to read authoritative database state. Tests verify read-after-write, loser read-after-contention,notoptionsremoval, and absence fromalloptions.Affected callers
The owner-layer change covers all existing consumers without caller workarounds:
WorkerLockglobal and lane locksRecurringSchedulerschedule mutation leasesPipelineAIConcurrencyLimiternumbered provider/site slotsConcurrency coverage
The two-session test starts a transaction, inserts the winner lease, and issues the competing plain insert asynchronously. It proves the contender blocks on the unique key until commit, resumes with MySQL duplicate error
1062, and cannot overwrite the winner token. The persisted row is also verified as non-autoloaded. Existing two-session exact stale-takeover and generation-CAS tests remain in place.Database assumptions
option_name.1062maps to a non-acquired result; no upsert clause is used.Verification
vendor/bin/phpcs inc/Core/OptionLeaseStore.php tests/Unit/Core/OptionLeaseStoreTest.phpphp tests/prefix-policy-audit.php: 11 passed, 0 failedphp tests/worker-lock-smoke.php: 33 assertions passedphp tests/recurring-scheduler-idempotency-smoke.php: all assertions passedphp tests/ai-step-backpressure-smoke.php: 61 assertions passedOptionLeaseStoreTest.phpbut the installed PHPUnit 9 launcher on PHP 8.4 emitted no output and reported zero executed tests; Homeboy correctly classified this as a zero-test harness failure. CI should execute the WordPress integration suite in its configured environment.Closes #3078