Skip to content

Commit

Permalink
runtime: remove write barrier in WaitForSigusr1
Browse files Browse the repository at this point in the history
WaitForSigusr1 registers a callback to be called on SIGUSR1 directly
from the runtime signal handler. Currently, this callback has a write
barrier in it, which can crash with a nil P if the GC is active and
the signal arrives on an M that doesn't have a P.

Fix this by recording the ID of the M that receives the signal instead
of the M itself, since that's all we needed anyway. To make sure there
are no other problems, this also lifts the callback into a package
function and marks it "go:nowritebarrierrec".

Fixes #35248.

Updates #35276, since in principle a write barrier at exactly the
wrong time while entering the scheduler could cause issues, though I
suspect that bug is unrelated.

Change-Id: I47b4bc73782efbb613785a93e381d8aaf6850826
Reviewed-on: https://go-review.googlesource.com/c/go/+/204620
Run-TryBot: Austin Clements <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>
Reviewed-by: Bryan C. Mills <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
  • Loading branch information
aclements authored and ianlancetaylor committed Nov 6, 2019
1 parent f891b7c commit b824048
Showing 1 changed file with 19 additions and 9 deletions.
28 changes: 19 additions & 9 deletions src/runtime/export_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ type M = m

var waitForSigusr1 struct {
park note
mp *m
mID int64
}

// WaitForSigusr1 blocks until a SIGUSR1 is received. It calls ready
Expand All @@ -38,24 +38,34 @@ func WaitForSigusr1(ready func(mp *M), timeoutNS int64) (int64, int64) {
unblocksig(_SIGUSR1)

mp := getg().m
testSigusr1 = func(gp *g) bool {
waitForSigusr1.mp = gp.m
notewakeup(&waitForSigusr1.park)
return true
}
testSigusr1 = waitForSigusr1Callback
ready(mp)
ok := notetsleepg(&waitForSigusr1.park, timeoutNS)
noteclear(&waitForSigusr1.park)
gotM := waitForSigusr1.mp
waitForSigusr1.mp = nil
gotM := waitForSigusr1.mID
testSigusr1 = nil

unlockOSThread()

if !ok {
return -1, -1
}
return mp.id, gotM.id
return mp.id, gotM
}

// waitForSigusr1Callback is called from the signal handler during
// WaitForSigusr1. It must not have write barriers because there may
// not be a P.
//
//go:nowritebarrierrec
func waitForSigusr1Callback(gp *g) bool {
if gp == nil || gp.m == nil {
waitForSigusr1.mID = -1
} else {
waitForSigusr1.mID = gp.m.id
}
notewakeup(&waitForSigusr1.park)
return true
}

// SendSigusr1 sends SIGUSR1 to mp.
Expand Down

0 comments on commit b824048

Please sign in to comment.