[codex] Fix thread pool coroutine scheduler

[gabewillen]

Summary

Cut over the thread-pool coroutine scheduler to the public thread_pool_scheduler API instead of the temporary _ref wrapper, and align co_sm with the intended deferred-await model for async multi-consumer schedulers.

What changed

• Removed thread_pool_scheduler_ref and moved fork/join support onto thread_pool_scheduler directly.
• Made thread_pool_scheduler::schedule(fn) nonblocking; RTC callers use run_or_schedule_and_wait(fn) or thread_pool_scheduler::join_group explicitly.
• Updated co_sm so async multi-consumer schedulers return pending bool_tasks rather than flattening dispatch into a join inside schedule.
• Added same-actor overlap rejection for async dispatch and made bool_task completion/continuation state safe for cross-thread completion.
• Removed thread-pool hot-path run counters.
• Added thread-pool co_sm coverage and a simple benchmark for blocking vs start-all/await-later dispatch.
• Updated README utility docs for the corrected scheduler behavior.

Why

The previous thread-pool integration copied upstream's blocking schedule behavior into the actor-facing scheduler path. That made process_event_async look asynchronous at the API level while still joining inside scheduler dispatch. This keeps strict schedulers strict and lets thread_pool_scheduler behave as the deferred-await scheduler.

Validation

• cmake --build /tmp/sml-cpp-build --target test_co_sm test_thread_pool_scheduler test_co_sm_thread_pool test_external_completion simple_thread_pool_co_sm -j4
• ctest --test-dir /tmp/sml-cpp-build -R 'test_(co_sm|thread_pool_scheduler|co_sm_thread_pool|external_completion)$' --output-on-failure
• ctest --test-dir /tmp/sml-cpp-build -E '^test_issues_test$' --output-on-failure
• c++ -std=c++20 -fno-exceptions -DBOOST_SML_DISABLE_EXCEPTIONS=1 -Iinclude -Itest -include common/test.hpp test/ft/co_sm.cpp -pthread -o /tmp/test_co_sm_noexc && /tmp/test_co_sm_noexc
• c++ -std=c++20 -fno-exceptions -DBOOST_SML_DISABLE_EXCEPTIONS=1 -Iinclude -Itest -include common/test.hpp test/ft/co_sm_thread_pool.cpp -pthread -o /tmp/test_co_sm_thread_pool_noexc && /tmp/test_co_sm_thread_pool_noexc
• git diff --check

Performance checks

Compared clean 8114aef baseline vs this branch with identical temporary -O3 -DNDEBUG microbenchmarks:

• raw sm::process_event: unchanged, about 1.7 ns/dispatch
• co_sm::process_event default/FIFO: unchanged, about 1.7 ns/dispatch
• co_sm + inline_scheduler async: unchanged, about 1.7 ns/dispatch
• co_sm + FIFO async fast path: unchanged, about 1.8 ns/dispatch
• strict scheduler without try_run_immediate: improved, about 14.7 -> 9.0 ns/dispatch
• deferred scheduler: unchanged, about 18.3 ns/dispatch
• external_completion_scheduler: unchanged, about 2.9-3.0 ns/dispatch
• thread-pool try_submit_with_completion: improved, about 117 -> 103 ns/op
• thread-pool 8-lane fork/join: improved, about 1413 -> 1277 ns/op

Note: full CTest currently has a path-sensitive test_issues_test fixture lookup when run from /tmp/sml-cpp-build; the same binary passes from the repo root. The scheduler-focused tests and full suite excluding that unrelated path issue pass.

---

Note

Medium Risk
Changes concurrency and coroutine lifetime contracts (overlap rejection, incomplete-task termination, cross-thread bool_task); behavior is heavily tested but callers must adopt nonblocking schedule vs run_or_schedule_and_wait.

Overview
Reworks co_sm + thread_pool_scheduler so thread-pool async dispatch is truly deferred: process_event_async queues work and returns a bool_task to poll/co_await, instead of blocking inside scheduler schedule.

thread_pool_scheduler: drops thread_pool_scheduler_ref; join_group and fork/join live on the pool type. schedule(fn) is nonblocking; blocking RTC uses run_or_schedule_and_wait or join_group. Hot-path run counters removed.

co_sm / bool_task: adds an async multi-consumer scheduler contract; bool_task uses atomic completion state and owner callbacks safe for worker-thread finish; result() before ready throws/terminates; destroying an incomplete task terminates. Same-actor overlapping async dispatches are rejected (immediate false task); same-stack process_event reentry still allowed. pooled_coroutine_allocator is spin-locked for concurrent use.

Adds test_co_sm_thread_pool, a simple thread-pool co_sm benchmark, and README docs for the new semantics.

^{Reviewed by Cursor Bugbot for commit c0393a9. Bugbot is set up for automated code reviews on this repo. Configure here.}

[Copilot]

Pull request overview

This PR reworks the thread_pool_scheduler / co_sm integration so thread-pool coroutine dispatch is truly deferred: schedule() becomes nonblocking, explicit join APIs are exposed on thread_pool_scheduler directly, and co_sm supports async multi-consumer schedulers by returning pending bool_tasks instead of joining during scheduling.

Changes:

• Removed thread_pool_scheduler_ref, moved fork/join support onto thread_pool_scheduler, and updated tests accordingly.
• Updated co_sm scheduler contract to support async multi-consumer schedulers and tightened bool_task completion/await semantics for cross-thread completion.
• Added thread-pool co_sm functional coverage and a microbenchmark; updated README utility docs to match the new deferred-await model.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
test/ft/thread_pool_scheduler.cpp	Updates scheduler tests to use thread_pool_scheduler directly and validates inline execution via run_or_schedule_and_wait.
test/ft/co_sm.cpp	Adjusts bool_task tests for new await behavior and exception-guarded test layout.
test/ft/co_sm_thread_pool.cpp	Adds end-to-end coverage for thread-pool-backed co_sm deferred dispatch, overlap rejection, allocator behavior, and termination-on-incomplete-task contract.
test/ft/CMakeLists.txt	Registers the new test_co_sm_thread_pool target.
README.md	Documents the nonblocking schedule() model and deferred-await usage patterns for thread-pool dispatch.
include/boost/sml/utility/thread_pool_scheduler.hpp	Adds join_group to the public scheduler, makes schedule() nonblocking, and removes run counters and the _ref wrapper.
include/boost/sml/utility/co_sm.hpp	Introduces async scheduler contract support, same-actor overlap rejection, and makes bool_task completion/continuation state cross-thread-safe with strict lifetime rules.
benchmark/simple/thread_pool_co_sm.cpp	Adds a benchmark comparing blocking vs start-all/await-later dispatch.
benchmark/simple/CMakeLists.txt	Adds the new benchmark target and links pthreads.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 35848c017d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit dde6264. Configure here.}

[chatgpt-codex-connector]

💡 Codex Review

sml.cpp/include/boost/sml/utility/co_sm.hpp

Line 826 in 8edc299

handle.resume();

Resume continuations after clearing same-actor owner

This resume still runs under active_owner_scope; after final suspend releases async_dispatch_active_, an awaiting coroutine can execute with active_async_dispatch_owner_ == this. If another thread starts a new async dispatch in that window, a synchronous process_event from the continuation will fail try_acquire() but pass the owner check and mutate the same state machine concurrently. Limit the owner scope to the actual state_machine_.process_event call and resume the coroutine after the thread-local owner has been restored.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[cursor]

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_5a53f6fb-62e1-4dde-bbe4-579d1f023da4)

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3a0bd85981

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[cursor]

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_d28e307d-58e8-42b1-a23c-4e20b205266e)