Skip to content

Commit 88368c8

Browse files
mhaynieclaude
andcommitted
fix: Fix dispatcher race conditions and add sanitizers to CI
- Fix race condition in try_pop_task: hold lock for entire operation - Fix lost tasks: re-queue extra FD tasks instead of discarding them - Require unique_lock parameter to enforce locking at compile time - Add ASan, TSan, and UBSan CI jobs to catch threading/memory issues Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent fb751ae commit 88368c8

File tree

2 files changed

+71
-29
lines changed

2 files changed

+71
-29
lines changed

.github/workflows/ccpp.yml

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -124,15 +124,54 @@ jobs:
124124
# port-name: mh-stuff
125125
# workflow-pat: ${{ secrets.WORKFLOW_PAT }}
126126

127+
sanitizers:
128+
runs-on: ubuntu-latest
129+
strategy:
130+
fail-fast: true
131+
matrix:
132+
sanitizer:
133+
- name: asan
134+
flags: -fsanitize=address -fno-omit-frame-pointer
135+
- name: tsan
136+
flags: -fsanitize=thread
137+
- name: ubsan
138+
flags: -fsanitize=undefined -fno-omit-frame-pointer
139+
140+
steps:
141+
- uses: actions/checkout@v4
142+
143+
- name: Install tools
144+
run: |
145+
sudo apt-get update
146+
sudo apt-get install -y clang-15 libc++-15-dev libc++abi-15-dev ninja-build libcurl4-openssl-dev
147+
148+
- name: Build with ${{ matrix.sanitizer.name }}
149+
env:
150+
CXX: clang++-15
151+
CXXFLAGS: ${{ matrix.sanitizer.flags }} -g -O1
152+
run: |
153+
mkdir build
154+
cd build
155+
cmake -G Ninja -DMH_STUFF_COMPILE_LIBRARY=ON ../
156+
cmake --build .
157+
158+
- name: Run tests with ${{ matrix.sanitizer.name }}
159+
env:
160+
ASAN_OPTIONS: detect_leaks=1:abort_on_error=1
161+
TSAN_OPTIONS: abort_on_error=1
162+
UBSAN_OPTIONS: print_stacktrace=1:abort_on_error=1
163+
run: |
164+
cd build
165+
ctest --output-on-failure
166+
127167
all-checks-passed:
128168
if: always()
129-
needs: [build-linux]
169+
needs: [build-linux, sanitizers]
130170
runs-on: ubuntu-latest
131171
steps:
132172
- name: Verify all checks passed
133-
run: |
134-
if [[ "${{ needs.build-linux.result }}" != "success" ]]; then
135-
echo "build-linux failed: ${{ needs.build-linux.result }}"
136-
exit 1
137-
fi
138-
echo "All checks passed!"
173+
if: ${{ !(contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled')) }}
174+
run: echo "All checks passed!"
175+
- name: Fail if any check failed
176+
if: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') }}
177+
run: exit 1

cpp/include/mh/concurrency/dispatcher.inl

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -66,37 +66,40 @@ namespace mh
6666

6767
coro::coroutine_handle<> try_pop_task()
6868
{
69+
std::unique_lock lock(m_TasksMutex);
70+
6971
// Check for ready FDs first
70-
auto ready_fd_tasks = check_fd_tasks();
72+
auto ready_fd_tasks = check_fd_tasks_locked(lock);
7173
if (!ready_fd_tasks.empty())
7274
{
73-
return ready_fd_tasks[0]; // Return first ready FD task
74-
}
75-
76-
if (!m_Tasks.empty() || !m_DelayTasks.empty())
77-
{
78-
std::lock_guard lock(m_TasksMutex);
79-
80-
if (!m_DelayTasks.empty())
75+
// Return first task, re-queue any extras
76+
auto task = ready_fd_tasks[0];
77+
for (size_t i = 1; i < ready_fd_tasks.size(); ++i)
8178
{
82-
auto now = clock_t::now();
83-
const task_delay_data &taskDelayData = m_DelayTasks.front();
84-
if (taskDelayData.m_DelayUntilTime <= now)
85-
{
86-
auto task = taskDelayData.m_Handle;
87-
m_DelayTasks.pop();
88-
return task;
89-
}
79+
m_Tasks.push(ready_fd_tasks[i]);
9080
}
81+
return task;
82+
}
9183

92-
if (!m_Tasks.empty())
84+
if (!m_DelayTasks.empty())
85+
{
86+
auto now = clock_t::now();
87+
const task_delay_data &taskDelayData = m_DelayTasks.front();
88+
if (taskDelayData.m_DelayUntilTime <= now)
9389
{
94-
auto task = m_Tasks.front();
95-
m_Tasks.pop();
90+
auto task = taskDelayData.m_Handle;
91+
m_DelayTasks.pop();
9692
return task;
9793
}
9894
}
9995

96+
if (!m_Tasks.empty())
97+
{
98+
auto task = m_Tasks.front();
99+
m_Tasks.pop();
100+
return task;
101+
}
102+
100103
return nullptr;
101104
}
102105

@@ -152,10 +155,10 @@ namespace mh
152155
}
153156

154157
// Check for ready FDs and return ready tasks
155-
std::vector<coro::coroutine_handle<>> check_fd_tasks()
158+
std::vector<coro::coroutine_handle<>> check_fd_tasks_locked(std::unique_lock<std::mutex>& lock)
156159
{
160+
assert(lock.owns_lock() && lock.mutex() == &m_TasksMutex);
157161
std::vector<coro::coroutine_handle<>> ready_tasks;
158-
std::lock_guard lock(m_TasksMutex);
159162

160163
#ifdef _WIN32
161164
// Windows: stub implementation for now

0 commit comments

Comments
 (0)