diff --git a/include/kpn/channel.hpp b/include/kpn/channel.hpp index 6ddad69..afaf9fe 100644 --- a/include/kpn/channel.hpp +++ b/include/kpn/channel.hpp @@ -444,6 +444,24 @@ private: // delivered after every value pushed before it. bool take_sentinel(T& out) { if (!has_eof_.load(std::memory_order_acquire)) return false; + // Re-check emptiness *after* observing has_eof_, not before. + // + // Callers check the ring is empty and then call this, but the producer + // can push a value and publish the sentinel in the window between those + // two steps — so the sentinel would be delivered with a real value still + // queued behind it, breaking the "sentinel is strictly last" contract + // that downstream teardown depends on. a0c4bf5 closed the variant where + // the caller's emptiness check used a stale tail_ snapshot; this is the + // one where the check is fresh but simply too early. + // + // Checking here is what makes it sound: the producer publishes the + // sentinel with a release store *after* its ring pushes, so a consumer + // that has observed has_eof_ has also observed every tail_ advance + // before it. If the ring is non-empty now, those values genuinely + // precede the sentinel and must be delivered first. + if (head_.load(std::memory_order_relaxed) + != tail_.load(std::memory_order_acquire)) + return false; out = extract(std::move(eof_value_)); has_eof_.store(false, std::memory_order_release); stats_.record_pop();