Skip to content

Commit fd6ba1c

Browse files
AndrewGMorganianlancetaylor
authored andcommitted
os/signal: fix a deadlock with syscall.AllThreadsSyscall() use
The syscall.AllThreadsSyscall() fixup mechanism needs to cooperate with signal handling to ensure a notetsleepg() thread can wake up to run the mDoFixup() function. Fixes #43149 Change-Id: I6651b25bc44a4de47d3fb71d0293d51aef8b79c7 Reviewed-on: https://go-review.googlesource.com/c/go/+/277434 Run-TryBot: Ian Lance Taylor <[email protected]> TryBot-Result: Go Bot <[email protected]> Trust: Austin Clements <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>
1 parent b0b0d98 commit fd6ba1c

File tree

4 files changed

+82
-2
lines changed

4 files changed

+82
-2
lines changed

src/os/signal/signal_linux_test.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// Copyright 2020 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
// +build linux
6+
7+
package signal
8+
9+
import (
10+
"os"
11+
"syscall"
12+
"testing"
13+
"time"
14+
)
15+
16+
const prSetKeepCaps = 8
17+
18+
// This test validates that syscall.AllThreadsSyscall() can reliably
19+
// reach all 'm' (threads) of the nocgo runtime even when one thread
20+
// is blocked waiting to receive signals from the kernel. This monitors
21+
// for a regression vs. the fix for #43149.
22+
func TestAllThreadsSyscallSignals(t *testing.T) {
23+
if _, _, err := syscall.AllThreadsSyscall(syscall.SYS_PRCTL, prSetKeepCaps, 0, 0); err == syscall.ENOTSUP {
24+
t.Skip("AllThreadsSyscall disabled with cgo")
25+
}
26+
27+
sig := make(chan os.Signal, 1)
28+
Notify(sig, os.Interrupt)
29+
30+
for i := 0; i <= 100; i++ {
31+
if _, _, errno := syscall.AllThreadsSyscall(syscall.SYS_PRCTL, prSetKeepCaps, uintptr(i&1), 0); errno != 0 {
32+
t.Fatalf("[%d] failed to set KEEP_CAPS=%d: %v", i, i&1, errno)
33+
}
34+
}
35+
36+
select {
37+
case <-time.After(10 * time.Millisecond):
38+
case <-sig:
39+
t.Fatal("unexpected signal")
40+
}
41+
Stop(sig)
42+
}

src/runtime/proc.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1515,6 +1515,7 @@ func syscall_runtime_doAllThreadsSyscall(fn func(bool) bool) {
15151515
if netpollinited() {
15161516
netpollBreak()
15171517
}
1518+
sigRecvPrepareForFixup()
15181519
_g_ := getg()
15191520
if raceenabled {
15201521
// For m's running without racectx, we loan out the

src/runtime/sigqueue.go

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,16 @@
1212
// sigsend is called by the signal handler to queue a new signal.
1313
// signal_recv is called by the Go program to receive a newly queued signal.
1414
// Synchronization between sigsend and signal_recv is based on the sig.state
15-
// variable. It can be in 3 states: sigIdle, sigReceiving and sigSending.
15+
// variable. It can be in 4 states: sigIdle, sigReceiving, sigSending and sigFixup.
1616
// sigReceiving means that signal_recv is blocked on sig.Note and there are no
1717
// new pending signals.
1818
// sigSending means that sig.mask *may* contain new pending signals,
1919
// signal_recv can't be blocked in this state.
2020
// sigIdle means that there are no new pending signals and signal_recv is not blocked.
21+
// sigFixup is a transient state that can only exist as a short
22+
// transition from sigReceiving and then on to sigIdle: it is
23+
// used to ensure the AllThreadsSyscall()'s mDoFixup() operation
24+
// occurs on the sleeping m, waiting to receive a signal.
2125
// Transitions between states are done atomically with CAS.
2226
// When signal_recv is unblocked, it resets sig.Note and rechecks sig.mask.
2327
// If several sigsends and signal_recv execute concurrently, it can lead to
@@ -59,6 +63,7 @@ const (
5963
sigIdle = iota
6064
sigReceiving
6165
sigSending
66+
sigFixup
6267
)
6368

6469
// sigsend delivers a signal from sighandler to the internal signal delivery queue.
@@ -112,13 +117,29 @@ Send:
112117
notewakeup(&sig.note)
113118
break Send
114119
}
120+
case sigFixup:
121+
// nothing to do - we need to wait for sigIdle.
122+
osyield()
115123
}
116124
}
117125

118126
atomic.Xadd(&sig.delivering, -1)
119127
return true
120128
}
121129

130+
// sigRecvPrepareForFixup is used to temporarily wake up the
131+
// signal_recv() running thread while it is blocked waiting for the
132+
// arrival of a signal. If it causes the thread to wake up, the
133+
// sig.state travels through this sequence: sigReceiving -> sigFixup
134+
// -> sigIdle -> sigReceiving and resumes. (This is only called while
135+
// GC is disabled.)
136+
//go:nosplit
137+
func sigRecvPrepareForFixup() {
138+
if atomic.Cas(&sig.state, sigReceiving, sigFixup) {
139+
notewakeup(&sig.note)
140+
}
141+
}
142+
122143
// Called to receive the next queued signal.
123144
// Must only be called from a single goroutine at a time.
124145
//go:linkname signal_recv os/signal.signal_recv
@@ -146,7 +167,16 @@ func signal_recv() uint32 {
146167
}
147168
notetsleepg(&sig.note, -1)
148169
noteclear(&sig.note)
149-
break Receive
170+
if !atomic.Cas(&sig.state, sigFixup, sigIdle) {
171+
break Receive
172+
}
173+
// Getting here, the code will
174+
// loop around again to sleep
175+
// in state sigReceiving. This
176+
// path is taken when
177+
// sigRecvPrepareForFixup()
178+
// has been called by another
179+
// thread.
150180
}
151181
case sigSending:
152182
if atomic.Cas(&sig.state, sigSending, sigIdle) {

src/runtime/sigqueue_plan9.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,13 @@ func sendNote(s *byte) bool {
9292
return true
9393
}
9494

95+
// sigRecvPrepareForFixup is a no-op on plan9. (This would only be
96+
// called while GC is disabled.)
97+
//
98+
//go:nosplit
99+
func sigRecvPrepareForFixup() {
100+
}
101+
95102
// Called to receive the next queued signal.
96103
// Must only be called from a single goroutine at a time.
97104
//go:linkname signal_recv os/signal.signal_recv

0 commit comments

Comments
 (0)