NotificationsYou must be signed in to change notification settings
Fork928
Star10.1k

fix: Race when shutting down and opening WebSockets#576

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

Jump to bottom

Merged

kylecarbs merged 1 commit intomainfromfixwaitrace

Mar 26, 2022

Merged

fix: Race when shutting down and opening WebSockets#576

kylecarbs merged 1 commit intomainfromfixwaitrace

Mar 26, 2022

Conversation

Copy link

Member

kylecarbs commentedMar 26, 2022

Adding to a WaitGroup while calling wait is a race condition. Surrounding
this in a mutex should solve the problem. Since context is used for
cancellation on all sockets, cleanup should occur properly.

See:https://github.com/coder/coder/runs/5701221057?check_suite_focus=true#step:10:98

fix: Race when shutting down and opening WebSockets

f869e5e

Adding to a WaitGroup while calling wait is a race condition. Surroundingthis in a mutex should solve the problem. Since context is used forcancellation on all sockets, cleanup should occur properly.See:https://github.com/coder/coder/runs/5701221057?check_suite_focus=true#step:10:98

kylecarbs requested a review fromcoadler

March 26, 2022 05:12

kylecarbs self-assigned this

Mar 26, 2022

Copy link

codecovbot commentedMar 26, 2022•
edited
Loading

Codecov Report

Merging#576 (f869e5e) intomain (ff2f647) willincrease coverage by0.12%.
The diff coverage is100.00%.

@@            Coverage Diff             @@##             main     #576      +/-   ##==========================================+ Coverage   63.20%   63.32%   +0.12%==========================================  Files         109      195      +86       Lines       10132    11215    +1083       Branches        0       85      +85     ==========================================+ Hits         6404     7102     +698- Misses       2998     3352     +354- Partials      730      761      +31

Flag	Coverage Δ
unittest-go-	`62.16% <100.00%> (+0.10%)`	⬆️
unittest-go-macos-latest	`57.92% <100.00%> (-0.19%)`	⬇️
unittest-go-ubuntu-latest	`61.12% <100.00%> (-0.26%)`	⬇️
unittest-go-windows-2022	`57.11% <100.00%> (?)`
unittest-js	`63.32% <ø> (?)`

Impacted Files	Coverage Δ
coderd/coderd.go	`96.42% <100.00%> (+0.10%)`	⬆️
coderd/provisionerdaemons.go	`59.15% <100.00%> (+0.64%)`	⬆️
coderd/workspaceresources.go	`58.37% <100.00%> (+0.76%)`	⬆️
peer/conn.go	`77.91% <0.00%> (-3.81%)`	⬇️
cli/cliui/job.go	`66.66% <0.00%> (-2.57%)`	⬇️
peer/channel.go	`83.04% <0.00%> (-2.34%)`	⬇️
site/src/components/Icons/CoderIcon.tsx	`100.00% <0.00%> (ø)`
site/src/components/Workspace/constants.ts	`100.00% <0.00%> (ø)`
site/src/util/swr.ts	`0.00% <0.00%> (ø)`
site/src/components/Button/SplitButton.tsx	`89.47% <0.00%> (ø)`
... and91 more

Continue to review full report at Codecov.

Legend -Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered byCodecov. Last updateff2f647...f869e5e. Read thecomment docs.