-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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 Goroutine leak in v2/command/formatter #10192
Conversation
AFAICT, doing so after all rainbow colors have been used once, next call will block - which is not what we expect, first color should be re-used - so the loop |
Oh makes sense, didn't know about this constraint! I will think of a different solution. |
Signed-off-by: AhmedGrati <ahmedgrati1999@gmail.com>
bb6eea4
to
b670aef
Compare
Done @ndeloof. |
Signed-off-by: AhmedGrati <ahmedgrati1999@gmail.com>
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
Cannot reproduce the failed e2e tests.
The error in the pipeline is really weird.
Any help would be much appreciated. |
I had to revert this PR, has this breaks compose up use of color assignment |
@ndeloof Can you give me a scenario that helps me to reproduce the error, please? |
plain good old |
Thanks @ndeloof, I'll fix it and open a new PR. |
What I did
In this PR, I fixed the Goroutine leak in the
v2/command/formatter
init function. The problem that we had previously, is known as The Forgotten Sender.To fix the issue, we should simply quit the infinite loop in the Goroutine, by sending a signal that indicates that the main thread finished its execution. It's done using
defer()
and thequitChan
channel.A test of the Goroutine leak was added to the unit tests to make sure that the fix is working properly.
Related issue
Fixes #10157.