Skip to content

Commit 3871991

Browse files
fix(tsan): eliminate data race in WalMmapWriter move constructor
The open() factory started the background thread with a pointer to a local WalMmapWriter, then returned it by value into expected<>. The move constructor raced with the bg thread: the thread accessed members of the local while the move was transferring ownership. Fix: defer bg thread start via bg_deferred_ flag. open() sets the flag instead of spawning the thread. The move constructor (which constructs the final destination object) starts the thread with a stable `this` pointer. Both move-ctor and move-assign handle the deferred flag and reset it after starting the thread. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent ce52c9e commit 3871991

1 file changed

Lines changed: 22 additions & 12 deletions

File tree

include/signet/ai/wal_mapped_segment.hpp

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -362,18 +362,22 @@ class WalMmapWriter {
362362
WalMmapWriter& operator=(const WalMmapWriter&) = delete;
363363

364364
WalMmapWriter(WalMmapWriter&& o) {
365-
// Stop the source's background thread before transferring ownership
366-
o.bg_stop_.store(true, std::memory_order_release);
367-
o.bg_cv_.notify_all();
368-
if (o.bg_thread_.joinable())
369-
o.bg_thread_.join();
365+
// If the source has a running bg thread, stop it before transferring.
366+
// If bg_deferred_, the thread was never started — skip join.
367+
if (!o.bg_deferred_) {
368+
o.bg_stop_.store(true, std::memory_order_release);
369+
o.bg_cv_.notify_all();
370+
if (o.bg_thread_.joinable())
371+
o.bg_thread_.join();
372+
}
370373

371374
opts_ = std::move(o.opts_);
372375
ring_ = std::move(o.ring_);
373376
active_idx_ = o.active_idx_;
374377
next_seq_ = o.next_seq_;
375378
next_seg_id_ = o.next_seg_id_;
376379
closed_ = o.closed_;
380+
bg_deferred_ = false;
377381
bg_stop_.store(false, std::memory_order_release);
378382
if (!closed_ && !ring_.empty())
379383
bg_thread_ = std::thread(&WalMmapWriter::bg_worker, this);
@@ -385,18 +389,21 @@ class WalMmapWriter {
385389
WalMmapWriter& operator=(WalMmapWriter&& o) {
386390
if (this != &o) {
387391
(void)close();
388-
// Stop source bg thread
389-
o.bg_stop_.store(true, std::memory_order_release);
390-
o.bg_cv_.notify_all();
391-
if (o.bg_thread_.joinable())
392-
o.bg_thread_.join();
392+
// Stop source bg thread (unless deferred)
393+
if (!o.bg_deferred_) {
394+
o.bg_stop_.store(true, std::memory_order_release);
395+
o.bg_cv_.notify_all();
396+
if (o.bg_thread_.joinable())
397+
o.bg_thread_.join();
398+
}
393399

394400
opts_ = std::move(o.opts_);
395401
ring_ = std::move(o.ring_);
396402
active_idx_ = o.active_idx_;
397403
next_seq_ = o.next_seq_;
398404
next_seg_id_ = o.next_seg_id_;
399405
closed_ = o.closed_;
406+
bg_deferred_ = false;
400407
bg_stop_.store(false, std::memory_order_release);
401408
if (!closed_ && !ring_.empty())
402409
bg_thread_ = std::thread(&WalMmapWriter::bg_worker, this);
@@ -477,8 +484,10 @@ class WalMmapWriter {
477484
w.active_idx_ = 0;
478485
w.init_slot_header(*w.ring_[0]);
479486

480-
// Start background thread
481-
w.bg_thread_ = std::thread(&WalMmapWriter::bg_worker, &w);
487+
// Background thread is started by the move constructor to avoid a
488+
// data race: starting the thread here would give it a pointer to the
489+
// local `w`, which is about to be moved into expected<WalMmapWriter>.
490+
w.bg_deferred_ = true;
482491

483492
return w;
484493
}
@@ -877,6 +886,7 @@ class WalMmapWriter {
877886
int64_t next_seq_ = 0;
878887
uint64_t next_seg_id_ = 0; // protected by bg_mutex_
879888
bool closed_ = false;
889+
bool bg_deferred_ = false; // bg thread start deferred until after move
880890

881891
std::thread bg_thread_;
882892
mutable std::mutex bg_mutex_;

0 commit comments

Comments
 (0)