cloudflare / cloudflare/cloudflared

Use `time.Timer` instead of `time.After` in either-or cases to avoid memory leak

Open
#384 1 comment 0 reactions 0 assignees View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.