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

use a timer instead of 'After' to avoid leaking resources #31

Merged
merged 1 commit into from
Jul 20, 2016

Conversation

whyrusleeping
Copy link
Contributor

Calling time.After creates a runtime timer that gets scheduled into the runtime. Doing this frequently will bog the scheduler down and have weird effects on the program. Using a timer instead will let us stop the timer and free up those resources when we exit the function

@whyrusleeping
Copy link
Contributor Author

cc @slackpad

@sean-
Copy link

sean- commented Jun 8, 2016

What does a time.Timer leak? time.Ticker can leak a resource. ?

@whyrusleeping
Copy link
Contributor Author

@sean- the time.Timer doesnt leak anything so long as you either Stop it, or it fires. time.Ticker will continue running until you call Stop on it. The issue here is that if we have a large number of rapid calls to read and a timeout is set, there are going to be many inflight time.After timers running since they don't get explicitly cancelled.

@armon armon merged commit d2be601 into hashicorp:master Jul 20, 2016
@armon
Copy link
Member

armon commented Jul 20, 2016

LGTM

lattwood added a commit to lattwood/yamux that referenced this pull request Jun 14, 2024
In hashicorp#31, the Read path was changed to move away from
time.After. This change was not reflected in the Write path, and this
commit rectifies that.
@lattwood
Copy link
Contributor

The write side of this functionality has been added to #127, as well as a bug fix around ensuring stopped Timers have their channels drained when appropriate.

lattwood added a commit to lattwood/yamux that referenced this pull request Jun 17, 2024
In hashicorp#31, the Read path was changed to move away from
time.After. This change was not reflected in the Write path, and this
commit rectifies that.
lattwood added a commit to lattwood/yamux that referenced this pull request Aug 21, 2024
In hashicorp#31, the Read path was changed to move away from
time.After. This change was not reflected in the Write path, and this
commit rectifies that.
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.

4 participants