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

runtime: race between stack shrinking and channel send/recv leads to bad sudog values [1.14 backport] #40642

gopherbot opened this issue Aug 7, 2020 · 5 comments
CherryPickApproved FrozenDueToAge


Copy link

@gopherbot gopherbot commented Aug 7, 2020

@mknyszek requested issue #40641 to be considered for backport to the next 1.14 minor release.

This should be fixed for Go 1.14 and Go 1.15. It's a bug that was introduced in Go 1.14, and may cause random and unavoidable crashes at any point in time. There may not be enough time to fix this for 1.15 (the failure is very rare, but we've seen it internally), and if not, it should definitely go in a point release.

@gopherbot please open a backport issue for 1.14.

@gopherbot gopherbot added the CherryPickCandidate label Aug 7, 2020
@gopherbot gopherbot added this to the Go1.14.8 milestone Aug 7, 2020
Copy link

@gopherbot gopherbot commented Aug 10, 2020

Change mentions this issue: [release-branch.go1.14] Revert "runtime: make copystack/sudog synchronization more explicit"

@dmitshur dmitshur removed this from the Go1.14.8 milestone Sep 1, 2020
@dmitshur dmitshur added this to the Go1.14.9 milestone Sep 1, 2020
@dmitshur dmitshur removed this from the Go1.14.9 milestone Sep 9, 2020
@dmitshur dmitshur added this to the Go1.14.10 milestone Sep 9, 2020
Copy link

@gopherbot gopherbot commented Sep 21, 2020

Change mentions this issue: [release-branch.go1.14] runtime: disable stack shrinking in activeStackChans race window

Copy link

@mknyszek mknyszek commented Sep 30, 2020

@dmitshur Same as #40643 (comment). Also I forgot to mention there that if you don't have time right now I can go bug someone else. :)

Copy link

@dmitshur dmitshur commented Oct 7, 2020

Approving per discussion in a release meeting. This backport applies to both 1.15 (#40643) and 1.14 (this issue).

@dmitshur dmitshur added CherryPickApproved and removed CherryPickCandidate labels Oct 7, 2020
Copy link

@gopherbot gopherbot commented Oct 7, 2020

Closed by merging 878da0b to release-branch.go1.14.

gopherbot pushed a commit that referenced this issue Oct 7, 2020
…ckChans race window

Currently activeStackChans is set before a goroutine blocks on a channel
operation in an unlockf passed to gopark. The trouble is that the
unlockf is called *after* the G's status is changed, and the G's status
is what is used by a concurrent mark worker (calling suspendG) to
determine that a G has successfully been suspended. In this window
between the status change and unlockf, the mark worker could try to
shrink the G's stack, and in particular observe that activeStackChans is
false. This observation will cause the mark worker to *not* synchronize
with concurrent channel operations when it should, and so updating
pointers in the sudog for the blocked goroutine (which may point to the
goroutine's stack) races with channel operations which may also
manipulate the pointer (read it, dereference it, update it, etc.).

Fix the problem by adding a new atomically-updated flag to the g struct
called parkingOnChan, which is non-zero in the race window above. Then,
in isShrinkStackSafe, check if parkingOnChan is zero. The race is
resolved like so:

* Blocking G sets parkingOnChan, then changes status in gopark.
* Mark worker successfully suspends blocking G.
* If the mark worker observes parkingOnChan is non-zero when checking
  isShrinkStackSafe, then it's not safe to shrink (we're in the race
* If the mark worker observes parkingOnChan as zero, then because
  the mark worker observed the G status change, it can be sure that
  gopark's unlockf completed, and gp.activeStackChans will be correct.

The risk of this change is low, since although it reduces the number of
places that stack shrinking is allowed, the window here is incredibly
small. Essentially, every place that it might crash now is replaced with
no shrink.

This change adds a test, but the race window is so small that it's hard
to trigger without a well-placed sleep in park_m. Also, this change
fixes stackGrowRecursive in proc_test.go to actually allocate a 128-byte
stack frame. It turns out the compiler was destructuring the "pad" field
and only allocating one uint64 on the stack.

For #40641.
Fixes #40642.

Change-Id: I7dfbe7d460f6972b8956116b137bc13bc24464e8
Run-TryBot: Michael Knyszek <>
TryBot-Result: Go Bot <>
Reviewed-by: Michael Pratt <>
Trust: Michael Knyszek <>
(cherry picked from commit eb3c6a9)
Reviewed-by: Austin Clements <>
NexediGitlab pushed a commit to SlapOS/slapos that referenced this issue Oct 27, 2020
Going Go1.14.9 -> Go1.14.10 brings in compiler and runtime fixes
including fix for crash in garbage-collector due to race condition:


Tested on helloworld SR.
@golang golang locked and limited conversation to collaborators Oct 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
CherryPickApproved FrozenDueToAge
None yet

No branches or pull requests

3 participants