Summary

touchWatchedKey() in multi.c has a break statement that exits the watcher iteration loop early when it encounters a watcher with wk->expired == 1 whose key still exists in the DB (dbFind != NULL). This causes all subsequent watchers in the list to never be visited — they never receive CLIENT_DIRTY_CAS, and their MULTI/EXEC transactions incorrectly succeed instead of being aborted.

The fix: Remove the break and let the code fall through to the existing CLIENT_DIRTY_CAS / unwatchAllKeys() path, which correctly flags the client as dirty.

Root Cause Analysis

The relevant code in touchWatchedKey() handles watchers whose key was already expired at WATCH time (wk->expired == 1):

if (wk->expired) {
    if (db == wk->db &&
        equalStringObjects(key, wk->key) &&
        dbFind(db, key->ptr) == NULL)
    {
        /* Expired key is deleted — no logical change. Clear flag. */
        wk->expired = 0;
        goto skip_client;
    }
    break;  // <-- BUG: exits entire loop, skipping remaining watchers
}

c->flags |= CLIENT_DIRTY_CAS;
unwatchAllKeys(c);

When dbFind != NULL (the expired key has been re-created), the break exits the while loop entirely. Every watcher after the current one in the list is silently skipped.

How to Trigger

On a standalone master, this bug is unreachable. Lazy expiry (expireIfNeeded with EXPIRE_FORCE_DELETE_EXPIRED) always deletes the expired key before the write creates a new value. This produces two touchWatchedKey calls: first with dbFind == NULL (which clears wk->expired flags via the goto skip_client path), then with dbFind != NULL (which notifies watchers normally since expired flags are already cleared). The break is never reached.

The bug is triggered on writable replicas (replica-read-only no) when a key is locally expired on the replica but remains alive on the master:

1. Master creates a key with no TTL (key never expires on master)
2. The key is replicated to the writable replica
3. On the replica, a local PEXPIREAT sets a short expiry (this local change is not replicated back)
4. The key expires on the replica; multiple clients WATCH it (wk->expired = 1)
5. Master does SET on the same key — since the key is not expired on the master, no DEL is propagated
6. The replica receives the replicated SET with CLIENT_MASTER flag
7. expireIfNeeded() returns KEY_VALID without deleting the expired key (due to CLIENT_MASTER check at db.c:2888)
8. A single touchWatchedKey() call occurs with dbFind != NULL
9. The break fires at the first expired watcher, skipping all remaining watchers
10. Skipped watchers' transactions incorrectly succeed on EXEC

This can also occur through clock skew between master and replica, where a key naturally expires on the replica before the master.

Changes

• src/multi.c: Replace break with a comment explaining the fall-through to the CLIENT_DIRTY_CAS path. When a key was expired at WATCH time but has since been re-created, this is a logical state change that should invalidate the transaction.

• tests/unit/multi.tcl: Two regression tests using a writable replica setup:

  • 3 expired watchers: verifies all three transactions are aborted when the watched key is re-created via replication
  • 2 expired watchers: minimal reproduction case