- Notifications
You must be signed in to change notification settings - Fork913
fix: fix race condition in pubsub startup#17088
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Except for the one question/concern, LGTM 👍🏻
| } | ||
| errCh <- err // won't block because we are buffered. | ||
| sentErrCh = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Do we need to protect this or is sequential execution guaranteed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It executes sequentially, at least by inspection. There is always a chance they could change the implementation and call-back concurrently, but it didn't seem worth while defending for that.
5f3a53f intomainUh oh!
There was an error while loading.Please reload this page.
Merge activity
|
Uh oh!
There was an error while loading.Please reload this page.
fixescoder/internal#525
If the context is canceled, the goroutine that is supposed to read from the
errChcould exit prematurely, leading to a goroutine leak. Refactors this code so it cannot block.