fix: never drop a wake — a node must not sleep with one outstanding
🚦 CI / changes (push) Successful in 14s
🚦 CI / docker (push) Has been skipped
🚦 CI / test (push) Failing after 4m21s
🚦 CI / tsan (push) Failing after 3m20s
🚦 CI / docs (push) Has been skipped

28e0667 stopped nodes blocking a worker on a full output, but replaced an
intermittent hang with a quieter one: the pipeline still wedged about 2
runs in 30, now with every worker idle in pthread_cond_wait rather than
asleep in a push. Nothing was blocked; nothing had been woken.

try_submit discarded any wake arriving while queued_ was up:

    if (queued_.compare_exchange_strong(expected, true, ...))
        scheduler_->submit(...);
    // else: silently gone

Wakes are edge-triggered — a channel fires its space callback once, on the
transition — so a dropped one never returns. A node could park a value,
release its worker, and sleep forever holding exactly the output its
consumer was waiting for, while its producer parked on an input channel
that would never drain.

try_submit now records the drop in wake_pending_, and release_and_recheck()
consumes it at every site that releases a node, giving one invariant: a
node never sleeps with a wake outstanding. This subsumes the two ad-hoc
re-checks added for the parked-retry and normal push paths, which only
moved the stall (1211 items to 6472) because each new early return was a
fresh chance to drop a wake. self_stop keeps a plain store — honouring a
pending wake there would resubmit a dead node.

Adds "a saturated chain never stalls", which reproduces this in about a
second where the pipeline needed ~30 runs. It asserts *progress does not
freeze* rather than a completion total: capacity-1 channels are slow, and
slow must never be reported as wedged. Verified in both directions — it
stalls after 1211 items on 28e0667 and passes here.

The existing chain test could not catch it: 40 items drain before any
strand occurs, and its producer emits forever, so fresh input keeps
re-triggering on_input_ready() and flushing the stranded value.

