balancer/rls: fix goroutine leak in control_channel_test - #8898
Conversation
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8898 +/- ##
==========================================
+ Coverage 81.41% 81.42% +0.01%
==========================================
Files 416 416
Lines 33429 33429
==========================================
+ Hits 27216 27221 +5
+ Misses 4660 4655 -5
Partials 1553 1553 🚀 New features to boost your workflow:
|
|
@eshitachandwani ma'am i didn't change anything |
|
Hey @maishivamhoo123 , the tests are failing on master too, so it is not because of your changes. We will be fixing it soon. |
|
@eshitachandwani can i also work on that issue ma'am? |
|
@eshitachandwani and I have one more question if it is falling here . what it didn't fail during testing in my local machine? |
Hey , thank you for volunteering to help but I am already working on it. |
The command you used will only run tests |
|
Hey @maishivamhoo123 , the tests have been fixed, please merge master to get the tests passing on your PR. |
|
@eshitachandwani @team can you please review the PR? |
|
/gemini review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request correctly fixes a potential goroutine leak in the RLS control channel tests. The issue stemmed from using an unbuffered channel to communicate results from a goroutine back to the main test goroutine, which was also waiting on a timeout. In the case of a timeout, the main goroutine would stop listening on the channel, causing the worker goroutine to block indefinitely when trying to send the result. By changing the channel to be buffered with a size of 1, the worker goroutine can send its result and terminate, even if the receiver is no longer present. The changes are applied consistently where this pattern occurs and are a good fix for this common concurrency issue in tests. The code is ready to be merged.
Description
This PR fixes a goroutine leak in
balancer/rls/control_channel_test.go.The leak occurred because an unbuffered channel was used in a test with
time.After. If the test timed out, the receiver would stop listening, causing the sender (the callback) to block forever. This change buffers the channel (make(chan error, 1)), allowing the goroutine to exit even if the receiver is gone.Related Issue
Fixes #8892
Testing
go test -v ./balancer/rls/...(Passed)./scripts/vet.sh(Passed)go test -race ./balancer/rls/...(Passed)RELEASE NOTES: n/a