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): fixing race condition found with test harness #107

Merged
merged 3 commits into from
Nov 9, 2023
Merged

fix(race): fixing race condition found with test harness #107

merged 3 commits into from
Nov 9, 2023

Conversation

guergabo
Copy link
Contributor

@guergabo guergabo commented Nov 9, 2023

Changes

  • Add a buffered channel of size 1 to get rid of race condition.
  • Update the callback to just write to the channel since a write to a closed channel already causes a panic.

Context
I made the assumption the sendOrPanic only cares that the api goroutine didn't die and is available to complete the request. Since we defer the close() in the api goroutine, it means that if the kernel can write to the channel (without panicing), the api layer goroutine is available and has not exited or paniced. so should be same behavior as before minus race condition.

@guergabo guergabo requested a review from dfarr November 9, 2023 14:31
@guergabo guergabo marked this pull request as draft November 9, 2023 14:38
Copy link

codecov bot commented Nov 9, 2023

Codecov Report

Merging #107 (41048d7) into main (9b14fce) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #107      +/-   ##
==========================================
- Coverage   67.18%   67.16%   -0.03%     
==========================================
  Files          57       57              
  Lines        5949     5944       -5     
==========================================
- Hits         3997     3992       -5     
  Misses       1667     1667              
  Partials      285      285              
Files Coverage Δ
internal/app/subsystems/api/service/service.go 87.43% <100.00%> (-0.30%) ⬇️

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@guergabo guergabo marked this pull request as ready for review November 9, 2023 14:45
@guergabo guergabo requested a review from dfarr November 9, 2023 17:55
@guergabo guergabo merged commit 96131fb into resonatehq:main Nov 9, 2023
4 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.

2 participants