Tests: 122/122.
This commit is contained in:
2026-08-02 18:33:17 +02:00
parent 28e06675f5
commit 9c5ce5f34a
2 changed files with 180 additions and 7 deletions
+94 -7
View File
@@ -265,6 +265,8 @@ private:
disable_inputs(std::make_index_sequence<input_count>{});
disable_outputs(std::make_index_sequence<output_count>{});
stats_.exec_start_us.store(0, std::memory_order_relaxed);
// Plain store, not release_and_recheck(): this node is stopping, and
// honouring a pending wake here would resubmit a dead node.
queued_.store(false, std::memory_order_release);
stop_flag_.store(true, std::memory_order_relaxed);
}
@@ -353,10 +355,32 @@ private:
: 0.5f), ...);
}
/// Submit unless already queued. A wake that arrives while this node is
/// queued or running is *recorded*, never dropped.
///
/// Wakes are edge-triggered: a channel fires its space callback on the
/// transition, once. If that lands while queued_ is up, the CAS below fails
/// and — before wake_pending_ — the wake was gone. A node could then park a
/// value, release its worker, and sleep forever holding output its consumer
/// was waiting for, with every worker idle in cond_wait and nothing left to
/// re-trigger it. Recording the drop turns the signal level-triggered: the
/// invariant is that a node never sleeps with a wake outstanding, enforced
/// by release_and_recheck() at every point that releases the node.
void try_submit(float priority) {
bool expected = false;
if (queued_.compare_exchange_strong(expected, true, std::memory_order_acq_rel))
scheduler_->submit([this] { fire_once(); }, priority);
else
wake_pending_.store(true, std::memory_order_release);
}
/// Clear queued_, then honour any wake that was dropped while it was up.
/// Every path that finishes or parks a firing must release the node through
/// here rather than storing queued_ directly.
void release_and_recheck(float priority = 0.5f) {
queued_.store(false, std::memory_order_release);
if (wake_pending_.exchange(false, std::memory_order_acq_rel))
try_submit(priority);
}
// ── Execution ─────────────────────────────────────────────────────────────
@@ -379,7 +403,7 @@ private:
if constexpr (!std::is_void_v<return_raw>) {
if (pending_) {
push_outputs(std::move(*pending_), std::make_index_sequence<output_count>{});
queued_.store(false, std::memory_order_release);
release_and_recheck();
if (pending_) {
// Close the lost-wakeup race: a space_callback that fired
// between the failed push and clearing queued_ was
@@ -408,7 +432,7 @@ private:
if constexpr (!std::is_void_v<return_raw>) {
if (pending_) {
push_outputs(std::move(*pending_), std::make_index_sequence<output_count>{});
queued_.store(false, std::memory_order_release);
release_and_recheck();
if (pending_) {
// Close the lost-wakeup race: a space_callback that fired
// between the failed push and clearing queued_ was
@@ -438,7 +462,7 @@ private:
// on_input_ready() resubmits when data actually lands.
if constexpr (input_count > 0) {
if (count_ready(std::make_index_sequence<input_count>{}) != input_count) {
queued_.store(false, std::memory_order_release);
release_and_recheck();
on_input_ready(); // data may have arrived while we checked
return;
}
@@ -483,10 +507,27 @@ private:
}
stats_.exec_start_us.store(0, std::memory_order_relaxed);
queued_.store(false, std::memory_order_release);
release_and_recheck();
if (stop_flag_.load(std::memory_order_relaxed)) return;
// Parked by the push above. Same situation as the retry path at the top
// of fire_once — and the same lost-wakeup race, which that path closes
// and this one did not. A space callback that fired while queued_ was
// still up got swallowed by try_submit's CAS, and the resubmit below
// cannot cover it: this firing consumed its input, so inputs are empty
// and on_input_ready() will not resubmit. The node would then hold its
// value forever while its consumer waits for exactly that value and its
// producer parks on an input channel that never drains. Re-check now
// that the flag is down.
if constexpr (!std::is_void_v<return_raw>) {
if (pending_) {
if (outputs_have_space(std::make_index_sequence<output_count>{}))
try_submit(0.5f);
return; // parked
}
}
// Source nodes always resubmit; others resubmit only if inputs are ready.
if constexpr (input_count == 0) {
try_submit(0.5f);
@@ -574,6 +615,8 @@ private:
output_channels_t output_channels_{};
std::atomic<bool> stop_flag_{true};
std::atomic<bool> queued_{false};
/// A wake that arrived while queued_ was up. See try_submit.
std::atomic<bool> wake_pending_{false};
/// The hidden one-slot output buffer (see push_outputs). Holding the value
/// here is what lets a node stop running without dropping it or occupying a
@@ -753,6 +796,8 @@ private:
disable_inputs(std::make_index_sequence<input_count>{});
disable_outputs(std::make_index_sequence<output_count>{});
stats_.exec_start_us.store(0, std::memory_order_relaxed);
// Plain store, not release_and_recheck(): this node is stopping, and
// honouring a pending wake here would resubmit a dead node.
queued_.store(false, std::memory_order_release);
stop_flag_.store(true, std::memory_order_relaxed);
}
@@ -834,10 +879,32 @@ private:
: 0.5f), ...);
}
/// Submit unless already queued. A wake that arrives while this node is
/// queued or running is *recorded*, never dropped.
///
/// Wakes are edge-triggered: a channel fires its space callback on the
/// transition, once. If that lands while queued_ is up, the CAS below fails
/// and — before wake_pending_ — the wake was gone. A node could then park a
/// value, release its worker, and sleep forever holding output its consumer
/// was waiting for, with every worker idle in cond_wait and nothing left to
/// re-trigger it. Recording the drop turns the signal level-triggered: the
/// invariant is that a node never sleeps with a wake outstanding, enforced
/// by release_and_recheck() at every point that releases the node.
void try_submit(float priority) {
bool expected = false;
if (queued_.compare_exchange_strong(expected, true, std::memory_order_acq_rel))
scheduler_->submit([this] { fire_once(); }, priority);
else
wake_pending_.store(true, std::memory_order_release);
}
/// Clear queued_, then honour any wake that was dropped while it was up.
/// Every path that finishes or parks a firing must release the node through
/// here rather than storing queued_ directly.
void release_and_recheck(float priority = 0.5f) {
queued_.store(false, std::memory_order_release);
if (wake_pending_.exchange(false, std::memory_order_acq_rel))
try_submit(priority);
}
void fire_once() {
@@ -856,7 +923,7 @@ private:
if constexpr (!std::is_void_v<return_raw>) {
if (pending_) {
push_outputs(std::move(*pending_), std::make_index_sequence<output_count>{});
queued_.store(false, std::memory_order_release);
release_and_recheck();
if (pending_) {
// Close the lost-wakeup race: a space_callback that fired
// between the failed push and clearing queued_ was
@@ -884,7 +951,7 @@ private:
// into pop_inputs on an empty channel.
if constexpr (input_count > 0) {
if (count_ready(std::make_index_sequence<input_count>{}) != input_count) {
queued_.store(false, std::memory_order_release);
release_and_recheck();
on_input_ready();
return;
}
@@ -926,8 +993,26 @@ private:
}
stats_.exec_start_us.store(0, std::memory_order_relaxed);
queued_.store(false, std::memory_order_release);
release_and_recheck();
if (stop_flag_.load(std::memory_order_relaxed)) return;
// Parked by the push above. Same situation as the retry path at the top
// of fire_once — and the same lost-wakeup race, which that path closes
// and this one did not. A space callback that fired while queued_ was
// still up got swallowed by try_submit's CAS, and the resubmit below
// cannot cover it: this firing consumed its input, so inputs are empty
// and on_input_ready() will not resubmit. The node would then hold its
// value forever while its consumer waits for exactly that value and its
// producer parks on an input channel that never drains. Re-check now
// that the flag is down.
if constexpr (!std::is_void_v<return_raw>) {
if (pending_) {
if (outputs_have_space(std::make_index_sequence<output_count>{}))
try_submit(0.5f);
return; // parked
}
}
if constexpr (input_count == 0) try_submit(0.5f);
else on_input_ready();
}
@@ -986,6 +1071,8 @@ private:
output_channels_t output_channels_{};
std::atomic<bool> stop_flag_{true};
std::atomic<bool> queued_{false};
/// A wake that arrived while queued_ was up. See try_submit.
std::atomic<bool> wake_pending_{false};
/// The hidden one-slot output buffer (see push_outputs). Holding the value
/// here is what lets a node stop running without dropping it or occupying a