Skip to content

Commit 4f34a52

Browse files
committed
runtime: terminate locked OS thread if its goroutine exits
runtime.LockOSThread is sometimes used when the caller intends to put the OS thread into an unusual state. In this case, we never want to return this thread to the runtime thread pool. However, currently exiting the goroutine implicitly unlocks its OS thread. Fix this by terminating the locked OS thread when its goroutine exits, rather than simply returning it to the pool. Fixes #20395. Change-Id: I3dcec63b200957709965f7240dc216fa84b62ad9 Reviewed-on: https://go-review.googlesource.com/46038 Run-TryBot: Austin Clements <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Keith Randall <[email protected]>
1 parent eff2b26 commit 4f34a52

File tree

9 files changed

+303
-2
lines changed

9 files changed

+303
-2
lines changed

src/runtime/crash_cgo_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -443,3 +443,12 @@ func TestCatchPanic(t *testing.T) {
443443
}
444444
}
445445
}
446+
447+
func TestCgoLockOSThreadExit(t *testing.T) {
448+
switch runtime.GOOS {
449+
case "plan9", "windows":
450+
t.Skipf("no pthreads on %s", runtime.GOOS)
451+
}
452+
t.Parallel()
453+
testLockOSThreadExit(t, "testprogcgo")
454+
}

src/runtime/crash_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ type buildexe struct {
4343
err error
4444
}
4545

