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.

Sign up for GitHub

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

Jump to bottom

time, runtime: scheduled timer may never fire if GOMAXPROCS is reduced [1.15 backport] #45731

Closed
gopherbot opened this issue Apr 23, 2021 · 5 comments
Closed

time, runtime: scheduled timer may never fire if GOMAXPROCS is reduced [1.15 backport] #45731

gopherbot opened this issue Apr 23, 2021 · 5 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge release-blocker
Milestone
Go1.15.12

Comments

@gopherbot
Copy link
Contributor

@ianlancetaylor requested issue #45716 to be considered for backport to the next 1.15 minor release.

@gopherbot Please open backport to 1.15 branch.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Apr 23, 2021
@gopherbot gopherbot mentioned this issue Apr 23, 2021
Closed
@gopherbot gopherbot added this to the Go1.15.12 milestone Apr 23, 2021
@ianlancetaylor
Copy link
Member

This was already fixed on the 1.16 branch by CL 300610. This issue is to backport that CL to the 1.15 branch as well.

@gopherbot
Copy link
Contributor Author

Change https://golang.org/cl/313130 mentions this issue: [release-branch.go1.15] runtime, time: disable preemption in addtimer

@gopherbot
Copy link
Contributor Author

Change https://golang.org/cl/313129 mentions this issue: [release-branch.go1.15] runtime, time: disable preemption in addtimer

@toothrot toothrot added the CherryPickApproved Used during the release process for point releases label Apr 29, 2021
@toothrot
Copy link
Contributor

Approved. This is a serious issue with no workaround.

@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Apr 29, 2021
@gopherbot
Copy link
Contributor Author

Closed by merging 72ccabc to release-branch.go1.15.

gopherbot pushed a commit that referenced this issue Apr 29, 2021
@prattmic @ianlancetaylor
[release-branch.go1.15] runtime, time: disable preemption in addtimer
72ccabc 
The timerpMask optimization updates a mask of Ps (potentially)
containing timers in pidleget / pidleput. For correctness, it depends on
the assumption that new timers can only be added to a P's own heap.

addtimer violates this assumption if it is preempted after computing pp.
That G may then run on a different P, but adding a timer to the original
P's heap.

Avoid this by disabling preemption while pp is in use.

Other uses of doaddtimer should be OK:

* moveTimers: always moves to the current P's heap
* modtimer, cleantimers, addAdjustedTimers, runtimer: does not add net
  new timers to the heap while locked

For #44868
Fixes #45731

Change-Id: I4a5d080865e854931d0a3a09a51ca36879101d72
Reviewed-on: https://go-review.googlesource.com/c/go/+/300610
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/313129
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
@golang golang locked and limited conversation to collaborators Apr 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge release-blocker
Projects
None yet
Milestone
Go1.15.12
Development

No branches or pull requests
3 participants
@toothrot @ianlancetaylor @gopherbot