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

Comments

@pjebs
Copy link
Contributor

@pjebs 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 this to the Go1.12 milestone Jul 4, 2018
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jul 4, 2018

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

@conspicuousClockwork conspicuousClockwork commented Jul 9, 2018

@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

@ianlancetaylor ianlancetaylor commented Jul 9, 2018

@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

@conspicuousClockwork conspicuousClockwork commented Jul 9, 2018

@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

@ianlancetaylor ianlancetaylor commented Jul 9, 2018

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

@conspicuousClockwork
Copy link
Contributor

@conspicuousClockwork conspicuousClockwork commented Jul 9, 2018

Yup, I'll go ahead and open that.

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 9, 2018

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

@gopherbot gopherbot closed this in a07ee41 Jul 11, 2018
@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.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.