Skip to content

🎣 Unify close/cancel/value precedence across all packages#10

Merged
amorey merged 1 commit into
mainfrom
fix/context-close-precedence
Jul 21, 2026
Merged

🎣 Unify close/cancel/value precedence across all packages#10
amorey merged 1 commit into
mainfrom
fix/context-close-precedence

Conversation

@amorey

@amorey amorey commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Summary

The context-aware methods disagreed with each other about what wins when a
close, a cancellation, and a ready value are all in play. SendContext in
chancore tested ChClosed above the ctx check but Dead below it, so the
same logical condition — this sender is finished — reported ErrClosed or
ctx.Err() depending on which arm happened to observe it. RecvContext
preferred a ready value over a cancelled context in broadcast and watch,
which meant a receiver looping against a fast enough sender took the value every
iteration and never noticed its own shutdown signal. Nothing pinned any of this,
so each package had drifted on its own.

This settles one ordering — closed > cancelled > value — and applies it
everywhere. An entry-time termination reports ErrClosed even for an
already-cancelled ctx, because that is the durable answer and a retry with a
fresh context would only return it again. A cancelled ctx on a live handle
reports ctx.Err() even when a value is ready, which is what keeps cancellation
observable under load. No path consumes-and-discards: a value is either
delivered to the caller or left in the queue/ring for another receiver.

Fixing the ordering surfaced two things worth their own attention. broadcast's
cancelled path had grown its own copy of the terminal decision and, with it, a
bare return that skipped unregisterLocked — a receiver abandoned on
ctx.Err() held its ring slot for the hub's lifetime. And the existing tests
could not actually cover the parked arms: a started channel signalled just
before a blocking call proves the goroutine was scheduled, not that it reached
the select, so the trigger raced the call's own fast path and the assertions
passed while covering the opposite of what they claimed.

Key Changes

  • Uniform precedence in SendContext (closed > cancelled) and
    RecvContext (closed > cancelled > value) across all seven packages.
    chancore.BufferedSend gains a closed() helper so ChClosed and Dead are
    always tested together and in one order, and rechecks both plus ctx after
    taking SendLock, since the readiness gate and the lock can each block for an
    unbounded time.
  • broadcast.RecvContext unregisters on the cancelled path. The whole
    precedence now runs as one ordered pass under mu, so the terminal exit and
    its tear-down obligation cannot drift apart again.
  • mpmc.Recv delegates to RecvContext with a never-cancelled context
    rather than duplicating the body — one place for precedence to change.
    Measured at well under 1ns against the hand-inlined version.
  • conformance_test.go (new, root): a table driving all seven
    architectures through the same ordering assertions via the module-wide
    Sender/Receiver interfaces, so a future package cannot quietly disagree.
  • internal/parked (new): sleep-free waiting on the runtime's own view of a
    goroutine, so a test can prove the goroutine under test actually parked in the
    arm it claims to exercise. Identity-based rather than count-based — a
    goroutine left parked in the same frame by an earlier test would otherwise
    satisfy the wait on its own.
  • chancore.BufferedRecv removed. The receiver needs a per-handle close
    arm, so mpmc's receive loops live in mpmc. Dead and Ready are now
    required and typed *CloseOnce; a send-side with no readiness condition
    passes an already-latched one rather than nil.
  • Docs: README.md gains the precedence rules and both close-race
    boundaries (a send racing the last receiver's close may deposit a value
    nothing drains — the same guarantee a plain buffered channel gives);
    CLAUDE.md updated for the chancore reshape.

Checklist

  • Add the correct emoji to the PR title
  • Related issue linked above, if any
  • Commit messages use conventional commit format
  • Changes are minimal and focused

Every package now resolves RecvContext as closed > cancelled > value and
SendContext as closed > cancelled, so an entry-time termination reports
ErrClosed even for an already-cancelled ctx and a cancelled ctx is
observable even when a value is ready. No path consumes-and-discards a
value: it is delivered or left in the queue/ring for another receiver.

Adds a root-level conformance table exercising the ordering across all
seven architectures, plus internal/parked, a sleep-free helper that waits
on the runtime's own view of a goroutine so tests provably reach the
parked arm they claim to cover. chancore drops BufferedRecv (mpmc's recv
loops need a per-handle close arm) and makes Dead/Ready required.

Signed-off-by: Andres Morey <root@andresmorey.com>
@amorey
amorey merged commit ff73ea8 into main Jul 21, 2026
22 checks passed
@amorey
amorey deleted the fix/context-close-precedence branch July 21, 2026 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant