RedisCache never unregisters its reader: use-after-free after the cache is destroyed #119

RedisAdapter version or revision

main at ff2d5b8. RedisCache.hpp is byte-identical through the #108 → #109 → #111 stack.

Redis version and deployment mode

Redis 7.4.11 standalone (TCP).

Observed behavior

RedisCache::registerCacheReader() registers a reader whose callback captures this:

_ra->addListsReader<Type>(_subkey, [this](...) { writeBuffer(entry); });

~RedisCache() = default never removes it. That leads to two heap-corrupting paths, both on the adapter's worker thread:

1. The adapter outlives the cache. This is the expected case: the cache holds a std::shared_ptr<RedisAdapter> so that many caches can share one adapter. The next stream entry on that key runs writeBuffer on a dead object. It reads readIndex, assigns into freed vectors, and locks a destroyed shared_mutex.
2. The cache is the adapter's only owner. _ra is declared before swapMutex/readIndex/buffers/lastWrite (RedisCache.hpp:19-29), so ~RedisCache frees buffers first and drops _ra last. Only then does the adapter's destructor stop the reader. Entries that arrive in that window are copied into freed vector storage.

Both paths write into freed heap memory. A consumer object that declares a shared adapter member before several RedisCache members hits path 2 whenever it is destroyed while data is flowing.

Expected: destroying a cache stops its callbacks, or at least makes them harmless.

Reproduction

auto ra = std::make_shared<RedisAdapter>("BUAF");
{
  RedisCache<float> cache(ra, "arr");
  std::vector<float> v; cache.copyReadBuffer(v);
}                                   // cache destroyed, adapter alive
// write {BUAF}:arr from another client, wait ~1 s

AddressSanitizer on main and at the stack head:

ERROR: AddressSanitizer: stack-use-after-scope ... READ of size 4 ... thread T1
  #0 RedisCache<float>::writeBuffer(...)
  #1 RedisCache<float>::registerCacheReader()::{lambda}
  #2 RedisAdapter::make_list_reader_callback<float>(...)

T1 is the ThreadPool worker, and the address is readIndex inside the destroyed cache. A heap-allocated cache gives heap-use-after-free … READ … in RedisCache<float>::writeBuffer, freed by ~unique_ptr. Path 2 was reproduced by making the cache the adapter's sole owner and creating and destroying it 30 times while a producer writes 64-float lists at about 2 kHz. The result is ASan WRITE of size 256 into freed memory, inside std::vector::operator= called from writeBuffer.

Heap writes of this kind are a plausible contributor to allocator aborts such as #73, although that is not shown here.

Build and runtime environment

• Ubuntu 24.04, gcc 13.3 with -fsanitize=address,undefined
• pinned redis-plus-plus 1.3.15 and hiredis 1.3.0
• a private Redis 7.4.11 in a container

Suggested fix

Calling _ra->removeReader(_subkey) in the destructor is not enough on its own. It removes every reader of that key, including other caches' readers, and a job may already be queued.

• Put the buffers, lock and flags in a std::shared_ptr<State>. Have the callback capture a std::weak_ptr<State> and do nothing if it has expired.
• Once Add owned stream subscriptions and safe payload decoding #108 lands, also register through subscribeStream() and keep the ReaderHandle as a member, reset in the destructor, so no new callbacks are queued. reset() does not wait for an in-flight callback, so the weak_ptr is still needed.

Related: #82 and #86 (other RedisCache defects), #120, #121.