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: Timer reset broken under heavy use since go1.16 timer optimizations added [1.16 backport] #47332

Closed
gopherbot opened this issue Jul 22, 2021 · 6 comments

Comments

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 22, 2021

@ianlancetaylor requested issue #47329 to be considered for backport to the next 1.16 minor release.

@gopherbot Please open backport to 1.16.

This bug also exists in 1.16. It can cause programs that use Timer.Reset to fail to run a timer when it is ready.

@toothrot
Copy link
Contributor

@toothrot toothrot commented Jul 22, 2021

Approved. This is a serious issue with no workaround.

@gopherbot
Copy link
Author

@gopherbot gopherbot commented Jul 22, 2021

Change https://golang.org/cl/336689 mentions this issue: [release-branch.go1.16] runtime: don't clear timerModifiedEarliest if adjustTimers is 0

@gopherbot
Copy link
Author

@gopherbot gopherbot commented Jul 22, 2021

Closed by merging ed8cbbc to release-branch.go1.16.

@gopherbot gopherbot closed this Jul 22, 2021
gopherbot pushed a commit that referenced this issue Jul 22, 2021
… adjustTimers is 0

This avoids a race when a new timerModifiedEarlier timer is created by
a different goroutine.

For #47329
Fixes #47332

Change-Id: I6f6c87b4a9b5491b201c725c10bc98e23e0ed9d1
Reviewed-on: https://go-review.googlesource.com/c/go/+/336432
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
(cherry picked from commit 798ec73)
Reviewed-on: https://go-review.googlesource.com/c/go/+/336689
@dmitshur
Copy link
Contributor

@dmitshur dmitshur commented Jul 27, 2021

@ianlancetaylor Should the follow up change CL 337309 also be cherry-picked to 1.16?

(I'll reopen this so we don't forget to make that decision before release time.)

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jul 27, 2021

I'm uncertain whether CL 337309 should be cherry picked to the 1.16 branch. The bug fix (CLs 336432, 336689) fixed a real bug that could cause timers to fail to fire. CL 337309 is a performance fix. I'm mildly concerned that CL 337309 will introduce some other unforeseen performance problem. On the 1.16 release branch we needed the bug fix, but I don't know how hard we want to change performance issues on that branch.

@cagedmantis
Copy link
Contributor

@cagedmantis cagedmantis commented Jul 29, 2021

Closed per Ian's comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants