Skip to content

10157 fix goroutine leak v2 - #10202

Merged
ndeloof merged 5 commits into
docker:v2from
AhmedGrati:10157-fix-goroutine-leak-v2
Jan 26, 2023
Merged

ndeloof merged 5 commits into
docker:v2from
AhmedGrati:10157-fix-goroutine-leak-v2

Conversation

@AhmedGrati

Copy link
Copy Markdown
Contributor

What I did
Fixed the goroutine leak, by removing the infinite goroutine that will send colors to the channel, and then refactored the rainbowColor() function inside the colors.go, to replace the logic of the infinite goroutine.

Related issue

Fixes #10157.

Screenshots of testing
Screen Shot 2023-01-25 at 10 07 22 AM

Screen Shot 2023-01-25 at 10 08 51 AM

@AhmedGrati
AhmedGrati force-pushed the 10157-fix-goroutine-leak-v2 branch from b21c763 to 07aa1cf Compare January 25, 2023 09:17
@laurazard
laurazard requested review from a team, StefanScherer, glours, laurazard, milas, ndeloof, nicksieger and ulyssessouza and removed request for a team January 25, 2023 15:41
Comment thread cmd/formatter/colors.go Resolved
@AhmedGrati
AhmedGrati requested review from ndeloof and removed request for StefanScherer, glours, laurazard, milas, nicksieger and ulyssessouza January 25, 2023 16:34
@codecov

codecov Bot commented Jan 25, 2023 •

Copy link
Copy Markdown

Codecov Report

Base: 73.89% // Head: 73.89% // No change to project coverage πŸ‘

Coverage data is based on head (d9d3bfc) compared to base (aa31387).
Patch has no changes to coverable lines.

❗ Current head d9d3bfc differs from pull request most recent head e6ef041. Consider uploading reports for the commit e6ef041 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##               v2   #10202   +/-   ##
=======================================
  Coverage   73.89%   73.89%           
=======================================
  Files           2        2           
  Lines         272      272           
=======================================
  Hits          201      201           
  Misses         60       60           
  Partials       11       11           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

β˜” View full report at Codecov.
πŸ“’ Do you have feedback about the report comment? Let us know in this issue.

@ndeloof
ndeloof requested review from a team, StefanScherer, glours, laurazard, milas, nicksieger and ulyssessouza and removed request for a team January 26, 2023 07:08

@glours glours left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Signed-off-by: AhmedGrati <ahmedgrati1999@gmail.com>
Signed-off-by: AhmedGrati <ahmedgrati1999@gmail.com>
Signed-off-by: AhmedGrati <ahmedgrati1999@gmail.com>
Signed-off-by: AhmedGrati <ahmedgrati1999@gmail.com>
Signed-off-by: AhmedGrati <ahmedgrati1999@gmail.com>
@glours
glours force-pushed the 10157-fix-goroutine-leak-v2 branch from d9d3bfc to e6ef041 Compare January 26, 2023 10:00
@ndeloof
ndeloof merged commit df70735 into docker:v2 Jan 26, 2023
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.

[BUG] goroutine leak in init() of v2/cmd/formatter ?

5 participants