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 <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Austin Clements <austin@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

3 files changed, 40 insertions, 2 deletions

diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index 592d621241..ca78587aad 100644
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -1515,6 +1515,7 @@ func syscall_runtime_doAllThreadsSyscall(fn func(bool) bool) {
 	if netpollinited() {
 		netpollBreak()
 	}
+	sigRecvPrepareForFixup()
 	_g_ := getg()
 	if raceenabled {
 		// For m's running without racectx, we loan out the
diff --git a/src/runtime/sigqueue.go b/src/runtime/sigqueue.go
index 0605f5da80..28b9e26d0f 100644
--- a/src/runtime/sigqueue.go
+++ b/src/runtime/sigqueue.go
@@ -12,12 +12,16 @@
 // sigsend is called by the signal handler to queue a new signal.
 // signal_recv is called by the Go program to receive a newly queued signal.
 // Synchronization between sigsend and signal_recv is based on the sig.state
-// variable. It can be in 3 states: sigIdle, sigReceiving and sigSending.
+// variable. It can be in 4 states: sigIdle, sigReceiving, sigSending and sigFixup.
 // sigReceiving means that signal_recv is blocked on sig.Note and there are no
 // new pending signals.
 // sigSending means that sig.mask *may* contain new pending signals,
 // signal_recv can't be blocked in this state.
 // sigIdle means that there are no new pending signals and signal_recv is not blocked.
+// sigFixup is a transient state that can only exist as a short
+// transition from sigReceiving and then on to sigIdle: it is
+// used to ensure the AllThreadsSyscall()'s mDoFixup() operation
+// occurs on the sleeping m, waiting to receive a signal.
 // Transitions between states are done atomically with CAS.
 // When signal_recv is unblocked, it resets sig.Note and rechecks sig.mask.
 // If several sigsends and signal_recv execute concurrently, it can lead to
@@ -59,6 +63,7 @@ const (
 	sigIdle = iota
 	sigReceiving
 	sigSending
+	sigFixup
 )
 
 // sigsend delivers a signal from sighandler to the internal signal delivery queue.
@@ -112,6 +117,9 @@ Send:
 				notewakeup(&sig.note)
 				break Send
 			}
+		case sigFixup:
+			// nothing to do - we need to wait for sigIdle.
+			osyield()
 		}
 	}
 
@@ -119,6 +127,19 @@ Send:
 	return true
 }
 
+// sigRecvPrepareForFixup is used to temporarily wake up the
+// signal_recv() running thread while it is blocked waiting for the
+// arrival of a signal. If it causes the thread to wake up, the
+// sig.state travels through this sequence: sigReceiving -> sigFixup
+// -> sigIdle -> sigReceiving and resumes. (This is only called while
+// GC is disabled.)
+//go:nosplit
+func sigRecvPrepareForFixup() {
+	if atomic.Cas(&sig.state, sigReceiving, sigFixup) {
+		notewakeup(&sig.note)
+	}
+}
+
 // Called to receive the next queued signal.
 // Must only be called from a single goroutine at a time.
 //go:linkname signal_recv os/signal.signal_recv
@@ -146,7 +167,16 @@ func signal_recv() uint32 {
 					}
 					notetsleepg(&sig.note, -1)
 					noteclear(&sig.note)
-					break Receive
+					if !atomic.Cas(&sig.state, sigFixup, sigIdle) {
+						break Receive
+					}
+					// Getting here, the code will
+					// loop around again to sleep
+					// in state sigReceiving. This
+					// path is taken when
+					// sigRecvPrepareForFixup()
+					// has been called by another
+					// thread.
 				}
 			case sigSending:
 				if atomic.Cas(&sig.state, sigSending, sigIdle) {
diff --git a/src/runtime/sigqueue_plan9.go b/src/runtime/sigqueue_plan9.go
index d5fe8f8b35..aebd2060e7 100644
--- a/src/runtime/sigqueue_plan9.go
+++ b/src/runtime/sigqueue_plan9.go
@@ -92,6 +92,13 @@ func sendNote(s *byte) bool {
 	return true
 }
 
+// sigRecvPrepareForFixup is a no-op on plan9. (This would only be
+// called while GC is disabled.)
+//
+//go:nosplit
+func sigRecvPrepareForFixup() {
+}
+
 // Called to receive the next queued signal.
 // Must only be called from a single goroutine at a time.
 //go:linkname signal_recv os/signal.signal_recv