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 goroutine leak in throttler logic. #2739

Merged
merged 1 commit into from Jan 23, 2018

Conversation

timoreimann
Copy link
Contributor

What does this PR do?

Fix a goroutine leak in the throttling logic.

Additional Notes

The variable managing throttling goroutines was only scoped to the local method, effectively disabling goroutine cleanup and producing a leak. We properly hook up the goroutines to live as long as the server does.

This PR partially supersedes #2728.

@timoreimann timoreimann added status/2-needs-review kind/bug/confirmed a confirmed bug (reproducible). labels Jan 22, 2018
@traefiker traefiker added this to the 1.5 milestone Jan 22, 2018
@timoreimann timoreimann force-pushed the fix-throttle-goroutine-leak branch 2 times, most recently from 4e58a0a to f185f2c Compare January 23, 2018 01:01
@ldez ldez added kind/bug/fix a bug fix and removed kind/bug/confirmed a confirmed bug (reproducible). labels Jan 23, 2018
Copy link
Member

@juliens juliens left a comment

Choose a reason for hiding this comment

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

👏 LGTM 👏

Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@mmatur mmatur left a comment

Choose a reason for hiding this comment

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

Nice catch @timoreimann
LGTM 👏

The variable managing throttling goroutines was only scoped to the local
method, effectively disabling goroutine cleanup and producing a leak. We
properly hook up the goroutines to live as long as the server does.
@traefiker traefiker merged commit 4cc17e1 into traefik:v1.5 Jan 23, 2018
@timoreimann timoreimann deleted the fix-throttle-goroutine-leak branch May 8, 2018 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants