Skip to content

v0.2.3 — Bus race fix (supersedes v0.2.2)

Choose a tag to compare

@a-radwan-20 a-radwan-20 released this 31 May 19:09
· 47 commits to main since this release

What's in this release

A single-commit patch fix for a data race in internal/agent/bus
that affected v0.2.2. This release supersedes v0.2.2 for production
use
— the wall-clock parallel mission execution shipped in v0.2.2
is unchanged, but the underlying bus subscription cleanup now no
longer races with concurrent in-flight publishes.

If you've already pulled v0.2.2 and you run missions with multiple
workers, upgrade to v0.2.3.

Fixed

Data race in bus.Unsubscribe* vs concurrent in-flight publish

CI's race detector flagged a write/read race on the subscriber
channel between:

  • the defer bus.UnsubscribeReliable(...) cleanup in the supervisor's
    Run (closing the chan as part of shutdown), and
  • the worker's fire-and-forget publishResult goroutine
    (PublishReliable snapshots the subs slice under read-lock, then
    iterates and sends after releasing the lock).

The window between "publisher snapshotted the chan" and "publisher
sent on the chan" is open enough that Unsubscribe could close the
chan in between. Under the race detector this fails the test. In
production it would surface as a panic: send on closed channel on
the publisher goroutine the next time that timing aligned.

The race had been latent on the broadcast Reliable path since v0.2.0.
v0.2.2's competing-consumer dispatch + the worker's async
publishResult goroutine made it reproducible in CI on
internal/agent/missionruntime.

The fix

UnsubscribeReliable and UnsubscribeCompeting now only remove the
subscription from the topic's slice — they no longer close the
channel. The chan is reclaimed by GC once both the subscriber
goroutine and the bus drop their references. Subscribers must exit
on their own context — which is what both the worker and the
supervisor already do (they each have case <-ctx.Done() in their
receive selects).

The case msg, ok := <-ch + if !ok branches in the consumers are
retained as defensive dead code in case external code ever closes
the chan in the future, but they're no longer the primary exit
signal.

Empty topic entries are also pruned from the bus's internal map on
Unsubscribe so the *TopicCount helpers report the actual count.

The legacy best-effort Publish / Unsubscribe API still closes-on-
unsubscribe and uses defer recover() around each send to swallow
the resulting panic. Left intentionally alone — it's a separate
fire-and-forget API with no ack-loop or snapshot semantics, and
changing it would be wider blast radius than the bug warrants.

Test contract update

TestUnsubscribeReliable_RemovesAndCloses renamed to
TestUnsubscribeReliable_RemovesFromTopic. The assertion changed
from "channel closes" to "topic disappears + subsequent publish
returns ErrNoSubscribers" — the API contract that mission consumers
actually depend on.

Verification

  • go test -race -count=20 -run TestRuntime_DoesNotReSpawnRunningMission ./internal/agent/missionruntime/ — 20/20 clean
  • Full repo go test -race -count=1 ./... — 127/127 packages pass
  • CI on the release commit — green on both Lint and Test (race detector)

Compatibility

Source-level: subscribers that explicitly relied on channel-close
as their exit signal (rather than ctx.Done()) need a small update.
In this repo, both the mission worker and supervisor already select
on ctx, so no consumer-side change was needed.

No mission schema, plan, or wire-format change. Upgrading from
v0.2.2 → v0.2.3 is a drop-in replacement.


Full changelog: v0.2.2...v0.2.3