Skip to content

Commit

Permalink
runtime: handle windows callback on non-go thread
Browse files Browse the repository at this point in the history
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: d429e3e
GitHub-Pull-Request: #25575
Reviewed-on: https://go-review.googlesource.com/114802
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
  • Loading branch information
billziss-gh authored and ianlancetaylor committed Jun 5, 2018
1 parent 4a2bec9 commit bb0fae6
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 5 deletions.
21 changes: 18 additions & 3 deletions src/runtime/proc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 0 additions & 2 deletions src/runtime/syscall_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit bb0fae6

Please sign in to comment.