Skip to content

Commit

Permalink
os: add handling of os.Interrupt for windows
Browse files Browse the repository at this point in the history
Add GenerateConsoleCtrlEvent call to internal syscall package.
Define ErrProcessDone while reviewing handling of os.Signal().
Update test to run for windows using the added call.

Fixes golang#42311
Fixes golang#46354

Change-Id: I460955efc76c4febe04b612ac9a0670e62ba5ff3
Reviewed-on: https://go-review.googlesource.com/c/go/+/367495
Trust: Patrik Nyblom <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
  • Loading branch information
iwdgo authored and ianlancetaylor committed Apr 3, 2022
1 parent 85b5f86 commit 3451844
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 43 deletions.
1 change: 1 addition & 0 deletions src/internal/syscall/windows/syscall_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,3 +344,4 @@ 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
9 changes: 9 additions & 0 deletions src/internal/syscall/windows/zsyscall_windows.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions src/os/exec/exec_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
package exec_test

import (
"internal/testenv"
"io"
"os"
"os/exec"
Expand Down Expand Up @@ -54,3 +55,20 @@ 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)
}
}
4 changes: 1 addition & 3 deletions src/os/exec_posix.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ 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). 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.
// the process to exit).
var (
Interrupt Signal = syscall.SIGINT
Kill Signal = syscall.SIGKILL
Expand Down
21 changes: 14 additions & 7 deletions src/os/exec_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,25 +47,32 @@ 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
}
if sig == Kill {
s, ok := sig.(syscall.Signal)
if !ok {
return syscall.EWINDOWS
}
if s == syscall.SIGKILL {
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 {
return NewSyscallError("DuplicateHandle", e)
}
runtime.KeepAlive(p)
defer syscall.CloseHandle(terminationHandle)
e = syscall.TerminateProcess(syscall.Handle(terminationHandle), 1)
e = syscall.TerminateProcess(terminationHandle, 1)
return NewSyscallError("TerminateProcess", e)
}
// TODO(rsc): Handle Interrupt too?
return syscall.Errno(syscall.EWINDOWS)
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
}

func (p *Process) release() error {
Expand Down
17 changes: 1 addition & 16 deletions src/os/signal/signal_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,6 @@ 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 = `
Expand Down Expand Up @@ -90,7 +75,7 @@ func main() {
}
go func() {
time.Sleep(1 * time.Second)
sendCtrlBreak(t, cmd.Process.Pid)
cmd.Process.Signal(os.Interrupt)
}()
err = cmd.Wait()
if err != nil {
Expand Down
18 changes: 1 addition & 17 deletions src/runtime/signal_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,22 +59,6 @@ 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) {
Expand Down Expand Up @@ -183,7 +167,7 @@ func TestLibraryCtrlHandler(t *testing.T) {
} else if strings.TrimSpace(line) != "ready" {
errCh <- fmt.Errorf("unexpected message: %v", line)
} else {
errCh <- sendCtrlBreak(cmd.Process.Pid)
errCh <- cmd.Process.Signal(syscall.SIGINT)
}
}()

Expand Down

0 comments on commit 3451844

Please sign in to comment.