Skip to content

Commit 172ad8d

Browse files
fix: ThreadSafeDeque race condition in kill_all()/notify_all() (#298)
* fix: ThreadSafeDeque race condition in kill_all()/notify_all() Fix a race condition in ThreadSafeDeque where kill_all() and notify_all() modified _is_waitable and signaled the condition variable without holding the mutex. This caused a deadlock when the consumer thread (read_row) checked is_waitable()==true but the worker thread called kill_all() before the consumer entered wait(), causing the notify signal to be lost. Timeline of the race: 1. Consumer: records->empty() == true 2. Consumer: records->is_waitable() == true (lock-free atomic read) 3. Worker: kill_all() sets _is_waitable=false, calls _cond.notify_all() 4. Consumer: wait() acquires lock, enters _cond.wait() - DEADLOCK (notification was already sent in step 3, predicate !is_waitable() was already true but checked AFTER the signal was lost) Fix: Acquire the mutex in kill_all() and notify_all() before modifying _is_waitable and signaling. This ensures the condition variable notification cannot be lost between the is_waitable() check and wait(). The bug is most easily triggered with small CSV files where the worker thread completes almost instantly, and is particularly reproducible in no_header() mode on Windows. Also adds a regression test (test_threadsafe_deque_race.cpp) that exercises the race window with rapid small-CSV iterations. * fix: namespace pollution in string_view header Move C++ standard detection and string_view includes before csv namespace to prevent <functional> from being included inside csv namespace. This prevents namespace collision where std:: gets resolved as csv::std:: * Added docs for TSD, timeout to tests --------- Co-authored-by: Vincent La <vincela9@gmail.com>
1 parent 8be1567 commit 172ad8d

11 files changed

Lines changed: 544 additions & 179 deletions

File tree

.claude/rules/testing-conventions.md

Lines changed: 0 additions & 163 deletions
This file was deleted.

0 commit comments

Comments
 (0)