Skip to content
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

fix: race condition (closes #63) #64

Merged
merged 1 commit into from
Jan 30, 2024
Merged

fix: race condition (closes #63) #64

merged 1 commit into from
Jan 30, 2024

Conversation

Wessie
Copy link
Contributor

@Wessie Wessie commented Jan 24, 2024

Logic race condition, if the decode goroutine can run before the caller can, the channel returned might not have a receiver yet and then the select/default construct will drop the event and close the channel.

The caller then ends up with a zero-value decodedEvent where decodedEvent.err and decodedEvent.event are both nil which passes the err check at line 434 and might enter line 440 where decodedEvent.event is then used as-is with no additional check for it being nil.

Fixed this scenario by giving the returned channel a buffer slot so that decode can always send its event.

Logic race condition, if the decode goroutine can run before the caller
can the channel returned might not have a receiver yet and then the
select/default construct will drop the event and close the channel.

The caller then ends up with a zero-value decodedEvent where
decodedEvent.err and decodedEvent.event are both nil which passes the
err check at line 434 and might enter line 440 where decodedEvent.event
is then used as-is with no additional check for it being nil.
@yunginnanet
Copy link

nailed it

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f477179) 55.99% compared to head (819351e) 56.25%.
Report is 3 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #64      +/-   ##
==========================================
+ Coverage   55.99%   56.25%   +0.26%     
==========================================
  Files          13       13              
  Lines        3488     3484       -4     
==========================================
+ Hits         1953     1960       +7     
+ Misses       1409     1401       -8     
+ Partials      126      123       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lrstanley lrstanley changed the title Fix #63 fix: race condition (closes #63) Jan 30, 2024
@lrstanley lrstanley merged commit 6cf311a into lrstanley:master Jan 30, 2024
11 checks 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.

4 participants