runtime: protect against external code calling ExitProcess

On Windows, we implement asynchronous preemption using SuspendThread
to suspend other threads in our process. However, SuspendThread is
itself actually asynchronous (it enqueues a kernel "asynchronous
procedure call" and returns). Unfortunately, Windows' ExitProcess API
kills all threads except the calling one and then runs APCs. As a
result, if SuspendThread and ExitProcess are called simultaneously,
the exiting thread can be suspended and the suspending thread can be
exited, leaving behind a ghost process consisting of a single thread
that's suspended.

We've already protected against the runtime's own calls to
ExitProcess, but if Go code calls external code, there's nothing
stopping that code from calling ExitProcess. For example, in #35775,
our own call to racefini leads to C code calling ExitProcess and
occasionally causing a deadlock.

This CL fixes this by introducing synchronization between calling
external code on Windows and preemption. It adds an atomic field to
the M that participates in a simple CAS-based synchronization protocol
to prevent suspending a thread running external code. We use this to
protect cgocall (which is used for both cgo calls and system calls on
Windows) and racefini.

Tested by running the flag package's TestParse test compiled in race
mode in a loop. Before this change, this would reliably deadlock after
a few minutes.

Fixes #35775.
Updates #10958, #24543.

Change-Id: I50d847abcdc2688b4f71eee6a75eca0f2fee892c
Reviewed-on: https://go-review.googlesource.com/c/go/+/213837
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: David Chase <drchase@google.com>

1 files changed, 19 insertions, 0 deletions

diff --git a/src/runtime/cgocall.go b/src/runtime/cgocall.go
index 5f8ff8139a..a4e64b00cc 100644
--- a/src/runtime/cgocall.go
+++ b/src/runtime/cgocall.go
@@ -90,6 +90,11 @@ import (
 type cgoCallers [32]uintptr
 
 // Call from Go to C.
+//
+// This must be nosplit because it's used for syscalls on some
+// platforms. Syscalls may have untyped arguments on the stack, so
+// it's not safe to grow or scan the stack.
+//
 //go:nosplit
 func cgocall(fn, arg unsafe.Pointer) int32 {
 	if !iscgo && GOOS != "solaris" && GOOS != "illumos" && GOOS != "windows" {
@@ -127,6 +132,13 @@ func cgocall(fn, arg unsafe.Pointer) int32 {
 	// saved by entersyscall here.
 	entersyscall()
 
+	// Tell asynchronous preemption that we're entering external
+	// code. We do this after entersyscall because this may block
+	// and cause an async preemption to fail, but at this point a
+	// sync preemption will succeed (though this is not a matter
+	// of correctness).
+	osPreemptExtEnter(mp)
+
 	mp.incgo = true
 	errno := asmcgocall(fn, arg)
 
@@ -135,6 +147,8 @@ func cgocall(fn, arg unsafe.Pointer) int32 {
 	mp.incgo = false
 	mp.ncgo--
 
+	osPreemptExtExit(mp)
+
 	exitsyscall()
 
 	// Note that raceacquire must be called only after exitsyscall has
@@ -188,12 +202,16 @@ func cgocallbackg(ctxt uintptr) {
 	exitsyscall() // coming out of cgo call
 	gp.m.incgo = false
 
+	osPreemptExtExit(gp.m)
+
 	cgocallbackg1(ctxt)
 
 	// At this point unlockOSThread has been called.
 	// The following code must not change to a different m.
 	// This is enforced by checking incgo in the schedule function.
 
+	osPreemptExtEnter(gp.m)
+
 	gp.m.incgo = true
 	// going back to cgo call
 	reentersyscall(savedpc, uintptr(savedsp))
@@ -352,6 +370,7 @@ func unwindm(restore *bool) {
 		if mp.ncgo > 0 {
 			mp.incgo = false
 			mp.ncgo--
+			osPreemptExtExit(mp)
 		}
 
 		releasem(mp)