From 8f3f7c14883149cd6dc28ff0a288342f41d11d72 Mon Sep 17 00:00:00 2001 From: Nuno Cruces Date: Tue, 15 Mar 2022 12:37:13 +0000 Subject: [PATCH 1/5] runtime: change TestCtrlHandler to run in ConPTY Fixes #51602. Previous test would not run in a pseudo-console (ConPTY). New test avoids taskkill entirely by having the child request its own console window be closed. Verified that this runs locally (within a real console), over SSH (within a pseudo-console), and that it breaks if #41884 were reverted. --- src/runtime/signal_windows_test.go | 13 ------------- src/runtime/testdata/testwinsignal/main.go | 18 +++++++++++++++++- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/runtime/signal_windows_test.go b/src/runtime/signal_windows_test.go index 7c88ab573eab2a..1c49881ef3a15d 100644 --- a/src/runtime/signal_windows_test.go +++ b/src/runtime/signal_windows_test.go @@ -10,7 +10,6 @@ import ( "os/exec" "path/filepath" "runtime" - "strconv" "strings" "syscall" "testing" @@ -113,18 +112,6 @@ func TestCtrlHandler(t *testing.T) { cmd.Wait() }() - // wait for child to be ready to receive signals - if line, err := outReader.ReadString('\n'); err != nil { - t.Fatalf("could not read stdout: %v", err) - } else if strings.TrimSpace(line) != "ready" { - t.Fatalf("unexpected message: %s", line) - } - - // gracefully kill pid, this closes the command window - if err := exec.Command("taskkill.exe", "/pid", strconv.Itoa(cmd.Process.Pid)).Run(); err != nil { - t.Fatalf("failed to kill: %v", err) - } - // check child received, handled SIGTERM if line, err := outReader.ReadString('\n'); err != nil { t.Fatalf("could not read stdout: %v", err) diff --git a/src/runtime/testdata/testwinsignal/main.go b/src/runtime/testdata/testwinsignal/main.go index 1e7c9475fd6631..9b31198169709d 100644 --- a/src/runtime/testdata/testwinsignal/main.go +++ b/src/runtime/testdata/testwinsignal/main.go @@ -2,8 +2,10 @@ package main import ( "fmt" + "log" "os" "os/signal" + "syscall" "time" ) @@ -11,7 +13,21 @@ func main() { c := make(chan os.Signal, 1) signal.Notify(c) - fmt.Println("ready") + kernel32 := syscall.NewLazyDLL("kernel32.dll") + getConsoleWindow := kernel32.NewProc("GetConsoleWindow") + hwnd, _, err := getConsoleWindow.Call() + if hwnd == 0 { + log.Fatal("no associated console: ", err) + } + + const _WM_CLOSE = 0x0010 + user32 := syscall.NewLazyDLL("user32.dll") + postMessage := user32.NewProc("PostMessageW") + ok, _, err := postMessage.Call(hwnd, _WM_CLOSE, 0, 0) + if ok == 0 { + log.Fatal("post message failed: ", err) + } + sig := <-c time.Sleep(time.Second) From cb5f4ceb4ccd699a4548a1d77e9ee31404704cb7 Mon Sep 17 00:00:00 2001 From: Nuno Cruces Date: Tue, 15 Mar 2022 14:30:02 +0000 Subject: [PATCH 2/5] Timeout subprocess to avoid leaking it. --- src/runtime/testdata/testwinsignal/main.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/runtime/testdata/testwinsignal/main.go b/src/runtime/testdata/testwinsignal/main.go index 9b31198169709d..5786d440a3518c 100644 --- a/src/runtime/testdata/testwinsignal/main.go +++ b/src/runtime/testdata/testwinsignal/main.go @@ -10,9 +10,11 @@ import ( ) func main() { + // register to receive all signals c := make(chan os.Signal, 1) signal.Notify(c) + // get console window handle kernel32 := syscall.NewLazyDLL("kernel32.dll") getConsoleWindow := kernel32.NewProc("GetConsoleWindow") hwnd, _, err := getConsoleWindow.Call() @@ -20,6 +22,7 @@ func main() { log.Fatal("no associated console: ", err) } + // close console window const _WM_CLOSE = 0x0010 user32 := syscall.NewLazyDLL("user32.dll") postMessage := user32.NewProc("PostMessageW") @@ -28,8 +31,15 @@ func main() { log.Fatal("post message failed: ", err) } - sig := <-c + // check if we receive syscall.SIGTERM + select { + case sig := <-c: + // prentend to take some time handling the signal + time.Sleep(time.Second) + // print signal name, "terminated" makes the test succeed + fmt.Println(sig) - time.Sleep(time.Second) - fmt.Println(sig) + case <-time.After(time.Second): + log.Fatal("timed out waiting for signal") + } } From dbf8c119647413ac8fea49afe66dac70c30c6177 Mon Sep 17 00:00:00 2001 From: Nuno Cruces Date: Tue, 15 Mar 2022 15:10:21 +0000 Subject: [PATCH 3/5] Child terminates itself when it looses stdin. --- src/runtime/signal_windows_test.go | 21 +++++++++++---------- src/runtime/testdata/testwinsignal/main.go | 22 +++++++++++----------- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/runtime/signal_windows_test.go b/src/runtime/signal_windows_test.go index 1c49881ef3a15d..c3ad95d62f0835 100644 --- a/src/runtime/signal_windows_test.go +++ b/src/runtime/signal_windows_test.go @@ -90,13 +90,16 @@ func TestCtrlHandler(t *testing.T) { // run test program cmd = exec.Command(exe) + var stdout bytes.Buffer var stderr bytes.Buffer + cmd.Stdout = &stdout cmd.Stderr = &stderr - outPipe, err := cmd.StdoutPipe() + inPipe, err := cmd.StdinPipe() if err != nil { - t.Fatalf("Failed to create stdout pipe: %v", err) + t.Fatalf("Failed to create stdin pipe: %v", err) } - outReader := bufio.NewReader(outPipe) + // keep inPipe alive until the end of the test + defer inPipe.Close() // in a new command window const _CREATE_NEW_CONSOLE = 0x00000010 @@ -112,17 +115,15 @@ func TestCtrlHandler(t *testing.T) { cmd.Wait() }() - // check child received, handled SIGTERM - if line, err := outReader.ReadString('\n'); err != nil { - t.Fatalf("could not read stdout: %v", err) - } else if expected, got := syscall.SIGTERM.String(), strings.TrimSpace(line); expected != got { - t.Fatalf("Expected '%s' got: %s", expected, got) - } - // check child exited gracefully, did not timeout if err := cmd.Wait(); err != nil { t.Fatalf("Program exited with error: %v\n%s", err, &stderr) } + + // check child received, handled SIGTERM + if expected, got := syscall.SIGTERM.String(), strings.TrimSpace(stdout.String()); expected != got { + t.Fatalf("Expected '%s' got: %s", expected, got) + } } // TestLibraryCtrlHandler tests that Go DLL allows calling program to handle console control events. diff --git a/src/runtime/testdata/testwinsignal/main.go b/src/runtime/testdata/testwinsignal/main.go index 5786d440a3518c..e0480b0f185783 100644 --- a/src/runtime/testdata/testwinsignal/main.go +++ b/src/runtime/testdata/testwinsignal/main.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "io" "log" "os" "os/signal" @@ -10,6 +11,11 @@ import ( ) func main() { + go func() { + io.Copy(io.Discard, os.Stdin) + log.Fatal("stdin is closed; terminating") + }() + // register to receive all signals c := make(chan os.Signal, 1) signal.Notify(c) @@ -31,15 +37,9 @@ func main() { log.Fatal("post message failed: ", err) } - // check if we receive syscall.SIGTERM - select { - case sig := <-c: - // prentend to take some time handling the signal - time.Sleep(time.Second) - // print signal name, "terminated" makes the test succeed - fmt.Println(sig) - - case <-time.After(time.Second): - log.Fatal("timed out waiting for signal") - } + sig := <-c + // pretend to take some time handling the signal + time.Sleep(time.Second) + // print signal name, "terminated" makes the test succeed + fmt.Println(sig) } From 1380c189c6807e93d6e87aadfcb15b50496c7fd0 Mon Sep 17 00:00:00 2001 From: Nuno Cruces Date: Thu, 17 Mar 2022 15:29:48 +0000 Subject: [PATCH 4/5] Documentation. --- src/runtime/testdata/testwinsignal/main.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/runtime/testdata/testwinsignal/main.go b/src/runtime/testdata/testwinsignal/main.go index e0480b0f185783..3ec93608805bf1 100644 --- a/src/runtime/testdata/testwinsignal/main.go +++ b/src/runtime/testdata/testwinsignal/main.go @@ -11,6 +11,8 @@ import ( ) func main() { + // ensure that this process terminates when the test times out, + // even if the expected signal never arrives go func() { io.Copy(io.Discard, os.Stdin) log.Fatal("stdin is closed; terminating") From b1421e4bed2dc729c266928f002b39374d7e391a Mon Sep 17 00:00:00 2001 From: Nuno Cruces Date: Thu, 17 Mar 2022 16:07:53 +0000 Subject: [PATCH 5/5] Documentation. --- src/runtime/testdata/testwinsignal/main.go | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/runtime/testdata/testwinsignal/main.go b/src/runtime/testdata/testwinsignal/main.go index 3ec93608805bf1..e1136f3887929b 100644 --- a/src/runtime/testdata/testwinsignal/main.go +++ b/src/runtime/testdata/testwinsignal/main.go @@ -11,18 +11,18 @@ import ( ) func main() { - // ensure that this process terminates when the test times out, - // even if the expected signal never arrives + // Ensure that this process terminates when the test times out, + // even if the expected signal never arrives. go func() { io.Copy(io.Discard, os.Stdin) log.Fatal("stdin is closed; terminating") }() - // register to receive all signals + // Register to receive all signals. c := make(chan os.Signal, 1) signal.Notify(c) - // get console window handle + // Get console window handle. kernel32 := syscall.NewLazyDLL("kernel32.dll") getConsoleWindow := kernel32.NewProc("GetConsoleWindow") hwnd, _, err := getConsoleWindow.Call() @@ -30,7 +30,7 @@ func main() { log.Fatal("no associated console: ", err) } - // close console window + // Send message to close the console window. const _WM_CLOSE = 0x0010 user32 := syscall.NewLazyDLL("user32.dll") postMessage := user32.NewProc("PostMessageW") @@ -40,8 +40,14 @@ func main() { } sig := <-c - // pretend to take some time handling the signal + + // Allow some time for the handler to complete if it's going to. + // + // (In https://go.dev/issue/41884 the handler returned immediately, + // which caused Windows to terminate the program before the goroutine + // that received the SIGTERM had a chance to actually clean up.) time.Sleep(time.Second) - // print signal name, "terminated" makes the test succeed + + // Print the signal's name: "terminated" makes the test succeed. fmt.Println(sig) }