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

time: clarify Ticker documentation #26220

Closed
pjebs opened this issue Jul 4, 2018 · 7 comments
Closed

time: clarify Ticker documentation #26220

pjebs opened this issue Jul 4, 2018 · 7 comments
Labels
Documentation FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@pjebs
Copy link
Contributor

pjebs commented Jul 4, 2018

The documentation for Stop() states:

Stop turns off a ticker. After Stop, no more ticks will be sent. Stop does not close the channel, to prevent a read from the channel succeeding incorrectly.

Can the documentation clarify what "to prevent a read from the channel succeeding incorrectly."means.

Should we close the channel ourself if we want to help the garbage collector?

@ianlancetaylor ianlancetaylor changed the title time.Ticker documentation clarification time: clarify Ticker documentation Jul 4, 2018
@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label Jul 4, 2018
@ianlancetaylor ianlancetaylor added this to the Go1.12 milestone Jul 4, 2018
@ianlancetaylor
Copy link
Contributor

Closing channels has no affect on the garbage collector, which doesn't care whether the channel is closed or not.

What the doc means is that if Stop closes the channel, then any read from the channel will complete, returning the zero value. If there is some goroutine sitting in a select statement reading from the ticker, that case will become ready to be chosen. That may cause the goroutine to take an unexpected action, as though a tick occurred, although it did not.

@conspicuousClockwork
Copy link
Contributor

@ianlancetaylor how does this sound?

Stop turns off a ticker. After Stop, no more ticks will be sent. Stop does not close the channel, in order to prevent potential side effects, such as the channel succeeding incorrectly.

@ianlancetaylor
Copy link
Contributor

@conspicuousClockwork Thanks. You need to say a read from the channel, as channels by themselves do not succeed or fail. And with that change it's not clear to me that your text is much of an improvement. If we want to say anything at all here, perhaps it should be along the lines of "in case another goroutine is concurrently reading from the channel, so that the read does not succeed incorrectly."

@conspicuousClockwork
Copy link
Contributor

@ianlancetaylor Whoops, I must have erased part of it when I was thinking about how the text could be adjusted. I was trying to make as few additions as I could to clarify/phrase it in a different way but your suggestion seems better.

Stop turns off a ticker. After Stop, no more ticks will be sent. Stop does not close the channel; this is in order to prevent potential side effects, such as a goroutine concurrently reading from the channel and succeeding incorrectly.

How is that?

@ianlancetaylor
Copy link
Contributor

Seems OK to me. Perhaps "the read succeeding incorrectly." Want to send a pull request?

@conspicuousClockwork
Copy link
Contributor

Yup, I'll go ahead and open that.

@gopherbot
Copy link

Change https://golang.org/cl/122715 mentions this issue: time/tick: add clarification to Stop() documentation

@golang golang locked and limited conversation to collaborators Jul 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants