Skip to content

Commit 6dfb1d5

Browse files
facontidavideclaude
andcommitted
Fix TSAN race condition in FileLogger2 with condition_variable
Use proper condition_variable synchronization to ensure the writer thread is fully ready before the constructor returns. This prevents race conditions where callbacks fire before the thread is initialized. Previous atomic-based approach still had TSAN warnings because the callback could start executing while the constructor was blocked waiting for the condition variable, causing a double-lock scenario. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent de0d2e9 commit 6dfb1d5

File tree

1 file changed

+17
-8
lines changed

1 file changed

+17
-8
lines changed

src/loggers/bt_file_logger_v2.cpp

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,11 @@ struct FileLogger2::PImpl
2727

2828
std::thread writer_thread;
2929
std::atomic_bool loop = true;
30-
std::atomic_bool ready = false; // Set to true when writer thread is running
30+
31+
// Synchronization for thread startup
32+
std::mutex ready_mutex;
33+
std::condition_variable ready_cv;
34+
bool ready = false;
3135
};
3236

3337
FileLogger2::FileLogger2(const BT::Tree& tree, std::filesystem::path const& filepath)
@@ -72,6 +76,13 @@ FileLogger2::FileLogger2(const BT::Tree& tree, std::filesystem::path const& file
7276
_p->file_stream.write(write_buffer.data(), 8);
7377

7478
_p->writer_thread = std::thread(&FileLogger2::writerLoop, this);
79+
80+
// Wait for writer thread to be ready before returning
81+
// This prevents race conditions where callbacks fire before thread is initialized
82+
{
83+
std::unique_lock<std::mutex> lock(_p->ready_mutex);
84+
_p->ready_cv.wait(lock, [this]() { return _p->ready; });
85+
}
7586
}
7687

7788
FileLogger2::~FileLogger2()
@@ -85,12 +96,6 @@ FileLogger2::~FileLogger2()
8596
void FileLogger2::callback(Duration timestamp, const TreeNode& node,
8697
NodeStatus /*prev_status*/, NodeStatus status)
8798
{
88-
// Wait until the writer thread is ready to avoid race during construction
89-
if(!_p->ready.load(std::memory_order_acquire))
90-
{
91-
return;
92-
}
93-
9499
Transition trans{};
95100
trans.timestamp_usec = uint64_t(ToUsec(timestamp - _p->first_timestamp));
96101
trans.node_uid = node.UID();
@@ -110,7 +115,11 @@ void FileLogger2::flush()
110115
void FileLogger2::writerLoop()
111116
{
112117
// Signal that the writer thread is ready to accept callbacks
113-
_p->ready.store(true, std::memory_order_release);
118+
{
119+
std::lock_guard<std::mutex> lock(_p->ready_mutex);
120+
_p->ready = true;
121+
}
122+
_p->ready_cv.notify_one();
114123

115124
// local buffer in this thread
116125
std::deque<Transition> transitions;

0 commit comments

Comments
 (0)