Skip to content

Fix streaming Reader race condition #55

New issue

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

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

Already on GitHub? Sign in 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
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