Bug: MULTI/EXEC with multiple SETs to existing keys applies last value to all keys

[SoumojitDalui]

Summary

When a single MULTI/EXEC block issues two or more SET commands targeting pre-existing keys with different values, all updated keys end up with the value of the last SET command. New-key inserts are unaffected.

Reproduction

SET src "1000"
SET dst "1000"
MULTI
  SET src "900"
  SET dst "1100"
EXEC
GET src  →  "1100"  (BUG: should be "900")
GET dst  →  "1100"  (correct)

Characterization

Scenario	Behavior
MULTI with 2 SETs to new keys, different values	Correct: each key gets its own value
MULTI with 2 SETs to existing keys, same value	Correct: both get the value
MULTI with 2 SETs to existing keys, different values	BUG: both get the last value
MULTI with 1 SET to an existing key	Correct: single overwrite works
MULTI with same key twice (existing)	Correct: last write wins (expected)

The bug only manifests when overwriting pre-existing keys because new-key inserts go through the PUT-Absent path in Sto, which works correctly.

Root Cause

File: examples/makoCon.cc, function execute_transaction

The C++ execute_transaction function uses the thread-local tl_val_buf (std::string) as the encoding buffer for all SET operations in the transaction loop. mbta_sharded_ordered_index::Put wraps the value in a StringWrapper, which stores only a pointer to the passed std::string — no copy is made.

Since all SET operations share the same tl_val_buf, all stored StringWrapper instances alias the same buffer. By the time Commit() runs, tl_val_buf holds the last encoded value, so all keys receive that value.

// Pseudocode of the buggy path:
for each SET operation:
    tl_val_buf = encode(value)           // overwrites the shared buffer
    Put(txn, key, tl_val_buf)            // StringWrapper stores POINTER to tl_val_buf
Commit()                                  // all StringWrappers point to the same tl_val_buf
                                          // which contains the last value

Impact

Severity: Critical. Any MULTI block that updates two or more existing keys with different values will silently corrupt data. Affected use cases:

• Bank transfers (two-account atomic update)
• Swap or move operations
• Multi-field record updates (if fields are separate keys)
• Batch counter updates

This bug is silent — no error is returned, the transaction commits successfully, and the data is wrong.

Suggested Fix

Pre-encode all SET values into independent std::string instances before the transaction loop, so each StringWrapper points to its own stable storage that outlives Commit().

+ // Pre-encode all SET values into independent strings
+ std::vector<std::string> encoded_vals(num_ops);
+ for (int i = 0; i < num_ops; i++) {
+     if (ops[i].type == SET) {
+         mako::Encode(ops[i].value, &encoded_vals[i]);
+     }
+ }

  for (int i = 0; i < num_ops; i++) {
      if (ops[i].type == SET) {
-         mako::Encode(ops[i].value, &tl_val_buf);
-         table->Put(txn, ops[i].key, tl_val_buf);
+         table->Put(txn, ops[i].key, encoded_vals[i]);
      }
      // ... other ops
  }
  Commit();

A reference implementation of this fix exists in commit cc8205c6 on mako-dev.

Why Existing Tests Did Not Catch This

The transaction tests (Tasks 2.1 and 2.7 in the correctness suite) used keys that had never been written before. New-key inserts go through the PUT-Absent path, which works correctly. The bug only appears on the PUT-Found (overwrite) path with multiple keys in one transaction.

This was discovered by the bank simulation workload test, which creates accounts first and then atomically transfers between them using MULTI/EXEC.

Test Environment

• Server: build/makoCon (Redis-compatible Mako server)
• Storage: In-memory Masstree (no RocksDB persistence)
• Client: Python 3.10.12 with redis-py 7.1.0
• Host: Linux 5.15.0-133-generic (x86_64)