From 36a8ac8a134b7791b9711103c17ac30bb676fe00 Mon Sep 17 00:00:00 2001 From: Bill Zissimopoulos Date: Fri, 25 May 2018 11:57:15 -0700 Subject: [PATCH 1/3] runtime: handle windows callback on non-go thread --- src/runtime/proc.go | 11 ++++++++--- src/runtime/syscall_windows_test.go | 2 -- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/runtime/proc.go b/src/runtime/proc.go index e312c575d0fb45..faebd239f7cae6 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -1293,7 +1293,7 @@ func mstart1() { //go:yeswritebarrierrec func mstartm0() { // Create an extra M for callbacks on threads not created by Go. - if iscgo && !cgoHasExtraM { + if (iscgo || GOOS == "windows") && !cgoHasExtraM { cgoHasExtraM = true newextram() } @@ -1618,7 +1618,7 @@ 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 not throw, because scheduler is not initialized yet. write(2, unsafe.Pointer(&earlycgocallback[0]), int32(len(earlycgocallback))) @@ -4215,8 +4215,13 @@ func checkdead() { return } + 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 2b057213f237a9..0f5e13f97ecc12 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 { From 51f9bd2b99084d2c6f7c43f3b36314577f478436 Mon Sep 17 00:00:00 2001 From: Bill Zissimopoulos Date: Sat, 2 Jun 2018 22:04:09 -0700 Subject: [PATCH 2/3] runtime: handle windows callback on non-go thread - add comments as per reviewer --- src/runtime/proc.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/runtime/proc.go b/src/runtime/proc.go index faebd239f7cae6..e6a416bdb86433 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -1293,6 +1293,8 @@ func mstart1() { //go:yeswritebarrierrec func mstartm0() { // Create an extra M for callbacks on threads not created by Go. + // An extra M is also needed on Windows for callbacks created by + // syscall.NewCallback. if (iscgo || GOOS == "windows") && !cgoHasExtraM { cgoHasExtraM = true newextram() @@ -1620,6 +1622,9 @@ func allocm(_p_ *p, fn func()) *m { func needm(x byte) { 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 using syscall.NewCallback. + // // Can not throw, because scheduler is not initialized yet. write(2, unsafe.Pointer(&earlycgocallback[0]), int32(len(earlycgocallback))) exit(1) @@ -4215,6 +4220,9 @@ 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.) var run0 int32 if !iscgo && cgoHasExtraM { run0 = 1 From d429e3eed923640edab580bdb47fcb81e75dbfe8 Mon Sep 17 00:00:00 2001 From: Bill Zissimopoulos Date: Sun, 3 Jun 2018 10:24:36 -0700 Subject: [PATCH 3/3] runtime: handle windows callback on non-go thread - additional comments as per reviewer --- src/runtime/proc.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/runtime/proc.go b/src/runtime/proc.go index e6a416bdb86433..9dc9bfbf64e329 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -1294,7 +1294,7 @@ func mstart1() { func mstartm0() { // Create an extra M for callbacks on threads not created by Go. // An extra M is also needed on Windows for callbacks created by - // syscall.NewCallback. + // syscall.NewCallback. See issue #6751 for details. if (iscgo || GOOS == "windows") && !cgoHasExtraM { cgoHasExtraM = true newextram() @@ -1623,7 +1623,8 @@ func needm(x byte) { 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 using syscall.NewCallback. + // 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))) @@ -4222,7 +4223,8 @@ func checkdead() { // 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.) + // accommodate callbacks created by syscall.NewCallback. See issue #6751 + // for details.) var run0 int32 if !iscgo && cgoHasExtraM { run0 = 1