Go: proposal: Go 2: time: deprecate time.Tick(d Duration)

[metala]

Proposal Summary

Deprecate time.Tick(d) in favour of time.NewTicker(d). It's a very special case, where the ticker never stops and the channel cannot be recovered by the garbage collector. The name time.Tick() does not reflect it and if you really want that same functionality, it's just time.NewTicker(d).C, with the exception when d <= 0.

Would you consider yourself a novice, intermediate, or experienced Go programmer?

Intermediate

What other languages do you have experience with?

Comfortable with C, Python, JavaScript, PHP, C#, Lua.

Would this change make Go easier or harder to learn, and why?

Easier, by applying the principle of least astonishment. We can see that this function is already abused in some public repositories.

Has this idea, or one like it, been proposed before?

To my knowledge, no.

Who does this proposal help, and why?

The newcomers to Go as it will decrease ambiguity. We can see that there were issues in the past (#11662, #17757) and you can find public repositories (in GitHub, for instance), where this function is used improperly.

What is the proposed change?

Mark the function time.Tick() as deprecated in the standard library documentation and remove it in a later version. It won't introduce changes to the language specification.

Is this change backward compatible?

It is breaking the Go 1 compatibility guarantee. Thus, this proposal is labeled Go2.

Show example code before and after the change.

Before

for t := range time.Tick(d) {
  // loop body
}

After

for t := range time.NewTicker(d).C {
  // loop body
}

What is the cost of this proposal?

It probably breaks more than a few packages and binaries, which use that function. No compile time or runtime cost.

I've based the proposal on the language change template, but I have removed the last few points I found to be irrelevant.

[metala]

@gopherbot, add labels Go2

[deanveloper]

I personally believe that time.Tick is fine, especially because it is clear (at least in my eyes) that the ticker never stops. The documentation also clearly points out the leak.

However I can understand it being deprecated, especially since there is a concise alternative.

[alanfo]

Well, despite its shortcomings which are clearly described in the documentation, time.Tick has always been intended as a convenience function for time.NewTicker as both have been in the standard library from the start.

That being so I can't really see the point in deprecating it now as it will mean that a vast amount of code which currently works fine will eventually need to be changed.

[bcmills]

To be clear: deprecating the function does not necessarily require that we remove it entirely. (We could leave it deprecated forever, or only disallow references to it from within modules that specify go 1.N.) See #30639.

[alanfo]

Thanks for that link as I'd always thought that deprecating something meant that you shouldn't use it as it's eventually going to be removed.

Having said that, there's stuff in Java which has been deprecated for years and there's still no sign of it being removed.

Whatever deprecation means, I'm still not really in favor of the proposal as time.Tick is occasionally useful.

[metala]

I will elaborate on the reason I opened that issue. I've used go doc time and picked time.Tick() for a client connection handler that should free the timer after the client has been disconnected and I assumed that it will free it automatically. It was a rookie mistake and from now on I use go doc -all <pkg> | less, instead. If the function name was TickForever(), I might have given it more thought.

I was convinced that I am not the only one who made that mistake or just read convenience wrapper and did not read further. So I have searched GitHub and googled for code that uses time.Tick() and sure enough I found some cases.

Here is a list of examples, which I believe are problematic, since there is an option to stop or break the loop, yet the ticker cannot be freed:

• https://github.com/golang/sys/blob/master/windows/svc/example/service.go#L23
• https://github.com/Netflix/hal-9001/blob/master/hal/periodic.go#L134
• https://github.com/pydio/cells/blob/master/common/service/utils.go#L28
• https://github.com/izikaj/iziproxy/blob/master/server/tcp_handle_signals.go#L49
• https://github.com/gocraft/health/blob/master/aggregator.go#L22
• https://github.com/GoogleCloudPlatform/compute-image-tools/blob/master/daisy/step_create_instances.go#L54
• https://github.com/rs/xstats/blob/master/statsd/statsd.go#L79
• https://github.com/flynn/flynn/blob/master/controller/scheduler/discoverd.go#L32

Cases that are not problematic, since the executable will exit and the runtime will stop the timer eventually, but are still misused and leak resources:

• https://github.com/cockroachdb/cockroach/blob/master/pkg/storage/client_raft_test.go#L1390 (do their leaktest even work?)
• https://github.com/solo-io/go-utils/blob/master/testutils/helper/curl.go#L61
• https://github.com/odeke-em/youtube-popular-bot/blob/master/cmd/youtube-popular-bot/main.go#L110
• https://github.com/influxdata/influx-stress/blob/master/cmd/insert.go#L119

I've found those examples using Google search string: intext:"time.Tick(" ext:go site:github.com

[bcmills]

This is closely related to #19702 (see https://github.com/golang/go/issues/19702#issuecomment-290855582 and its reply), in that it addresses an empirical mismatch between users' assumptions about garbage collection for time.Ticker and the actual GC behavior as implemented.

[ianlancetaylor]

Perhaps not exactly what this issue is about, but I want to mention https://golang.org/cl/214999, which is an attempt to garbage collect unreferenced tickers. It may be possible to get that CL into 1.15.

[metala]

That changeset is nice, but it just removes the symptoms and it also bloats the runtime, just like if we add garbage collection of goroutines.

Now that I am thinking that it's not just an issue about time.Tick(), it may be better if we use static analysis to check if the goroutine can exit without stopping a Ticker.

PS. I am not sure if my proposal is still relevant.

[ianlancetaylor]

Bloating the runtime is a valid complaint, but you write "removes the symptoms" as though that is a bad thing. It seems to me that removing the symptoms is equivalent to removing the problem.

[metala]

Ok, you are right.

I guess we can close the proposal, since the patchset solves the problem.

[ianlancetaylor]

Proposal withdrawn.