Skip to content

Fix streaming Reader race condition #55

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Apr 12, 2025

Conversation

douglaswth
Copy link
Member

  • There was a race condition between creating a streaming.Reader with NewReader and calling its Subscribe where events could be consumed before there is any channel to send them to.
  • To fix this, the default behavior is now to only start the reader's read goroutine when Subscribe is called after it has inserted a channel that will receive events.
  • Alternatively, there is now an option WithReaderStartExplicitly which will only start the read goroutine when the new Start is called allowing multiple calls to Subscribe before receiving events.

- There was a race condition between creating a `streaming.Reader` with
  `NewReader` and calling its `Subscribe` where events could be consumed
  before there is any channel to send them to.
- To fix this, the default behavior is now to only start the reader's
  `read` goroutine when `Subscribe` is called after it has inserted a
  channel that will receive events.
- Alternatively, there is now an option `WithReaderStartExplicitly`
  which will only start the `read` goroutine when the new `Start` is
  called allowing multiple calls to `Subscribe` before receiving events.
@douglaswth douglaswth requested a review from raphael April 10, 2025 20:20
@raphael
Copy link
Member

raphael commented Apr 11, 2025

Thank you, that makes sense. I wonder if we need the explicit start option - can we do without?

@douglaswth
Copy link
Member Author

Thank you, that makes sense. I wonder if we need the explicit start option - can we do without?

I think we can leave it out, I'll remove it.

@douglaswth douglaswth marked this pull request as ready for review April 11, 2025 21:50
@raphael
Copy link
Member

raphael commented Apr 12, 2025

Thank you!

@raphael raphael merged commit 5ed8559 into main Apr 12, 2025
4 checks passed
@raphael raphael deleted the fix-streaming-reader-race-condition branch April 12, 2025 18:37
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants