-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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 when starting a service while the agent serviceManager
is …
#12302
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.
LGTM! For anyone else wondering why this fixes the problem, I believe it's because the calls to the WaitGroup.Add
all come from Agent.AddService
which acquires this stateLock
. So by moving the lock above serviceManager.Stop
(where WaitGroup.Wait
is called) we know that calls to the two WaitGroup
methods will not race with each other.
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/579623. |
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/579667. |
🍒✅ Cherry pick of commit 4f0a71d onto |
#12302) * fix race when starting a service while the agent `serviceManager` is stopping * add changelog
🍒✅ Cherry pick of commit 4f0a71d onto |
#12302) * fix race when starting a service while the agent `serviceManager` is stopping * add changelog
🍒✅ Cherry pick of commit 4f0a71d onto |
#12302) * fix race when starting a service while the agent `serviceManager` is stopping * add changelog
This fix a data race when we add a service while the service manager is stopping. A waitGroup Add is not safe to be called while waiting on the waitGroup.