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

x/time: possible race in rate/Limiter.WaitN leading to Timer being leaked #33868

Closed
toby-jn opened this issue Aug 27, 2019 · 2 comments
Closed

Comments

@toby-jn
Copy link

toby-jn commented Aug 27, 2019

What version of Go are you using (go version)?

$ go version
go version go1.12.9 linux/amd64

github.com/golang/time: 9d24e82272b4f38b78bc8cff74fa936d31ccd8ef

Does this issue reproduce with the latest release?

Yes

What did you do?

Looking at the code for the rate limiter, in WaitN https://github.com/golang/time/blob/9d24e82272b4f38b78bc8cff74fa936d31ccd8ef/rate/rate.go#L252 the return value of t.Stop() is not checked, if the timer expired between the context being cancelled and the defer being executed then this timer will be blocked sending on t.C and I believe will be leaked.

What did you expect to see?

Timer should be drained after calling Stop(), this should be done in the case <-ctx.Done()

What did you see instead?

Result of Stop() not checked and the timer potentially leaked.

@gopherbot gopherbot added this to the Unreleased milestone Aug 27, 2019
@bcmills
Copy link
Contributor

bcmills commented Aug 27, 2019

A time.Timer does not need to be drained to avoid leaks. (The Timer.C channel is buffered, and buffered channels can be GC'd even if there are values in the buffer.)

@bcmills
Copy link
Contributor

bcmills commented Aug 27, 2019

I don't think there is a bug here, but please reopen if you observe a leak empirically.

@bcmills bcmills closed this as completed Aug 27, 2019
@golang golang locked and limited conversation to collaborators Aug 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants