[release-branch.go1.15] runtime: block signals in needm before allocating M
Otherwise, if a signal occurs just after we allocated the M,
we can deadlock if the signal handler needs to allocate an M
itself.
For #42207
Fixes #42636
Change-Id: I76f44547f419e8b1c14cbf49bf602c6e645d8c14
Reviewed-on: https://go-review.googlesource.com/c/go/+/265759
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
(cherry picked from commit 368c40116434532dc0b53b72fa04788ca6742898)
Reviewed-on: https://go-review.googlesource.com/c/go/+/271847
diff --git a/src/runtime/crash_cgo_test.go b/src/runtime/crash_cgo_test.go
index 4872189..57c3639 100644
--- a/src/runtime/crash_cgo_test.go
+++ b/src/runtime/crash_cgo_test.go
@@ -600,3 +600,16 @@
t.Fatalf("want %s, got %s\n", want, output)
}
}
+
+// Issue #42207.
+func TestNeedmDeadlock(t *testing.T) {
+ switch runtime.GOOS {
+ case "plan9", "windows":
+ t.Skipf("no signals on %s", runtime.GOOS)
+ }
+ output := runTestProg(t, "testprogcgo", "NeedmDeadlock")
+ want := "OK\n"
+ if output != want {
+ t.Fatalf("want %s, got %s\n", want, output)
+ }
+}
diff --git a/src/runtime/os_js.go b/src/runtime/os_js.go
index ff0ee3a..94983b3 100644
--- a/src/runtime/os_js.go
+++ b/src/runtime/os_js.go
@@ -59,7 +59,7 @@
}
//go:nosplit
-func msigsave(mp *m) {
+func sigsave(p *sigset) {
}
//go:nosplit
diff --git a/src/runtime/os_plan9.go b/src/runtime/os_plan9.go
index 9e187d2..8f5380d 100644
--- a/src/runtime/os_plan9.go
+++ b/src/runtime/os_plan9.go
@@ -167,7 +167,7 @@
mp.errstr = (*byte)(mallocgc(_ERRMAX, nil, true))
}
-func msigsave(mp *m) {
+func sigsave(p *sigset) {
}
func msigrestore(sigmask sigset) {
diff --git a/src/runtime/os_windows.go b/src/runtime/os_windows.go
index 769197d..81d9ccf 100644
--- a/src/runtime/os_windows.go
+++ b/src/runtime/os_windows.go
@@ -830,7 +830,7 @@
}
//go:nosplit
-func msigsave(mp *m) {
+func sigsave(p *sigset) {
}
//go:nosplit
diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index 2f1272d..7fa19d8 100644
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -569,7 +569,7 @@
typelinksinit() // uses maps, activeModules
itabsinit() // uses activeModules
- msigsave(_g_.m)
+ sigsave(&_g_.m.sigmask)
initSigmask = _g_.m.sigmask
goargs()
@@ -1536,6 +1536,18 @@
exit(1)
}
+ // Save and block signals before getting an M.
+ // The signal handler may call needm itself,
+ // and we must avoid a deadlock. Also, once g is installed,
+ // any incoming signals will try to execute,
+ // but we won't have the sigaltstack settings and other data
+ // set up appropriately until the end of minit, which will
+ // unblock the signals. This is the same dance as when
+ // starting a new m to run Go code via newosproc.
+ var sigmask sigset
+ sigsave(&sigmask)
+ sigblock()
+
// Lock extra list, take head, unlock popped list.
// nilokay=false is safe here because of the invariant above,
// that the extra list always contains or will soon contain
@@ -1553,14 +1565,8 @@
extraMCount--
unlockextra(mp.schedlink.ptr())
- // Save and block signals before installing g.
- // Once g is installed, any incoming signals will try to execute,
- // but we won't have the sigaltstack settings and other data
- // set up appropriately until the end of minit, which will
- // unblock the signals. This is the same dance as when
- // starting a new m to run Go code via newosproc.
- msigsave(mp)
- sigblock()
+ // Store the original signal mask for use by minit.
+ mp.sigmask = sigmask
// Install g (= m->g0) and set the stack bounds
// to match the current stack. We don't actually know
@@ -3417,7 +3423,7 @@
// a signal handler before exec if a signal is sent to the process
// group. See issue #18600.
gp.m.locks++
- msigsave(gp.m)
+ sigsave(&gp.m.sigmask)
sigblock()
// This function is called before fork in syscall package.
diff --git a/src/runtime/signal_unix.go b/src/runtime/signal_unix.go
index 80fd2d6..003c7b0 100644
--- a/src/runtime/signal_unix.go
+++ b/src/runtime/signal_unix.go
@@ -1032,15 +1032,15 @@
return true
}
-// msigsave saves the current thread's signal mask into mp.sigmask.
+// sigsave saves the current thread's signal mask into *p.
// This is used to preserve the non-Go signal mask when a non-Go
// thread calls a Go function.
// This is nosplit and nowritebarrierrec because it is called by needm
// which may be called on a non-Go thread with no g available.
//go:nosplit
//go:nowritebarrierrec
-func msigsave(mp *m) {
- sigprocmask(_SIG_SETMASK, nil, &mp.sigmask)
+func sigsave(p *sigset) {
+ sigprocmask(_SIG_SETMASK, nil, p)
}
// msigrestore sets the current thread's signal mask to sigmask.
@@ -1112,7 +1112,7 @@
// thread's signal mask. When this is called all signals have been
// blocked for the thread. This starts with m.sigmask, which was set
// either from initSigmask for a newly created thread or by calling
-// msigsave if this is a non-Go thread calling a Go function. It
+// sigsave if this is a non-Go thread calling a Go function. It
// removes all essential signals from the mask, thus causing those
// signals to not be blocked. Then it sets the thread's signal mask.
// After this is called the thread can receive signals.
diff --git a/src/runtime/testdata/testprogcgo/needmdeadlock.go b/src/runtime/testdata/testprogcgo/needmdeadlock.go
new file mode 100644
index 0000000..5a9c359
--- /dev/null
+++ b/src/runtime/testdata/testprogcgo/needmdeadlock.go
@@ -0,0 +1,95 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build !plan9,!windows
+
+package main
+
+// This is for issue #42207.
+// During a call to needm we could get a SIGCHLD signal
+// which would itself call needm, causing a deadlock.
+
+/*
+#include <signal.h>
+#include <pthread.h>
+#include <sched.h>
+#include <unistd.h>
+
+extern void GoNeedM();
+
+#define SIGNALERS 10
+
+static void* needmSignalThread(void* p) {
+ pthread_t* pt = (pthread_t*)(p);
+ int i;
+
+ for (i = 0; i < 100; i++) {
+ if (pthread_kill(*pt, SIGCHLD) < 0) {
+ return NULL;
+ }
+ usleep(1);
+ }
+ return NULL;
+}
+
+// We don't need many calls, as the deadlock is only likely
+// to occur the first couple of times that needm is called.
+// After that there will likely be an extra M available.
+#define CALLS 10
+
+static void* needmCallbackThread(void* p) {
+ int i;
+
+ for (i = 0; i < SIGNALERS; i++) {
+ sched_yield(); // Help the signal threads get started.
+ }
+ for (i = 0; i < CALLS; i++) {
+ GoNeedM();
+ }
+ return NULL;
+}
+
+static void runNeedmSignalThread() {
+ int i;
+ pthread_t caller;
+ pthread_t s[SIGNALERS];
+
+ pthread_create(&caller, NULL, needmCallbackThread, NULL);
+ for (i = 0; i < SIGNALERS; i++) {
+ pthread_create(&s[i], NULL, needmSignalThread, &caller);
+ }
+ for (i = 0; i < SIGNALERS; i++) {
+ pthread_join(s[i], NULL);
+ }
+ pthread_join(caller, NULL);
+}
+*/
+import "C"
+
+import (
+ "fmt"
+ "os"
+ "time"
+)
+
+func init() {
+ register("NeedmDeadlock", NeedmDeadlock)
+}
+
+//export GoNeedM
+func GoNeedM() {
+}
+
+func NeedmDeadlock() {
+ // The failure symptom is that the program hangs because of a
+ // deadlock in needm, so set an alarm.
+ go func() {
+ time.Sleep(5 * time.Second)
+ fmt.Println("Hung for 5 seconds")
+ os.Exit(1)
+ }()
+
+ C.runNeedmSignalThread()
+ fmt.Println("OK")
+}
