SetCommand::CommitOn commits the value twice in the branch that overwrites a non-string key with a string. Because CommitSet moves the value when the source EloqString is owned (which is exactly the replay / remote-cluster / cloned-command case), the first call empties value_ and the second stores "".
Evidence
src/redis_command.cpp:1417-1435 (SetCommand::CommitOn, non-string-object branch):
else { // existing object is not string type
std::unique_ptr<...> new_obj_uptr;
if (obj_expire_ts_ != UINT64_MAX) {
new_obj_uptr = std::make_unique<RedisStringTTLObject>();
auto &str_obj = static_cast<RedisStringTTLObject &>(*new_obj_uptr);
str_obj.CommitSet(value_); // commit #1
new_obj_uptr->SetTTL(obj_expire_ts_);
} else {
new_obj_uptr = StringCommand::CreateObject(nullptr);
auto &str_obj = static_cast<RedisStringObject &>(*new_obj_uptr);
str_obj.CommitSet(value_); // commit #1
}
auto &str_obj = static_cast<RedisStringObject &>(*new_obj_uptr);
str_obj.CommitSet(value_); // commit #2 — redundant + harmful
return ...;
}
src/redis_string_object.cpp:442-454 — CommitSet moves when the value is not a View:
void RedisStringObject::CommitSet(EloqString &val) {
if (val.Type() == StorageType::View) { str_obj_ = val.Clone(); } // normal client cmd: copy
else { str_obj_ = std::move(val); } // replay/cloned cmd: MOVE
}
Normal client SET stores a View into the request buffer, so both calls clone and the bug is masked. Replayed-from-WAL / remote-cluster-forwarded / MULTI-and-Lua-cloned commands carry an owned value, so commit #1 moves it out and commit #2 stores empty.
Repro
HSET k f v (k is a hash), then SET k hello where k is owned by a remote node group (cluster) or after a crash + WAL replay → GET k returns "" instead of hello.
Fix: delete the trailing unconditional str_obj.CommitSet(value_); (lines 1433-1434) — each sub-branch already commits.
Found during a code audit (docs PR #492). Verified against source at the cited lines.
🤖 Found with Claude Code
SetCommand::CommitOncommits the value twice in the branch that overwrites a non-string key with a string. BecauseCommitSetmoves the value when the sourceEloqStringis owned (which is exactly the replay / remote-cluster / cloned-command case), the first call emptiesvalue_and the second stores"".Evidence
src/redis_command.cpp:1417-1435(SetCommand::CommitOn, non-string-object branch):src/redis_string_object.cpp:442-454—CommitSetmoves when the value is not aView:Normal client SET stores a
Viewinto the request buffer, so both calls clone and the bug is masked. Replayed-from-WAL / remote-cluster-forwarded / MULTI-and-Lua-cloned commands carry an owned value, so commit #1 moves it out and commit #2 stores empty.Repro
HSET k f v(k is a hash), thenSET k hellowhere k is owned by a remote node group (cluster) or after a crash + WAL replay →GET kreturns""instead ofhello.Fix: delete the trailing unconditional
str_obj.CommitSet(value_);(lines 1433-1434) — each sub-branch already commits.Found during a code audit (docs PR #492). Verified against source at the cited lines.
🤖 Found with Claude Code