Race Condition Between GC Chunk Reset and Background Expired ReplReq Cleanup

[Besroy]

During GC task processing, a move_to_chunk is selected from m_reserved_chunk_queue and reset via purge_reserved_chunk() to ensure the chunk is completely clean. This operation resets the old append blk allocator and creates a new one to replace it.

At the time of chunk reset, there may exist stale rreqs associated with the move_to_chunk. When these rreqs are cleaned up by the background gc_repl_reqs(), they will free blocks on the chunk, creating two potential race conditions:

Risk 1: Free on New Allocator

• If the expired rreq is cleaned up AFTER the new allocator is created, the free operation targets the new allocator instead of the old one
• Currently no immediate impact because append allocator's free() only increments m_freeable_nblks counter
• Future risk: If real free operations are implemented, this will cause unexpected free on wrong allocator

Risk 2: Free on Destroyed Allocator

• If the expired rreq is cleaned up AFTER the old allocator is reset but BEFORE the new allocator is created, the free operation accesses a destroyed allocator
• This causes crashes due to accessing freed memory (e.g., destroyed superblock). Here is a timeline of observed crash:
  T1: cp_flush obtains pointer to old allocator
  T2: GC resets allocator (destroys old superblock), set m_is_dirty=false on old alloactor
  T3: gc_repl_reqs frees expired rreq → sets m_is_dirty=true on old allocator pointer
  T4: cp_flush executes on old allocator → enter m_sb write due to m_is_dirty is true -> accesses destroyed superblock → crash

[Besroy]

One solution is clear chunk rreqs before purge chunk, but this introduces a potential double-free issue.

Currently this risk is acceptable because append allocator's free() only increments a counter (m_freeable_nblks), so double-free just causes an incorrect counter value. If the free logic is updated in the future to perform actual deallocation, BlkAllocator::free() should be made idempotent at that time.

Double-Free Race Timeline
T1: gc_repl_reqs() collects snapshot: expired_rreqs = [rreq1, ...]
T2: clear_chunk_req(chunk_id) executes:
- async_free_blk(blkid1) → waiter queued in blk_read_tracker
- m_repl_key_req_map.erase(key) in callback
T3: gc_repl_reqs() processes snapshot:
- async_free_blk(blkid1) again → second waiter queued
- Both waiters will execute allocator->free(blkid1)

Root Cause: blk_read_tracker::wait_on() does NOT deduplicate - multiple waiters for the same blkid will all be invoked.

Risk: Same blk freed twice, causing incorrect free counter (currently harmless with append allocator's counter-only implementation).

Root Solution: Make BlkAllocator::free() idempotent - check if blk is already freed before freeing.

[xiaoxichen]

can we add an atomic into repl_req_t to prevent double freeing ?