[go] os: add handling of os.Interrupt for windows
Constantin Konstantinidis (Gerrit)
Constantin Konstantinidis has uploaded this change for review.
os: add handling of os.Interrupt for windows
Add GenerateConsoleCtrlEvent call to internal syscall package.
Port test of ErrProcessDone from *nix to windows.
Update test to run for windows using the added call.
Fixes #46354
Change-Id: Icabafaa7be030d330500882a042410835339fde5
---
M src/internal/syscall/windows/syscall_windows.go
M src/internal/syscall/windows/zsyscall_windows.go
M src/os/exec/exec_windows_test.go
M src/os/exec_posix.go
M src/os/exec_windows.go
M src/os/signal/signal_windows_test.go
M src/runtime/signal_windows_test.go
7 files changed, 60 insertions(+), 43 deletions(-)
diff --git a/src/internal/syscall/windows/syscall_windows.go b/src/internal/syscall/windows/syscall_windows.go
index f8965d0..07d0cc7 100644
--- a/src/internal/syscall/windows/syscall_windows.go
+++ b/src/internal/syscall/windows/syscall_windows.go
@@ -344,3 +344,4 @@
//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 aaad4a5..4de662c 100644
--- a/src/internal/syscall/windows/zsyscall_windows.go
+++ b/src/internal/syscall/windows/zsyscall_windows.go
@@ -54,6 +54,7 @@
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")
@@ -161,6 +162,14 @@
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 35ae0b0..b3190f0 100644
--- a/src/os/exec/exec_windows_test.go
+++ b/src/os/exec/exec_windows_test.go
@@ -8,6 +8,7 @@
import (
"fmt"
+ "internal/testenv"
"io"
"os"
"os/exec"
@@ -96,3 +97,20 @@
t.Error("no SYSTEMROOT found")
}
}
+
+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 e1e7d53..3dc18a8 100644
--- a/src/os/exec_posix.go
+++ b/src/os/exec_posix.go
@@ -15,9 +15,7 @@
// 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
diff --git a/src/os/exec_windows.go b/src/os/exec_windows.go
index 239bed1..bc232e0 100644
--- a/src/os/exec_windows.go
+++ b/src/os/exec_windows.go
@@ -47,13 +47,14 @@
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 {
@@ -61,11 +62,17 @@
}
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 {
diff --git a/src/os/signal/signal_windows_test.go b/src/os/signal/signal_windows_test.go
index 9b14551..89c072c 100644
--- a/src/os/signal/signal_windows_test.go
+++ b/src/os/signal/signal_windows_test.go
@@ -15,21 +15,6 @@
"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 = `
@@ -90,7 +75,7 @@
}
go func() {
time.Sleep(1 * time.Second)
- sendCtrlBreak(t, cmd.Process.Pid)
+ cmd.Process.Signal(os.Interrupt)
}()
err = cmd.Wait()
if err != nil {
diff --git a/src/runtime/signal_windows_test.go b/src/runtime/signal_windows_test.go
index add23cd..1f329f4 100644
--- a/src/runtime/signal_windows_test.go
+++ b/src/runtime/signal_windows_test.go
@@ -59,22 +59,6 @@
}
}
-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) {
@@ -183,7 +167,7 @@
} 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)
}
}()
To view, visit change 409294. To unsubscribe, or for help writing mail filters, visit settings.
Constantin Konstantinidis (Gerrit)
Constantin Konstantinidis abandoned this change.
To view, visit change 409294. To unsubscribe, or for help writing mail filters, visit settings.
Constantin Konstantinidis (Gerrit)
Constantin Konstantinidis restored this change.
To view, visit change 409294. To unsubscribe, or for help writing mail filters, visit settings.
Constantin Konstantinidis (Gerrit)
Attention is currently required from: Alex Brainman, Austin Clements, Brad Fitzpatrick, Bryan Mills, Dmitry Vyukov, Ian Lance Taylor, Ian Lance Taylor, Keith Randall, Martin Möhrmann, Michael Knyszek, Michael Pratt, Patrik Nyblom, Rob Pike, Robert Griesemer, Russ Cox.
1 comment:
Patchset:
After revert of https://go-review.googlesource.com/c/go/+/367495, it seems that a new path is needed. Last answers to revert are on the previous patch.
If this is not the appropriate procedure, please let me know.
To view, visit change 409294. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Attention is currently required from: Alex Brainman, Austin Clements, Brad Fitzpatrick, Constantin Konstantinidis, Dmitry Vyukov, Ian Lance Taylor, Ian Lance Taylor, Keith Randall, Martin Möhrmann, Michael Knyszek, Michael Pratt, Patrik Nyblom, Robert Griesemer, Russ Cox.
Patch set 1:Hold +1
2 comments:
Commit Message:
Patch Set #1, Line 13:
Fixes #46354As I noted in https://go-review.googlesource.com/c/go/+/367495/10#message-956cf0707395b5ef8e2420122c16ace56e626860, this is an API addition and needs an explicit proposal.
`os.Process.Signal` today sends a signal only to the designated process. This change would cause it to send certain signals to an entire process group, and I believe that difference in behavior is what caused the `x/tools` failures observed at CL 367495.
Patch Set #1, Line 13:
#46354The relationship between this issue and #46354 is not clear to me.
That issue (along with #29744) is reporting what appears to be a race between runtime initialization and the `CTRL_BREAK` signal handler, but I don't see anything in this CL (or in its precursor CL 367495) that would specifically address that race. (At the very least, more detail in the commit message would be helpful.)
To view, visit change 409294. To unsubscribe, or for help writing mail filters, visit settings.