From c216c3aa9fe34cd81e9d4bcc28c64257064eddc9 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Fri, 17 Aug 2018 15:42:19 -0400 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: Alex Brainman --- src/runtime/os_windows.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) 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) } -- 2.50.0