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

runtime: fatal error: checkdead: runnable g [1.14 backport] #40398

Closed
gopherbot opened this issue Jul 24, 2020 · 3 comments
Closed

runtime: fatal error: checkdead: runnable g [1.14 backport] #40398

gopherbot opened this issue Jul 24, 2020 · 3 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone
Go1.14.9

Comments

@gopherbot
Copy link
Contributor

@prattmic requested issue #40368 to be considered for backport to the next 1.14 minor release.

@gopherbot backport to 1.14 please. This crash could affect any program running with GOMAXPROCS=1.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Jul 24, 2020
@gopherbot gopherbot mentioned this issue Jul 24, 2020
Closed
@gopherbot gopherbot added this to the Go1.14.7 milestone Jul 24, 2020
@gopherbot
Copy link
Contributor Author

Change https://golang.org/cl/245297 mentions this issue: [release-branch.go1.14] runtime: ensure startm new M is consistently visible to checkdead

@dmitshur dmitshur mentioned this issue Jul 30, 2020
Closed
@toothrot toothrot added the CherryPickApproved Used during the release process for point releases label Aug 4, 2020
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Aug 4, 2020
@toothrot
Copy link
Contributor

toothrot commented Aug 4, 2020

Approving. This is a serious problem with no workaround.

@toothrot toothrot modified the milestones: Go1.14.7, Go1.14.8 Aug 6, 2020
@gopherbot
Copy link
Contributor Author

Closed by merging 17fd967 to release-branch.go1.14.

gopherbot pushed a commit that referenced this issue Aug 22, 2020
@prattmic @dmitshur
[release-branch.go1.14] runtime: ensure startm new M is consistently …
17fd967 
…visible to checkdead

If no M is available, startm first grabs an idle P, then drops
sched.lock and calls newm to start a new M to run than P.

Unfortunately, that leaves a window in which a G (e.g., returning from a
syscall) may find no idle P, add to the global runq, and then in stopm
discover that there are no running M's, a condition that should be
impossible with runnable G's.

To avoid this condition, we pre-allocate the new M ID in startm before
dropping sched.lock. This ensures that checkdead will see the M as
running, and since that new M must eventually run the scheduler, it will
handle any pending work as necessary.

Outside of startm, most other calls to newm/allocm don't have a P at
all. The only exception is startTheWorldWithSema, which always has an M
if there is 1 P (i.e., the currently running M), and if there is >1 P
the findrunnable spinning dance ensures the problem never occurs.

This has been tested with strategically placed sleeps in the runtime to
help induce the correct race ordering, but the timing on this is too
narrow for a test that can be checked in.

For #40368
Fixes #40398

Change-Id: If5e0293a430cc85154b7ed55bc6dadf9b340abe2
Reviewed-on: https://go-review.googlesource.com/c/go/+/245018
Run-TryBot: Michael Pratt <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Austin Clements <[email protected]>
(cherry picked from commit 85afa2e)
Reviewed-on: https://go-review.googlesource.com/c/go/+/245297
@dmitshur dmitshur modified the milestones: Go1.14.8, Go1.14.9 Sep 1, 2020
@golang golang locked and limited conversation to collaborators Sep 1, 2021
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
Projects
None yet
Milestone
Go1.14.9
Development

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