Stack overflow threadid

[dietmarkuehl]

No description provided.

[Copilot]

Pull request overview

This PR appears to address a coroutine resumption / stack overflow issue by changing the synchronization scheme used by sender_awaitable, and adjusts the Makefile’s compiler/flags handling on macOS (notably for arm64).

Changes:

• Replace sender_awaitable’s completion/suspend handshake from atomic<bool> to atomic<std::thread::id> with CAS-based logic.
• Update macOS compiler selection (clang++ on arm64; g++-15 otherwise) and start passing CMAKE_CXX_FLAGS from Makefile variables during CMake configure.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
include/beman/execution/detail/sender_awaitable.hpp	Alters coroutine resumption coordination logic using thread-id atomics and CAS.
Makefile	Adjusts Darwin compiler selection and propagates flags into CMake via CMAKE_CXX_FLAGS.

Comments suppressed due to low confidence (2)

include/beman/execution/detail/sender_awaitable.hpp:124

• In await_suspend, CAS failure is used to infer that completion happened; however, CAS can also fail simply because the awaitable was constructed on a different thread than await_suspend runs on (or due to the race described above). In that case holds_alternative<monostate> will be true and this returns unhandled_stopped() even though the sender hasn’t completed. Completion detection should be based on an explicit completion/suspended state (like the prior atomic<bool> protocol), not thread-id equality.

    ::std::coroutine_handle<> await_suspend(::std::coroutine_handle<Promise> handle) noexcept {
        ::beman::execution::start(state);
        ::std::thread::id id(::std::this_thread::get_id());
        if (not ::std::get<1>(this->result)
                    .compare_exchange_strong(id, ::std::thread::id{}, ::std::memory_order_acq_rel)) {
            if (::std::holds_alternative<::std::monostate>(::std::get<0>(this->result))) {
                return ::std::get<2>(this->result).promise().unhandled_stopped();
            }
            return ::std::move(handle);
        }

Makefile:141

• This change shifts configuration from the widely-used CXXFLAGS environment variable to CXX_FLAGS. The CMake build logic in this repo reads $ENV{CXXFLAGS} (e.g., cmake/prelude.cmake and cmake/cxx-modules-rules.cmake), so builds that rely on those hooks won’t see the stdlib/extra flags anymore. Either continue exporting CXXFLAGS as well, or update the CMake files to consistently use CXX_FLAGS.

	  -D CMAKE_CXX_COMPILER=$(CXX) --log-level=VERBOSE --fresh \
	  -D CMAKE_CXX_FLAGS="$(CXX_FLAGS) $(SAN_FLAGS)"
	# XXX -D CMAKE_CXX_FLAGS="$(CXX_FLAGS) $(SAN_FLAGS)"

---

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

[Copilot]

std::atomic<std::thread::id> is not guaranteed to be well-formed/portable: std::atomic<T> requires T to be trivially copyable, and std::thread::id is not specified to be. This can fail to compile on some standard libraries. Consider keeping the original std::atomic<bool> state (or using an integral state enum) and store any thread-id info separately if needed.

[Copilot]

This compare_exchange_strong-based gate can resume the coroutine before it has actually suspended: if start(state) causes completion to happen on a different thread before await_suspend executes its CAS, this CAS will fail (thread ids differ) and resume() will call coroutine_handle::resume() while the coroutine is still running, which is undefined behavior. The original atomic<bool> handshake avoided this by only resuming after await_suspend published the suspended state; this should use a proper suspend/completion state machine (e.g., atomic enum/int) rather than comparing thread ids.

[Copilot]

export CXX_FLAGS=-stdlib=$(CXXLIB) overwrites the existing CXX_FLAGS (e.g., drops -g) and is later overwritten by the SANITIZER-specific CXX_FLAGS = ... assignments, so the stdlib selection can be lost. Also, -stdlib=... is a clang-specific flag; if the else branch selects g++-15, passing -stdlib=libstdc++ via CMAKE_CXX_FLAGS will likely fail. Consider keeping stdlib selection in a separate variable and appending it to CXX_FLAGS, and only adding -stdlib=... when using clang/libc++.

[coveralls]

coverage: 95.164% (+0.02%) from 95.14%
when pulling ef19c00 on stack-overflow-threadid
into 3650634 on main.