cloudflare / cloudflare/cloudflared
Use `time.Timer` instead of `time.After` in either-or cases to avoid memory leak
- Dominant language
- Go
- Stars
- 15.6k
- Forks
- 1.4k
- PR merge metrics
- No merged PRs in 30d
Description
The docs for [time.After](https://golang.org/pkg/time/#After) says
> 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.
So, it is okay to use `After` if it is guaranteed that the case will always be executed (so it will be GC'ed). Apart from unit tests, I found that in [this code](https://github.com/cloudflare/cloudflared/blob/eed7d7bbc90ccaca129bc6b6c3fd18b19e6bf856/h2mux/h2mux.go#L309), the usage of `time.After` could be avoided, as there is a possibility of the [other case](https://github.com/cloudflare/cloudflared/blob/eed7d7bbc90ccaca129bc6b6c3fd18b19e6bf856/h2mux/h2mux.go#L305) to be true. Moreover, this select statement is inside a for loop, which can leak more memory.
If this issue sounds valid, l can raise a PR to replace `time.After` with `time.Timer` along with unit tests :)
Thanks.
Contributor guide
Assessment
This issue has not been assessed yet.