Skip to content

Commit efd204c

Browse files
[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 <[email protected]> Run-TryBot: Ian Lance Taylor <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Bryan C. Mills <[email protected]> (cherry picked from commit 368c401) Reviewed-on: https://go-review.googlesource.com/c/go/+/271847
1 parent 730d5f4 commit efd204c

File tree

7 files changed

+131
-17
lines changed

7 files changed

+131
-17
lines changed

src/runtime/crash_cgo_test.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -600,3 +600,16 @@ func TestEINTR(t *testing.T) {
600600
t.Fatalf("want %s, got %s\n", want, output)
601601
}
602602
}
603+
604+
// Issue #42207.
605+
func TestNeedmDeadlock(t *testing.T) {
606+
switch runtime.GOOS {
607+
case "plan9", "windows":
608+
t.Skipf("no signals on %s", runtime.GOOS)
609+
}
610+
output := runTestProg(t, "testprogcgo", "NeedmDeadlock")
611+
want := "OK\n"
612+
if output != want {
613+
t.Fatalf("want %s, got %s\n", want, output)
614+
}
615+
}

src/runtime/os_js.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ func mpreinit(mp *m) {
5959
}
6060

6161
//go:nosplit
62-
func msigsave(mp *m) {
62+
func sigsave(p *sigset) {
6363
}
6464

6565
//go:nosplit

src/runtime/os_plan9.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ func mpreinit(mp *m) {
167167
mp.errstr = (*byte)(mallocgc(_ERRMAX, nil, true))
168168
}
169169

170-
func msigsave(mp *m) {
170+
func sigsave(p *sigset) {
171171
}
172172

173173
func msigrestore(sigmask sigset) {

src/runtime/os_windows.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -830,7 +830,7 @@ func mpreinit(mp *m) {
830830
}
831831

832832
//go:nosplit
833-
func msigsave(mp *m) {
833+
func sigsave(p *sigset) {
834834
}
835835

836836
//go:nosplit

src/runtime/proc.go

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -569,7 +569,7 @@ func schedinit() {
569569
typelinksinit() // uses maps, activeModules
570570
itabsinit() // uses activeModules
571571

572-
msigsave(_g_.m)
572+
sigsave(&_g_.m.sigmask)
573573
initSigmask = _g_.m.sigmask
574574

575575
goargs()
@@ -1536,6 +1536,18 @@ func needm(x byte) {
15361536
exit(1)
15371537
}
15381538

1539+
// Save and block signals before getting an M.
1540+
// The signal handler may call needm itself,
1541+
// and we must avoid a deadlock. Also, once g is installed,
1542+
// any incoming signals will try to execute,
1543+
// but we won't have the sigaltstack settings and other data
1544+
// set up appropriately until the end of minit, which will
1545+
// unblock the signals. This is the same dance as when
1546+
// starting a new m to run Go code via newosproc.
1547+
var sigmask sigset
1548+
sigsave(&sigmask)
1549+
sigblock()
1550+
15391551
// Lock extra list, take head, unlock popped list.
15401552
// nilokay=false is safe here because of the invariant above,
15411553
// that the extra list always contains or will soon contain
@@ -1553,14 +1565,8 @@ func needm(x byte) {
15531565
extraMCount--
15541566
unlockextra(mp.schedlink.ptr())
15551567

1556-
// Save and block signals before installing g.
1557-
// Once g is installed, any incoming signals will try to execute,
1558-
// but we won't have the sigaltstack settings and other data
1559-
// set up appropriately until the end of minit, which will
1560-
// unblock the signals. This is the same dance as when
1561-
// starting a new m to run Go code via newosproc.
1562-
msigsave(mp)
1563-
sigblock()
1568+
// Store the original signal mask for use by minit.
1569+
mp.sigmask = sigmask
15641570

15651571
// Install g (= m->g0) and set the stack bounds
15661572
// to match the current stack. We don't actually know
@@ -3417,7 +3423,7 @@ func beforefork() {
34173423
// a signal handler before exec if a signal is sent to the process
34183424
// group. See issue #18600.
34193425
gp.m.locks++
3420-
msigsave(gp.m)
3426+
sigsave(&gp.m.sigmask)
34213427
sigblock()
34223428

34233429
// This function is called before fork in syscall package.

src/runtime/signal_unix.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1032,15 +1032,15 @@ func sigfwdgo(sig uint32, info *siginfo, ctx unsafe.Pointer) bool {
10321032
return true
10331033
}
10341034

1035-
// msigsave saves the current thread's signal mask into mp.sigmask.
1035+
// sigsave saves the current thread's signal mask into *p.
10361036
// This is used to preserve the non-Go signal mask when a non-Go
10371037
// thread calls a Go function.
10381038
// This is nosplit and nowritebarrierrec because it is called by needm
10391039
// which may be called on a non-Go thread with no g available.
10401040
//go:nosplit
10411041
//go:nowritebarrierrec
1042-
func msigsave(mp *m) {
1043-
sigprocmask(_SIG_SETMASK, nil, &mp.sigmask)
1042+
func sigsave(p *sigset) {
1043+
sigprocmask(_SIG_SETMASK, nil, p)
10441044
}
10451045

10461046
// msigrestore sets the current thread's signal mask to sigmask.
@@ -1112,7 +1112,7 @@ func minitSignalStack() {
11121112
// thread's signal mask. When this is called all signals have been
11131113
// blocked for the thread. This starts with m.sigmask, which was set
11141114
// either from initSigmask for a newly created thread or by calling
1115-
// msigsave if this is a non-Go thread calling a Go function. It
1115+
// sigsave if this is a non-Go thread calling a Go function. It
11161116
// removes all essential signals from the mask, thus causing those
11171117
// signals to not be blocked. Then it sets the thread's signal mask.
11181118
// After this is called the thread can receive signals.
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
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 !plan9,!windows
6+
7+
package main
8+
9+
// This is for issue #42207.
10+
// During a call to needm we could get a SIGCHLD signal
11+
// which would itself call needm, causing a deadlock.
12+
13+
/*
14+
#include <signal.h>
15+
#include <pthread.h>
16+
#include <sched.h>
17+
#include <unistd.h>
18+
19+
extern void GoNeedM();
20+
21+
#define SIGNALERS 10
22+
23+
static void* needmSignalThread(void* p) {
24+
pthread_t* pt = (pthread_t*)(p);
25+
int i;
26+
27+
for (i = 0; i < 100; i++) {
28+
if (pthread_kill(*pt, SIGCHLD) < 0) {
29+
return NULL;
30+
}
31+
usleep(1);
32+
}
33+
return NULL;
34+
}
35+
36+
// We don't need many calls, as the deadlock is only likely
37+
// to occur the first couple of times that needm is called.
38+
// After that there will likely be an extra M available.
39+
#define CALLS 10
40+
41+
static void* needmCallbackThread(void* p) {
42+
int i;
43+
44+
for (i = 0; i < SIGNALERS; i++) {
45+
sched_yield(); // Help the signal threads get started.
46+
}
47+
for (i = 0; i < CALLS; i++) {
48+
GoNeedM();
49+
}
50+
return NULL;
51+
}
52+
53+
static void runNeedmSignalThread() {
54+
int i;
55+
pthread_t caller;
56+
pthread_t s[SIGNALERS];
57+
58+
pthread_create(&caller, NULL, needmCallbackThread, NULL);
59+
for (i = 0; i < SIGNALERS; i++) {
60+
pthread_create(&s[i], NULL, needmSignalThread, &caller);
61+
}
62+
for (i = 0; i < SIGNALERS; i++) {
63+
pthread_join(s[i], NULL);
64+
}
65+
pthread_join(caller, NULL);
66+
}
67+
*/
68+
import "C"
69+
70+
import (
71+
"fmt"
72+
"os"
73+
"time"
74+
)
75+
76+
func init() {
77+
register("NeedmDeadlock", NeedmDeadlock)
78+
}
79+
80+
//export GoNeedM
81+
func GoNeedM() {
82+
}
83+
84+
func NeedmDeadlock() {
85+
// The failure symptom is that the program hangs because of a
86+
// deadlock in needm, so set an alarm.
87+
go func() {
88+
time.Sleep(5 * time.Second)
89+
fmt.Println("Hung for 5 seconds")
90+
os.Exit(1)
91+
}()
92+
93+
C.runNeedmSignalThread()
94+
fmt.Println("OK")
95+
}

0 commit comments

Comments
 (0)