From bb0fae603bd19e096e38c3321d95bf114f40dcff Mon Sep 17 00:00:00 2001 From: Bill Zissimopoulos Date: Sun, 3 Jun 2018 17:25:29 +0000 Subject: [PATCH] runtime: handle windows callback on non-go thread Adds an extra M in mstartm0 and accounts for it in checkdead. This allows Windows callbacks created with syscall.NewCallback and syscall.NewCallbackCDecl to be called on a non-Go thread. Fixes #6751 Change-Id: I57626bc009a6370b9ca0827ab64b14b01dec39d4 GitHub-Last-Rev: d429e3eed923640edab580bdb47fcb81e75dbfe8 GitHub-Pull-Request: golang/go#25575 Reviewed-on: https://go-review.googlesource.com/114802 Reviewed-by: Alex Brainman Reviewed-by: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Gobot Gobot --- src/runtime/proc.go | 21 ++++++++++++++++++--- src/runtime/syscall_windows_test.go | 2 -- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/runtime/proc.go b/src/runtime/proc.go index 9f4feebfd7..9908951544 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -1293,7 +1293,9 @@ func mstart1() { //go:yeswritebarrierrec func mstartm0() { // Create an extra M for callbacks on threads not created by Go. - if iscgo && !cgoHasExtraM { + // An extra M is also needed on Windows for callbacks created by + // syscall.NewCallback. See issue #6751 for details. + if (iscgo || GOOS == "windows") && !cgoHasExtraM { cgoHasExtraM = true newextram() } @@ -1618,8 +1620,12 @@ func allocm(_p_ *p, fn func()) *m { // put the m back on the list. //go:nosplit func needm(x byte) { - if iscgo && !cgoHasExtraM { + if (iscgo || GOOS == "windows") && !cgoHasExtraM { // Can happen if C/C++ code calls Go from a global ctor. + // Can also happen on Windows if a global ctor uses a + // callback created by syscall.NewCallback. See issue #6751 + // for details. + // // Can not throw, because scheduler is not initialized yet. write(2, unsafe.Pointer(&earlycgocallback[0]), int32(len(earlycgocallback))) exit(1) @@ -4215,8 +4221,17 @@ func checkdead() { return } + // If we are not running under cgo, but we have an extra M then account + // for it. (It is possible to have an extra M on Windows without cgo to + // accommodate callbacks created by syscall.NewCallback. See issue #6751 + // for details.) + var run0 int32 + if !iscgo && cgoHasExtraM { + run0 = 1 + } + run := mcount() - sched.nmidle - sched.nmidlelocked - sched.nmsys - if run > 0 { + if run > run0 { return } if run < 0 { diff --git a/src/runtime/syscall_windows_test.go b/src/runtime/syscall_windows_test.go index 2b057213f2..0f5e13f97e 100644 --- a/src/runtime/syscall_windows_test.go +++ b/src/runtime/syscall_windows_test.go @@ -251,8 +251,6 @@ func TestBlockingCallback(t *testing.T) { } func TestCallbackInAnotherThread(t *testing.T) { - t.Skip("Skipping failing test (see golang.org/issue/6751 for details)") - d := GetDLL(t, "kernel32.dll") f := func(p uintptr) uintptr {