46-
func runTestProg(t *testing.T, binary, name string) string {
46+
func runTestProg(t *testing.T, binary, name string, env ...string) string {
4747
testenv.MustHaveGoBuild(t)
4848

4949
exe, err := buildTestProg(t, binary)
@@ -52,6 +52,7 @@ func runTestProg(t *testing.T, binary, name string) string {
5252
}
5353

5454
cmd := testenv.CleanCmdEnv(exec.Command(exe, name))
55+
cmd.Env = append(cmd.Env, env...)
5556
var b bytes.Buffer
5657
cmd.Stdout = &b
5758
cmd.Stderr = &b

src/runtime/proc.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2633,6 +2633,7 @@ func goexit0(gp *g) {
26332633
atomic.Xadd(&sched.ngsys, -1)
26342634
}
26352635
gp.m = nil
2636+
locked := gp.lockedm != 0
26362637
gp.lockedm = 0
26372638
_g_.m.lockedg = 0
26382639
gp.paniconfault = false
@@ -2655,6 +2656,15 @@ func goexit0(gp *g) {
26552656
}
26562657
_g_.m.lockedExt = 0
26572658
gfput(_g_.m.p.ptr(), gp)
2659+
if locked {
2660+
// The goroutine may have locked this thread because
2661+
// it put it in an unusual kernel state. Kill it
2662+
// rather than returning it to the thread pool.
2663+
2664+
// Return to mstart, which will release the P and exit
2665+
// the thread.
2666+
gogo(&_g_.m.g0.sched)
2667+
}
26582668
schedule()
26592669
}
26602670

@@ -3419,8 +3429,10 @@ func dolockOSThread() {
34193429
// LockOSThread wires the calling goroutine to its current operating system thread.
34203430
// The calling goroutine will always execute in that thread,
34213431
// and no other goroutine will execute in it,
3422-
// until the calling goroutine exits or has made as many calls to
3432+
// until the calling goroutine has made as many calls to
34233433
// UnlockOSThread as to LockOSThread.
3434+
// If the calling goroutine exits without unlocking the thread,
3435+
// the thread will be terminated.
34243436
func LockOSThread() {
34253437
if atomic.Load(&newmHandoff.haveTemplateThread) == 0 {
34263438
// If we need to start a new thread from the locked

src/runtime/proc_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -746,3 +746,20 @@ func TestLockOSThreadNesting(t *testing.T) {
746746
}
747747
}()
748748
}
749+
750+
func TestLockOSThreadExit(t *testing.T) {
751+
testLockOSThreadExit(t, "testprog")
752+
}
753+
754+
func testLockOSThreadExit(t *testing.T, prog string) {
755+
output := runTestProg(t, prog, "LockOSThreadMain", "GOMAXPROCS=1")
756+
want := "OK\n"
757+
if output != want {
758+
t.Errorf("want %s, got %s\n", want, output)
759+
}
760+
761+
output = runTestProg(t, prog, "LockOSThreadAlt")
762+
if output != want {
763+
t.Errorf("want %s, got %s\n", want, output)
764+
}
765+
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Copyright 2017 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 main
8+
9+
import (
10+
"bytes"
11+
"fmt"
12+
"io/ioutil"
13+
"os"
14+
"syscall"
15+
)
16+
17+
func gettid() int {
18+
return syscall.Gettid()
19+
}
20+
21+
func tidExists(tid int) (exists, supported bool) {
22+
stat, err := ioutil.ReadFile(fmt.Sprintf("/proc/self/task/%d/stat", tid))
23+
if os.IsNotExist(err) {
24+
return false, true
25+
}
26+
// Check if it's a zombie thread.
27+
state := bytes.Fields(stat)[2]
28+
return !(len(state) == 1 && state[0] == 'Z'), true
29+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// Copyright 2017 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 main
8+
9+
func gettid() int {
10+
return 0
11+
}
12+
13+
func tidExists(tid int) (exists, supported bool) {
14+
return false, false
15+
}
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
// Copyright 2017 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+
package main
6+
7+
import (
8+
"os"
9+
"runtime"
10+
"time"
11+
)
12+
13+
var mainTID int
14+
15+
func init() {
16+
registerInit("LockOSThreadMain", func() {
17+
// init is guaranteed to run on the main thread.
18+
mainTID = gettid()
19+
})
20+
register("LockOSThreadMain", LockOSThreadMain)
21+
22+
registerInit("LockOSThreadAlt", func() {
23+
// Lock the OS thread now so main runs on the main thread.
24+
runtime.LockOSThread()
25+
})
26+
register("LockOSThreadAlt", LockOSThreadAlt)
27+
}
28+
29+
func LockOSThreadMain() {
30+
// gettid only works on Linux, so on other platforms this just
31+
// checks that the runtime doesn't do anything terrible.
32+
33+
// This requires GOMAXPROCS=1 from the beginning to reliably
34+
// start a goroutine on the main thread.
35+
if runtime.GOMAXPROCS(-1) != 1 {
36+
println("requires GOMAXPROCS=1")
37+
os.Exit(1)
38+
}
39+
40+
ready := make(chan bool, 1)
41+
go func() {
42+
// Because GOMAXPROCS=1, this *should* be on the main
43+
// thread. Stay there.
44+
runtime.LockOSThread()
45+
if mainTID != 0 && gettid() != mainTID {
46+
println("failed to start goroutine on main thread")
47+
os.Exit(1)
48+
}
49+
// Exit with the thread locked, which should exit the
50+
// main thread.
51+
ready <- true
52+
}()
53+
<-ready
54+
time.Sleep(1 * time.Millisecond)
55+
// Check that this goroutine is still running on a different
56+
// thread.
57+
if mainTID != 0 && gettid() == mainTID {
58+
println("goroutine migrated to locked thread")
59+
os.Exit(1)
60+
}
61+
println("OK")
62+
}
63+
64+
func LockOSThreadAlt() {
65+
// This is running locked to the main OS thread.
66+
67+
var subTID int
68+
ready := make(chan bool, 1)
69+
go func() {
70+
// This goroutine must be running on a new thread.
71+
runtime.LockOSThread()
72+
subTID = gettid()
73+
ready <- true
74+
// Exit with the thread locked.
75+
}()
76+
<-ready
77+
runtime.UnlockOSThread()
78+
for i := 0; i < 100; i++ {
79+
time.Sleep(1 * time.Millisecond)
80+
// Check that this goroutine is running on a different thread.
81+
if subTID != 0 && gettid() == subTID {
82+
println("locked thread reused")
83+
os.Exit(1)
84+
}
85+
exists, supported := tidExists(subTID)
86+
if !supported || !exists {
87+
goto ok
88+
}
89+
}
90+
println("sub thread", subTID, "still running")
91+
return
92+
ok:
93+
println("OK")
94+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Copyright 2017 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+
#include <stdint.h>
8+
9+
uint32_t threadExited;
10+
11+
void setExited(void *x) {
12+
__sync_fetch_and_add(&threadExited, 1);
13+
}
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
// Copyright 2017 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+
import (
10+
"os"
11+
"runtime"
12+
"sync/atomic"
13+
"time"
14+
"unsafe"
15+
)
16+
17+
/*
18+
#include <pthread.h>
19+
#include <stdint.h>
20+
21+
extern uint32_t threadExited;
22+
23+
void setExited(void *x);
24+
*/
25+
import "C"
26+
27+
var mainThread C.pthread_t
28+
29+
func init() {
30+
registerInit("LockOSThreadMain", func() {
31+
// init is guaranteed to run on the main thread.
32+
mainThread = C.pthread_self()
33+
})
34+
register("LockOSThreadMain", LockOSThreadMain)
35+
36+
registerInit("LockOSThreadAlt", func() {
37+
// Lock the OS thread now so main runs on the main thread.
38+
runtime.LockOSThread()
39+
})
40+
register("LockOSThreadAlt", LockOSThreadAlt)
41+
}
42+
43+
func LockOSThreadMain() {
44+
// This requires GOMAXPROCS=1 from the beginning to reliably
45+
// start a goroutine on the main thread.
46+
if runtime.GOMAXPROCS(-1) != 1 {
47+
println("requires GOMAXPROCS=1")
48+
os.Exit(1)
49+
}
50+
51+
ready := make(chan bool, 1)
52+
go func() {
53+
// Because GOMAXPROCS=1, this *should* be on the main
54+
// thread. Stay there.
55+
runtime.LockOSThread()
56+
self := C.pthread_self()
57+
if C.pthread_equal(mainThread, self) == 0 {
58+
println("failed to start goroutine on main thread")
59+
os.Exit(1)
60+
}
61+
// Exit with the thread locked, which should exit the
62+
// main thread.
63+
ready <- true
64+
}()
65+
<-ready
66+
time.Sleep(1 * time.Millisecond)
67+
// Check that this goroutine is still running on a different
68+
// thread.
69+
self := C.pthread_self()
70+
if C.pthread_equal(mainThread, self) != 0 {
71+
println("goroutine migrated to locked thread")
72+
os.Exit(1)
73+
}
74+
println("OK")
75+
}
76+
77+
func LockOSThreadAlt() {
78+
// This is running locked to the main OS thread.
79+
80+
var subThread C.pthread_t
81+
ready := make(chan bool, 1)
82+
C.threadExited = 0
83+
go func() {
84+
// This goroutine must be running on a new thread.
85+
runtime.LockOSThread()
86+
subThread = C.pthread_self()
87+
// Register a pthread destructor so we can tell this
88+
// thread has exited.
89+
var key C.pthread_key_t
90+
C.pthread_key_create(&key, (*[0]byte)(unsafe.Pointer(C.setExited)))
91+
C.pthread_setspecific(key, unsafe.Pointer(new(int)))
92+
ready <- true
93+
// Exit with the thread locked.
94+
}()
95+
<-ready
96+
for i := 0; i < 100; i++ {
97+
time.Sleep(1 * time.Millisecond)
98+
// Check that this goroutine is running on a different thread.
99+
self := C.pthread_self()
100+
if C.pthread_equal(subThread, self) != 0 {
101+
println("locked thread reused")
102+
os.Exit(1)
103+
}
104+
if atomic.LoadUint32((*uint32)(&C.threadExited)) != 0 {
105+
println("OK")
106+
return
107+
}
108+
}
109+
println("sub thread still running")
110+
os.Exit(1)
111+
}

0 commit comments

Comments
 (0)