diff --git a/src/Integrations/Divi/Design/DesignLock.php b/src/Integrations/Divi/Design/DesignLock.php index 747d1fb..21aa837 100644 --- a/src/Integrations/Divi/Design/DesignLock.php +++ b/src/Integrations/Divi/Design/DesignLock.php @@ -15,7 +15,8 @@ defined('ABSPATH') || exit; * DiviCheckRoute (add_option() reads first and writes after, so two requests could both win; audit L6). A lock * older than STALE_AFTER seconds (a request that died) is taken over with a conditional UPDATE on the value just * read, so only one taker wins; release() deletes the row only while it still holds this instance's token, so a - * holder that lost a stale lock cannot free the new holder's. + * holder that lost a stale lock cannot free the new holder's. A taken lock drops the cache of the autoloaded + * options, so the inventory read under the lock (and design_rev) is the stored one. */ final class DesignLock { @@ -65,6 +66,9 @@ final class DesignLock } if ($taken) { $this->tokens[$option] = $token; + // Audit L5: what the holder reads and rewrites must be the stored state, not the request-start cache + // (autoloaded options like Divi's live in "alloptions"); another writer may have finished since. + wp_cache_delete('alloptions', 'options'); } return $taken; diff --git a/tests/Unit/Integrations/Divi/Design/DesignLockTest.php b/tests/Unit/Integrations/Divi/Design/DesignLockTest.php index dc260aa..e448db1 100644 --- a/tests/Unit/Integrations/Divi/Design/DesignLockTest.php +++ b/tests/Unit/Integrations/Divi/Design/DesignLockTest.php @@ -94,4 +94,34 @@ final class DesignLockTest extends TestCase $this->expectException(InvalidArgumentException::class); DesignLock::option('pages'); } + + /** + * Audit L5 (re-review): get_option('et_divi') comes from the object cache loaded at the start of the request + * (autoload / alloptions). A writer that finished meanwhile is invisible until the lock holder drops that cache. + */ + public function testTheLockHolderReadsEtDiviFreshNotFromTheRequestStartCache(): void + { + $db = $this->table(); + $stored = ['et_divi' => ['color' => 'alt']]; + $cache = []; + Functions\when('get_option')->alias(static function (string $name) use (&$stored, &$cache) { + if (! isset($cache['options:' . $name]) && ! isset($cache['options:alloptions'][$name])) { + $cache['options:alloptions'][$name] = $stored[$name] ?? false; + } + + return $cache['options:' . $name] ?? $cache['options:alloptions'][$name]; + }); + Functions\when('wp_cache_delete')->alias(static function (string $key, string $group = '') use (&$cache): bool { + unset($cache[$group . ':' . $key]); + + return true; + }); + + self::assertSame(['color' => 'alt'], get_option('et_divi'), 'request B cached the old array'); + $stored['et_divi'] = ['color' => 'neu']; // request A wrote and released meanwhile + + self::assertTrue((new DesignLock())->acquire('colors')); + self::assertSame(['color' => 'neu'], get_option('et_divi')); + self::assertNotSame([], $db->rows); + } } diff --git a/tests/Unit/TestCase.php b/tests/Unit/TestCase.php index 269a3bc..5f9a98b 100644 --- a/tests/Unit/TestCase.php +++ b/tests/Unit/TestCase.php @@ -33,6 +33,7 @@ abstract class TestCase extends BaseTestCase 'absint' => static fn ($value): int => abs((int) $value), 'wp_slash' => static fn ($value) => $value, 'wp_unslash' => static fn ($value) => $value, + 'wp_cache_delete' => true, 'sanitize_text_field' => static fn ($value): string => trim(strip_tags((string) $value)), ]);