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 409d53783947155955a2ce187346a06a5d305873..03dd95bf175d20c48d18b1948b08e38b412da822 100644 (file)
--- 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)
                }