Skip to content

Commit 8cfa019

Browse files
ncruceszx2c4
authored andcommitted
runtime: block console ctrlhandler when the signal is handled
Fixes #41884 I can confirm this change fixes my issue. I can't confirm that this doesn't break any and everything else. I see that this code has been tweaked repeatedly, so I would really welcome guidance into further testing. Change-Id: I1986dd0c2f30cfe10257f0d8c658988d6986f7a6 GitHub-Last-Rev: 92f02c9 GitHub-Pull-Request: #41886 Reviewed-on: https://go-review.googlesource.com/c/go/+/261057 Run-TryBot: Jason A. Donenfeld <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Jason A. Donenfeld <[email protected]> Trust: Jason A. Donenfeld <[email protected]> Trust: Alex Brainman <[email protected]>
1 parent ff9e836 commit 8cfa019

File tree

3 files changed

+90
-0
lines changed

3 files changed

+90
-0
lines changed

src/runtime/os_windows.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ const (
4646
//go:cgo_import_dynamic runtime._SetThreadPriority SetThreadPriority%2 "kernel32.dll"
4747
//go:cgo_import_dynamic runtime._SetUnhandledExceptionFilter SetUnhandledExceptionFilter%1 "kernel32.dll"
4848
//go:cgo_import_dynamic runtime._SetWaitableTimer SetWaitableTimer%6 "kernel32.dll"
49+
//go:cgo_import_dynamic runtime._Sleep Sleep%1 "kernel32.dll"
4950
//go:cgo_import_dynamic runtime._SuspendThread SuspendThread%1 "kernel32.dll"
5051
//go:cgo_import_dynamic runtime._SwitchToThread SwitchToThread%0 "kernel32.dll"
5152
//go:cgo_import_dynamic runtime._TlsAlloc TlsAlloc%0 "kernel32.dll"
@@ -97,6 +98,7 @@ var (
9798
_SetThreadPriority,
9899
_SetUnhandledExceptionFilter,
99100
_SetWaitableTimer,
101+
_Sleep,
100102
_SuspendThread,
101103
_SwitchToThread,
102104
_TlsAlloc,
@@ -1094,6 +1096,11 @@ func ctrlhandler1(_type uint32) uint32 {
10941096
}
10951097

10961098
if sigsend(s) {
1099+
if s == _SIGTERM {
1100+
// Windows terminates the process after this handler returns.
1101+
// Block indefinitely to give signal handlers a chance to clean up.
1102+
stdcall1(_Sleep, uintptr(_INFINITE))
1103+
}
10971104
return 1
10981105
}
10991106
return 0

src/runtime/signal_windows_test.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"os/exec"
1212
"path/filepath"
1313
"runtime"
14+
"strconv"
1415
"strings"
1516
"syscall"
1617
"testing"
@@ -79,6 +80,69 @@ func sendCtrlBreak(pid int) error {
7980
return nil
8081
}
8182

83+
// TestCtrlHandler tests that Go can gracefully handle closing the console window.
84+
// See https://golang.org/issues/41884.
85+
func TestCtrlHandler(t *testing.T) {
86+
testenv.MustHaveGoBuild(t)
87+
t.Parallel()
88+
89+
// build go program
90+
exe := filepath.Join(t.TempDir(), "test.exe")
91+
cmd := exec.Command(testenv.GoToolPath(t), "build", "-o", exe, "testdata/testwinsignal/main.go")
92+
out, err := testenv.CleanCmdEnv(cmd).CombinedOutput()
93+
if err != nil {
94+
t.Fatalf("failed to build go exe: %v\n%s", err, out)
95+
}
96+
97+
// run test program
98+
cmd = exec.Command(exe)
99+
var stderr bytes.Buffer
100+
cmd.Stderr = &stderr
101+
outPipe, err := cmd.StdoutPipe()
102+
if err != nil {
103+
t.Fatalf("Failed to create stdout pipe: %v", err)
104+
}
105+
outReader := bufio.NewReader(outPipe)
106+
107+
// in a new command window
108+
const _CREATE_NEW_CONSOLE = 0x00000010
109+
cmd.SysProcAttr = &syscall.SysProcAttr{
110+
CreationFlags: _CREATE_NEW_CONSOLE,
111+
HideWindow: true,
112+
}
113+
if err := cmd.Start(); err != nil {
114+
t.Fatalf("Start failed: %v", err)
115+
}
116+
defer func() {
117+
cmd.Process.Kill()
118+
cmd.Wait()
119+
}()
120+
121+
// wait for child to be ready to receive signals
122+
if line, err := outReader.ReadString('\n'); err != nil {
123+
t.Fatalf("could not read stdout: %v", err)
124+
} else if strings.TrimSpace(line) != "ready" {
125+
t.Fatalf("unexpected message: %s", line)
126+
}
127+
128+
// gracefully kill pid, this closes the command window
129+
if err := exec.Command("taskkill.exe", "/pid", strconv.Itoa(cmd.Process.Pid)).Run(); err != nil {
130+
t.Fatalf("failed to kill: %v", err)
131+
}
132+
133+
// check child received, handled SIGTERM
134+
if line, err := outReader.ReadString('\n'); err != nil {
135+
t.Fatalf("could not read stdout: %v", err)
136+
} else if expected, got := syscall.SIGTERM.String(), strings.TrimSpace(line); expected != got {
137+
t.Fatalf("Expected '%s' got: %s", expected, got)
138+
}
139+
140+
// check child exited gracefully, did not timeout
141+
if err := cmd.Wait(); err != nil {
142+
t.Fatalf("Program exited with error: %v\n%s", err, &stderr)
143+
}
144+
}
145+
82146
// TestLibraryCtrlHandler tests that Go DLL allows calling program to handle console control events.
83147
// See https://golang.org/issues/35965.
84148
func TestLibraryCtrlHandler(t *testing.T) {
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package main
2+
3+
import (
4+
"fmt"
5+
"os"
6+
"os/signal"
7+
"time"
8+
)
9+
10+
func main() {
11+
c := make(chan os.Signal, 1)
12+
signal.Notify(c)
13+
14+
fmt.Println("ready")
15+
sig := <-c
16+
17+
time.Sleep(time.Second)
18+
fmt.Println(sig)
19+
}

0 commit comments

Comments
 (0)