wp_cache_replace() creates a missing Redis key while returning false

Hi,

Thanks for maintaining Redis Object Cache.

While testing version 2.8.0 against a Redis/PhpRedis instance, I found behavior that appears to violate the WordPress wp_cache_replace() contract.

WordPress documents wp_cache_replace() as replacing a value only when the key already exists:

https://developer.wordpress.org/reference/functions/wp_cache_replace/

Environment

Here’s my config:

• Redis Object Cache: 2.8.0
• Drop-in: unmodified includes/object-cache.php from the official 2.8.0 release
• Drop-in SHA-256: 713e9a12018e865f7daf71d84e6257b7756251560f04aac6cc6b5b15bf9d0b33
• Client: PhpRedis 6.2.0
• Redis server: 8.0.2
• PHP: 8.4.24

Minimal repro. (With Redis Object Cache enabled and connected, run):

wp eval '
$key = "redis-cache-replace-repro-" . wp_generate_uuid4();
$group = "redis-cache-replace-repro";

$replace_result = wp_cache_replace($key, "created", $group);

$found = null;
$value = wp_cache_get($key, $group, true, $found);

var_export([
    "replace_result" => $replace_result,
    "found_after_replace" => $found,
    "value_after_replace" => $value,
]);

wp_cache_delete($key, $group);
'

Expected result

Because the key did not exist, wp_cache_replace() should return false and leave it absent:

array (
  'replace_result' => false,
  'found_after_replace' => false,
  'value_after_replace' => false,
)

Actual result

wp_cache_replace() returns false, but the key has been created in Redis:

array (
  'replace_result' => false,
  'found_after_replace' => true,
  'value_after_replace' => 'created',
)

I also reproduced this directly, outside of WordPress. The observed values were:

bool(false)
bool(true)
string(7) "created"

Suspected cause

In WP_Object_Cache::add_or_replace(), add() uses an atomic conditional Redis SET ... NX, but the replace() branch performs an unconditional SET or SETEX:

https://github.com/rhubarbgroup/redis-cache/blob/2.8.0/includes/object-cache.php#L1307-L1375

The method checks the request-local $this->cache only after the Redis write has already occurred. Consequently:

1. Replacing a missing key creates it while returning false.
2. Replacing a key that exists in Redis but has not been loaded into the current request changes it while returning false.

Would using Redis SET ... XX for the persistent replace() path be the appropriate fix? That would make the existence check and write atomic, analogous to the existing NX implementation for add().

I noticed the repository already contains testReplace() and testWpCacheReplace(). Is it possible those tests are exercising WordPress core’s runtime WP_Object_Cache rather than the Redis drop-in because WordPress is bootstrapped before the drop-in is copied? I may be overlooking part of the test setup, so confirmation would be appreciated.

Cheers.
Dan

• This topic was modified 2 weeks, 1 day ago by turbodb.
• This topic was modified 2 weeks, 1 day ago by turbodb.