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 time.NewTimer() to avoid possible memory leaks #13800

Merged
merged 2 commits into from
May 8, 2024

Conversation

bnovil
Copy link
Contributor

@bnovil bnovil commented Mar 26, 2024

What type of PR is this?

Uncomment one line below and remove others.

Bug fix

What does this PR do? Why is it needed?
Using time.After() in a loop may cause possible memory leaks. As Golang code comment says, Timer is not recovered until fires. In a loop, too many timers may be created and are not garbage collected.

// After waits for the duration to elapse and then sends the current time
// on the returned channel.
// It is equivalent to NewTimer(d).C.
// The underlying Timer is not recovered by the garbage collector
// until the timer fires. If efficiency is a concern, use NewTimer
// instead and call Timer.Stop if the timer is no longer needed.
func After(d Duration) <-chan Time {
	return NewTimer(d).C
}

So this PR is fixing possible memory leaks by using time.NewTimer() and Reset()

Which issues(s) does this PR fix?

Fixes #

Other notes for review

@bnovil bnovil requested a review from a team as a code owner March 26, 2024 08:06
@bnovil bnovil requested review from rauljordan, saolyn and potuz March 26, 2024 08:06
Copy link
Member

@terencechain terencechain left a comment

Choose a reason for hiding this comment

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

Thank you!

@prestonvanloon prestonvanloon enabled auto-merge May 7, 2024 17:17
@prestonvanloon prestonvanloon added this pull request to the merge queue May 8, 2024
Merged via the queue into prysmaticlabs:develop with commit 41edee9 May 8, 2024
17 checks passed
ErnestK pushed a commit to ErnestK/prysm that referenced this pull request May 19, 2024
Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com>
prestonvanloon pushed a commit that referenced this pull request May 31, 2024
Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com>
(cherry picked from commit 41edee9)
nisdas pushed a commit that referenced this pull request Jul 4, 2024
Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com>
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.

3 participants