Repository navigation
RedisCache::writeBuffer uses entry.front(): it keeps the oldest entry of a batch (stale value) and crashes on an empty batch #121
Description
RedisAdapter version or revision
main at ff2d5b8. RedisCache.hpp is unchanged through the #108 → #109 → #111 stack; #108 changes which inputs crash (details below).
Redis version and deployment mode
Redis 7.4.11 standalone (TCP).
Observed behavior
RedisCache<T>::writeBuffer takes its data and timestamp from entry.front() (RedisCache.hpp:33 and :42). That is wrong in two ways.
1. Stale value. A legacy list callback receives every entry newer than the reader's cursor, in ascending ID order: XREAD is issued without COUNT, and the cursor then advances to the batch's last ID. When two or more entries arrive in one reply, the cache keeps the oldest value and timestamp and discards the newer ones for good. It stays stale until some later write. Multi-entry batches are routine:
- a producer writing faster than one XREAD round trip plus dispatch, especially over a network;
addLists()batches, which keep all n entries;- reader restarts, which happen on every reader registration or removal in the bucket;
- reconnects, which deliver the outage backlog at once.
2. Crash on an empty batch. The legacy list-reader wrapper calls the user callback even when nothing in the batch decoded. writeBuffer then dereferences a null begin() and segfaults on the worker thread, taking the process down.
- On
main, the trigger is an entry with an empty_payload (a zero-length waveform written withaddSingleList), or an entry without a_field. - With Add owned stream subscriptions and safe payload decoding #108, any entry whose
_length is not a multiple ofsizeof(T)also triggers it. Examples: a strayredis-cli XADD {X}:wave * _ hello!, or a producer/consumer type mismatch.maindecoded those into garbage values instead. - With Add owned stream subscriptions and safe payload decoding #108, an empty list decodes successfully and no longer crashes here. Instead it leaves the cache empty, so the next
copyReadBuffer(value)crashes in Null pointer dereference in RedisCache::copyReadBuffer early-return path #82's path.
Expected: the cache holds the newest entry of every batch, and an empty or undecodable batch leaves the last good value in place.
Reproduction
- Stale value: initialise a
RedisCache<float>on{B}:x. Write value 1 and value 2 in oneMULTI/EXEC, so that the blocked XREAD returns both. Result onmainand at the stack head: Redis latest…-1 value=2, RedisCachetime=…-0 value=1. The cache is stale, and thenewValueAvailableflag is set. - Crash: create
RedisCache<float>(adapter, "wave"), then write an entry with no_field, an empty_(onmain), or a 6-byte_(with Add owned stream subscriptions and safe payload decoding #108):
RedisCache.hpp:33:55: runtime error: member access within null pointer of type 'const struct value_type'
ERROR: AddressSanitizer: SEGV on unknown address 0x000000000010 ... in std::vector<float>::operator= (worker thread)
Build and runtime environment
- Ubuntu 24.04, gcc 13.3 with
-fsanitize=address,undefined,-std=c++20 - pinned redis-plus-plus 1.3.15 and hiredis 1.3.0
- a private Redis 7.4.11 in a container
Suggested fix
- In
writeBuffer:if (entry.empty()) return; const auto& latest = entry.back();, then uselatest.secondfor the buffer andlatest.firstforlastWrite. - In both legacy typed wrappers: invoke the user callback only
if (!ret.empty()). This also protects other consumers that indexats[0], as this repo's own tests do (the Add owned stream subscriptions and safe payload decoding #108 review has the details). - Add a test that delivers two entries in one XREAD and asserts that the cache holds the second.
Activity