Skip to content

fix(service): Start's teardown does not close the inbound channel (follow-up of #4) - #7

Merged
inkly merged 1 commit into
ReCasaOS:mainfrom
ppenguin:fix/bus-teardown
Oct 24, 2026
Merged

inkly merged 1 commit into
ReCasaOS:mainfrom
ppenguin:fix/bus-teardown

Conversation

@ppenguin

Copy link
Copy Markdown
Contributor

Follow-up of #4 (review).

  • teardown: inbound channel set to nil, not closed: a Publish/Trigger holding it could pick the send over ctx.Done and panic (send on closed channel); senders end on ctx.Done
  • action subscribers: subscriberBuffer (64), as event subscribers (was 1)
  • TestEventTypeService: receive with a 10 s timeout
  • test TestSendersRacingTheEndOfStartDoNotPanic: 64 senders racing cancel(), 300 rounds → panics on main, passes here; go test -race ./... green

- a Publish/Trigger holding the channel could pick the send over ctx.Done and panic
  (send on closed channel); the field is set to nil, senders end on ctx.Done
- action subscribers get subscriberBuffer (64) like event subscribers (was 1)
- TestEventTypeService: the receive has a 10 s timeout
- test: 64 senders racing cancel(), 300 rounds: panics before, passes after

@inkly inkly left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this fixes it: the new test panics on main and passes here, and senders already return through ctx.Done. The subscriber buffer and the receive timeout are welcome too. Optional nit: TestEventsPublishedTogetherAllArrive could use Ready() instead of polling s.stop. Approving. We'll squash-merge with a plain commit message.

@inkly
inkly merged commit 680fbff into ReCasaOS:main Oct 24, 2026
1 check passed
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.

2 participants