go: runtime: TestLibraryCtrlHandler fails occasionally with register ABI enabled

Found a failure on the windows-amd64-regabi bot:

--- FAIL: TestLibraryCtrlHandler (6.32s)
    signal_windows_test.go:205: Program exited with error: exit status 1
        FAILURE: No signal received
FAIL
FAIL	runtime	52.447s

Seems like a flake, but we should look into it.

About this issue

Original URL
State: closed
Created 3 years ago
Comments: 63 (57 by maintainers)

Commits related to this issue

runtime: add missing import "C" in TestLibraryCtrlHandler CL 211139 added TestLibraryCtrlHandler. But the CL left out import "C" line in the test file that is supposed to be build with Cgo. While de... — committed to golang/go by alexbrainman 3 years ago
runtime: replace --buildmode with -buildmode in tests While debugging issue #45638, I discovered that some tests were using --buildmode command line parameter instead of -buildmode. The --buildmode ... — committed to golang/go by alexbrainman 3 years ago
runtime: abort when receiving a signal on a non-Go thread on Windows Currently if a signal lands on a non-Go thread that's handled by the Go handler, Go will emit a message. However, unlike everywher... — committed to golang/go by mknyszek 3 years ago
runtime: wait for Go runtime to initialize in Windows signal test The test harness waits for "ready" as a sign that the Go runtime has installed its signal handler and is ready to be tested. But actu... — committed to golang/go by zx2c4 3 years ago

Most upvoted comments

@dmitshur I have been working on this issue.

I think the problem here is with the runtime.TestLibraryCtrlHandler. The test is racy and broken.

For example, the test was introduced together with the fix in CL 211139. But the test still passes, if I revert to commit before CL 211139.

c:\Users\Alex\dev\go\src\runtime>git status
HEAD detached at 5756808ce8
nothing to commit, working tree clean

c:\Users\Alex\dev\go\src\runtime>go test -v -count=1 -run=yCtrlH
=== RUN   TestLibraryCtrlHandler
--- PASS: TestLibraryCtrlHandler (2.98s)
PASS
ok      runtime 3.569s

c:\Users\Alex\dev\go\src\runtime>git checkout HEAD~ os_windows.go
Updated 1 path from 2e2e0c0cf8

c:\Users\Alex\dev\go\src\runtime>git diff --staged
diff --git a/src/runtime/os_windows.go b/src/runtime/os_windows.go
index 7576565599..bddc25729a 100644
--- a/src/runtime/os_windows.go
+++ b/src/runtime/os_windows.go
@@ -1031,11 +1031,7 @@ func ctrlhandler1(_type uint32) uint32 {
        if sigsend(s) {
                return 1
        }
-       if !islibrary && !isarchive {
-               // Only exit the program if we don't have a DLL.
-               // See https://golang.org/issues/35965.
-               exit(2) // SIGINT, SIGTERM, etc
-       }
+       exit(2) // SIGINT, SIGTERM, etc
        return 0
 }


c:\Users\Alex\dev\go\src\runtime>go test -v -count=1 -run=yCtrlH
=== RUN   TestLibraryCtrlHandler
--- PASS: TestLibraryCtrlHandler (3.86s)
PASS
ok      runtime 4.270s

c:\Users\Alex\dev\go\src\runtime>

The runtime.TestLibraryCtrlHandler assumes that by the time LoadLibrary returns that Go runtime has finished initialising (including calling SetConsoleCtrlHandler in runtime)

https://github.com/golang/go/blob/831573cd21e65c37b4e1bf5d44dc23b125084b7a/src/runtime/testdata/testwinlibsignal/main.c#L35-L42

But there is nothing in the test to verify that SetConsoleCtrlHandler was called or not. In fact, if I comment out LoadLibrary call in the test, the test still passes.

So we need to replace the test first. And then we should be able to tell, if we have a problem or not.

It is pretty hard to build proper test here. But I think I am making progress. I will report more when I am ready with the test.

I hope it helps.

Alex

alexbrainman on May 21, 2021

Cute. Nice analysis.

ianlancetaylor on Apr 30, 2021