From: Austin Clements <austin@google.com>
Date: Fri, 17 Aug 2018 19:42:19 +0000 (-0400)
Subject: runtime: fix race between unminit and Windows profile loop
X-Git-Tag: go1.12beta1~913
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=c216c3aa9fe34cd81e9d4bcc28c64257064eddc9;p=gostls13.git

runtime: fix race between unminit and Windows profile loop

Currently, the Windows profile loop isn't robust against racing with
unminit. For example,

T1 is running profileloop1, T2 is another thread
T1: thread := atomic.Loaduintptr(&T2.thread)
T2: calls unminit, which does CloseHandle(T2.thread)
T1: attempts to suspends T2

In this case the SuspendThread will fail, but currently we ignore this
failure and forge ahead, which will cause further failures and
probably bad profile data.

Handle this race by defending against SuspendThread failing. If
SuspendThread succeeds, then we know the thread is no longer going
anywhere.

Change-Id: I4726553239b17f05ca07a0cf7df49631e0cb550d
Reviewed-on: https://go-review.googlesource.com/c/129685
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
---

diff --git a/src/runtime/os_windows.go b/src/runtime/os_windows.go
index 409d537839..03dd95bf17 100644
--- a/src/runtime/os_windows.go
+++ b/src/runtime/os_windows.go
@@ -858,14 +858,14 @@ func profileloop()
 
 var profiletimer uintptr
 
-func profilem(mp *m) {
+func profilem(mp *m, thread uintptr) {
 	var r *context
 	rbuf := make([]byte, unsafe.Sizeof(*r)+15)
 
 	// align Context to 16 bytes
 	r = (*context)(unsafe.Pointer((uintptr(unsafe.Pointer(&rbuf[15]))) &^ 15))
 	r.contextflags = _CONTEXT_CONTROL
-	stdcall2(_GetThreadContext, mp.thread, uintptr(unsafe.Pointer(r)))
+	stdcall2(_GetThreadContext, thread, uintptr(unsafe.Pointer(r)))
 
 	var gp *g
 	switch GOARCH {
@@ -906,9 +906,16 @@ func profileloop1(param uintptr) uint32 {
 			if thread == 0 || mp.profilehz == 0 || mp.blocked {
 				continue
 			}
-			stdcall1(_SuspendThread, thread)
+			// mp may exit between the load above and the
+			// SuspendThread, so be careful.
+			if int32(stdcall1(_SuspendThread, thread)) == -1 {
+				// The thread no longer exists.
+				continue
+			}
 			if mp.profilehz != 0 && !mp.blocked {
-				profilem(mp)
+				// Pass the thread handle in case mp
+				// was in the process of shutting down.
+				profilem(mp, thread)
 			}
 			stdcall1(_ResumeThread, thread)
 		}