From f86f9a3038eb6db513a0ea36bc2af7a13b005e99 Mon Sep 17 00:00:00 2001 From: Bryan Mills Date: Mon, 4 Apr 2022 14:45:45 +0000 Subject: [PATCH] Revert "os: add handling of os.Interrupt for windows" This reverts CL 367495. Reason for revert: broke `x/tools` tests on Windows. Change-Id: Iab6b33259181c9520cf8db1e5b6edfeba763f974 Reviewed-on: https://go-review.googlesource.com/c/go/+/397997 Trust: Bryan Mills Run-TryBot: Bryan Mills TryBot-Result: Gopher Robot Reviewed-by: Cherry Mui --- .../syscall/windows/syscall_windows.go | 1 - .../syscall/windows/zsyscall_windows.go | 9 -------- src/os/exec/exec_windows_test.go | 18 ---------------- src/os/exec_posix.go | 4 +++- src/os/exec_windows.go | 21 +++++++------------ src/os/signal/signal_windows_test.go | 17 ++++++++++++++- src/runtime/signal_windows_test.go | 18 +++++++++++++++- 7 files changed, 43 insertions(+), 45 deletions(-) diff --git a/src/internal/syscall/windows/syscall_windows.go b/src/internal/syscall/windows/syscall_windows.go index 07d0cc7ccc..f8965d0bab 100644 --- a/src/internal/syscall/windows/syscall_windows.go +++ b/src/internal/syscall/windows/syscall_windows.go @@ -344,4 +344,3 @@ func LoadGetFinalPathNameByHandle() error { //sys DestroyEnvironmentBlock(block *uint16) (err error) = userenv.DestroyEnvironmentBlock //sys RtlGenRandom(buf []byte) (err error) = advapi32.SystemFunction036 -//sys GenerateConsoleCtrlEvent(ctrlEvent uint32, processGroupID uint32) (err error) = kernel32.GenerateConsoleCtrlEvent diff --git a/src/internal/syscall/windows/zsyscall_windows.go b/src/internal/syscall/windows/zsyscall_windows.go index 4de662cc08..aaad4a5b94 100644 --- a/src/internal/syscall/windows/zsyscall_windows.go +++ b/src/internal/syscall/windows/zsyscall_windows.go @@ -54,7 +54,6 @@ var ( procSetTokenInformation = modadvapi32.NewProc("SetTokenInformation") procSystemFunction036 = modadvapi32.NewProc("SystemFunction036") procGetAdaptersAddresses = modiphlpapi.NewProc("GetAdaptersAddresses") - procGenerateConsoleCtrlEvent = modkernel32.NewProc("GenerateConsoleCtrlEvent") procGetACP = modkernel32.NewProc("GetACP") procGetComputerNameExW = modkernel32.NewProc("GetComputerNameExW") procGetConsoleCP = modkernel32.NewProc("GetConsoleCP") @@ -162,14 +161,6 @@ func GetAdaptersAddresses(family uint32, flags uint32, reserved uintptr, adapter return } -func GenerateConsoleCtrlEvent(ctrlEvent uint32, processGroupID uint32) (err error) { - r1, _, e1 := syscall.Syscall(procGenerateConsoleCtrlEvent.Addr(), 2, uintptr(ctrlEvent), uintptr(processGroupID), 0) - if r1 == 0 { - err = errnoErr(e1) - } - return -} - func GetACP() (acp uint32) { r0, _, _ := syscall.Syscall(procGetACP.Addr(), 0, 0, 0, 0) acp = uint32(r0) diff --git a/src/os/exec/exec_windows_test.go b/src/os/exec/exec_windows_test.go index 503867f9c8..8e31e47190 100644 --- a/src/os/exec/exec_windows_test.go +++ b/src/os/exec/exec_windows_test.go @@ -7,7 +7,6 @@ package exec_test import ( - "internal/testenv" "io" "os" "os/exec" @@ -55,20 +54,3 @@ func TestNoInheritHandles(t *testing.T) { t.Fatalf("got exit code %d; want 88", exitError.ExitCode()) } } - -func TestErrProcessDone(t *testing.T) { - testenv.MustHaveGoBuild(t) - // On Windows, ProcAttr cannot be empty - p, err := os.StartProcess(testenv.GoToolPath(t), []string{""}, - &os.ProcAttr{Dir: "", Env: nil, Files: []*os.File{os.Stdin, os.Stdout, os.Stderr}, Sys: nil}) - if err != nil { - t.Errorf("starting test process: %v", err) - } - _, err = p.Wait() - if err != nil { - t.Errorf("Wait: %v", err) - } - if got := p.Signal(os.Kill); got != os.ErrProcessDone { - t.Fatalf("got %v want %v", got, os.ErrProcessDone) - } -} diff --git a/src/os/exec_posix.go b/src/os/exec_posix.go index 3dc18a84bd..e1e7d53a27 100644 --- a/src/os/exec_posix.go +++ b/src/os/exec_posix.go @@ -15,7 +15,9 @@ import ( // The only signal values guaranteed to be present in the os package on all // systems are os.Interrupt (send the process an interrupt) and os.Kill (force -// the process to exit). +// the process to exit). On Windows, sending os.Interrupt to a process with +// os.Process.Signal is not implemented; it will return an error instead of +// sending a signal. var ( Interrupt Signal = syscall.SIGINT Kill Signal = syscall.SIGKILL diff --git a/src/os/exec_windows.go b/src/os/exec_windows.go index bc232e0a00..239bed198f 100644 --- a/src/os/exec_windows.go +++ b/src/os/exec_windows.go @@ -47,14 +47,13 @@ func (p *Process) wait() (ps *ProcessState, err error) { func (p *Process) signal(sig Signal) error { handle := atomic.LoadUintptr(&p.handle) + if handle == uintptr(syscall.InvalidHandle) { + return syscall.EINVAL + } if p.done() { return ErrProcessDone } - s, ok := sig.(syscall.Signal) - if !ok { - return syscall.EWINDOWS - } - if s == syscall.SIGKILL { + if sig == Kill { var terminationHandle syscall.Handle e := syscall.DuplicateHandle(^syscall.Handle(0), syscall.Handle(handle), ^syscall.Handle(0), &terminationHandle, syscall.PROCESS_TERMINATE, false, 0) if e != nil { @@ -62,17 +61,11 @@ func (p *Process) signal(sig Signal) error { } runtime.KeepAlive(p) defer syscall.CloseHandle(terminationHandle) - e = syscall.TerminateProcess(terminationHandle, 1) + e = syscall.TerminateProcess(syscall.Handle(terminationHandle), 1) return NewSyscallError("TerminateProcess", e) } - if s == syscall.SIGINT { - e := windows.GenerateConsoleCtrlEvent(syscall.CTRL_BREAK_EVENT, uint32(p.Pid)) - if e != nil { - return NewSyscallError("GenerateConsoleCtrlEvent", e) - } - return nil - } - return syscall.EWINDOWS + // TODO(rsc): Handle Interrupt too? + return syscall.Errno(syscall.EWINDOWS) } func (p *Process) release() error { diff --git a/src/os/signal/signal_windows_test.go b/src/os/signal/signal_windows_test.go index 89c072ca6e..9b14551572 100644 --- a/src/os/signal/signal_windows_test.go +++ b/src/os/signal/signal_windows_test.go @@ -15,6 +15,21 @@ import ( "time" ) +func sendCtrlBreak(t *testing.T, pid int) { + d, e := syscall.LoadDLL("kernel32.dll") + if e != nil { + t.Fatalf("LoadDLL: %v\n", e) + } + p, e := d.FindProc("GenerateConsoleCtrlEvent") + if e != nil { + t.Fatalf("FindProc: %v\n", e) + } + r, _, e := p.Call(syscall.CTRL_BREAK_EVENT, uintptr(pid)) + if r == 0 { + t.Fatalf("GenerateConsoleCtrlEvent: %v\n", e) + } +} + func TestCtrlBreak(t *testing.T) { // create source file const source = ` @@ -75,7 +90,7 @@ func main() { } go func() { time.Sleep(1 * time.Second) - cmd.Process.Signal(os.Interrupt) + sendCtrlBreak(t, cmd.Process.Pid) }() err = cmd.Wait() if err != nil { diff --git a/src/runtime/signal_windows_test.go b/src/runtime/signal_windows_test.go index 1f329f4548..add23cd292 100644 --- a/src/runtime/signal_windows_test.go +++ b/src/runtime/signal_windows_test.go @@ -59,6 +59,22 @@ func TestVectoredHandlerDontCrashOnLibrary(t *testing.T) { } } +func sendCtrlBreak(pid int) error { + kernel32, err := syscall.LoadDLL("kernel32.dll") + if err != nil { + return fmt.Errorf("LoadDLL: %v\n", err) + } + generateEvent, err := kernel32.FindProc("GenerateConsoleCtrlEvent") + if err != nil { + return fmt.Errorf("FindProc: %v\n", err) + } + result, _, err := generateEvent.Call(syscall.CTRL_BREAK_EVENT, uintptr(pid)) + if result == 0 { + return fmt.Errorf("GenerateConsoleCtrlEvent: %v\n", err) + } + return nil +} + // TestCtrlHandler tests that Go can gracefully handle closing the console window. // See https://golang.org/issues/41884. func TestCtrlHandler(t *testing.T) { @@ -167,7 +183,7 @@ func TestLibraryCtrlHandler(t *testing.T) { } else if strings.TrimSpace(line) != "ready" { errCh <- fmt.Errorf("unexpected message: %v", line) } else { - errCh <- cmd.Process.Signal(syscall.SIGINT) + errCh <- sendCtrlBreak(cmd.Process.Pid) } }() -- 2.50.0