Skip to content
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 potential goroutine leaks #3170

Merged
merged 1 commit into from
Apr 21, 2022
Merged

Fix potential goroutine leaks #3170

merged 1 commit into from
Apr 21, 2022

Conversation

nodir-t
Copy link

@nodir-t nodir-t commented Apr 13, 2022

Summary

This fixes potential minor goroutine leaks.

Implementation details

Sending to an unbuffered channel without a select and without a guarantee that something
will receive from that channel, is a recipe for a goroutine leak.
Make the channel buffered to avoid the issue.

Also, with a buffered channel, there is no no need to spin up a new goroutine just to send a
dummy object into it. Instead, just send right after channel creation.

Testing

cd agent
go test -tags unit ./acs/...
go test -tags unit ./tcs/...

New tests cover the changes: no

Description for the changelog

Fix potential goroutine leaks

Licensing

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Sending to an unbuffered channel without a select and without a guarantee that something
will receive from that channel, is a recipe for a gourutine leak.
Make the channel buffered to avoid it.

Also, with a buffered channel, no need to spin up a new goroutine
to begin with. Just send right after channel creation.
@nodir-t nodir-t marked this pull request as ready for review April 13, 2022 23:39
@nodir-t nodir-t changed the title Fix two potential goroutine leaks Fix potential goroutine leaks Apr 13, 2022
@sparrc sparrc merged commit a520be4 into aws:dev Apr 21, 2022
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.

5 participants