CurlMulti: cv.notify_one().resume() on a temporary task is UB when contended → SIGSEGV under concurrency

[Toilettrauma]

Summary

CurlMulti notifies a transfer's awaiter with

handle_awaiter.cv.notify_one().resume();

in deliver_finished() and abort_cancelled(). coro::condition_variable::notify_one() returns an owning coro::task<void>, and calling .resume() on a temporary is only safe while that task never suspends. It can suspend — and when it does, the temporary is destroyed at the end of the full expression, taking a suspended coroutine frame with it. That frame has already registered itself in mutex_'s waiter list, so the next unlock() resumes a dangling handle.

Result: SIGSEGV under concurrent load.

Symptom

EXC_BAD_ACCESS (SIGSEGV), KERN_INVALID_ADDRESS at 0x0000000000000000

0  std::coroutine_handle<void>::resume() const
1  coro::mutex::unlock()
2  coro::condition_variable::awaiter_with_predicate::await_suspend(std::coroutine_handle<void>)
3  ...
4  std::coroutine_handle<void>::resume() const
5  coro::thread_pool::executor(unsigned long)

A null/dangling coroutine handle being resumed out of the mutex's waiter list — not out of anything the caller owns.

Root cause

awaiter_with_predicate::on_notify() (libcoro) acquires the same mutex the caller may be contending for:

auto condition_variable::awaiter_with_predicate::on_notify() -> coro::task<notify_status_t>
{
    co_await m_lock.m_mutex->lock();      // <-- suspends if contended
    if (m_predicate())
    {
        m_awaiting_coroutine.resume();
        co_return notify_status_t::ready;
    }
    m_lock.m_mutex->unlock();
    co_return notify_status_t::not_ready;
}

So notify_one() is a coroutine that takes mutex_. .resume() on the returned temporary starts it; if it parks on that lock, the task object dies at the ; while suspended.

abort_all() is the only caller that does this correctly — it is a coroutine, so it can co_await:

co_await awaiter_context.cv.notify_one();

deliver_finished() / abort_cancelled() are plain functions and cannot, which is presumably why .resume() was reached for.

Why "just notify with the lock released" is not the fix

The current code already notifies without holding mutex_, and still crashes. on_notify() contends with the pool threads inside perform_handle(), which take mutex_ to register their awaiter — not only with the polling thread. Releasing the lock narrows the window; it does not remove the bet.

Conversely, taking mutex_ around deliver_finished() (the natural fix for the map race below) guarantees the contention and turns the probabilistic crash into a deterministic one on the first transfer. Verified.

Reproduction

Needs concurrency: one request at a time never opens the window. It reproduces reliably with a few dozen concurrent perform_request() calls in flight (a paged list view plus per-item detail fetches, in our case). Single-request usage looks completely healthy.

Secondary issues found while investigating

Both stem from perform_handle() running on whichever thread awaited it, while yield() runs on the dedicated yield_thread_:

1. condition_awaiters_ is accessed without the lock from the poller. deliver_finished() / abort_cancelled() iterate and .at() the map with no mutex_ held, while pool threads insert/erase under it. abort_all() does take it — the asymmetry is the giveaway. Real UB on the map's tree.

2. curl_multi_add_handle() is called off the polling thread. perform_handle() adds the easy handle from a pool thread while the poller is inside curl_multi_perform() / curl_multi_poll(). A multi handle may only be used from one thread at a time; curl_multi_wakeup() is the sole exception (the comment right above it in perform_handle says as much).

Suggested direction

The three requirements do not hold together while the cv is in the design:

• notify must be co_awaited, so the notifier must be a coroutine;
• yield() must not suspend, or the poll loop migrates onto a pool thread permanently (coro::mutex resumes its waiter on the unlocking thread) and Requestor::shutdown()'s this_thread == yield_thread_ check silently stops meaning anything;
• notify must not run under mutex_, because on_notify() takes it.

Options, roughly in order of effort:

1. Hand the notify to an executor — libcoro's own escape hatch, notify_one(executor) → spawn_detached(notify_one()), where the executor owns the task instead of a temporary. Requires giving CurlMulti an executor.
2. Drop the condition_variable for a one-shot awaiter. Each HandleAwaiterContext has exactly one waiter; a cv is a multi-waiter broadcast primitive doing far more than is needed here. Parking the awaiter ourselves means resuming it ourselves — no foreign task, no foreign mutex. It also frees the mutex to be a plain std::mutex, since nothing would need to await it, and yield() / yield_executor() could then become ordinary synchronous code (they never actually suspend — they block in curl_multi_poll).
3. For add_handle: queue the easy handles in perform_handle() and curl_multi_wakeup(); let the poller drain the queue and add them, so every multi call is on one thread. Note the queued CURL* is borrowed from the curlpp::Easy on the awaiting coroutine's frame, so an entry must be dropped from the queue before its awaiter is woken — waking it unwinds the frame and destroys the handle.

Happy to send a PR if the direction looks right.