[go] os: terminate windows processes via handle directly
Jason A. Donenfeld (Gerrit)
Jason A. Donenfeld has uploaded this change for review.
os: terminate windows processes via handle directly
We already have a handle to the process, so use that for termination,
rather than the more race-prone PID method, which is heavier since it
opens a new handle too.
Change-Id: I2958c1817f12f3dd783412baacbf629049f6956a
---
M src/os/exec_windows.go
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/src/os/exec_windows.go b/src/os/exec_windows.go
index 5710401..ee1e4fb 100644
--- a/src/os/exec_windows.go
+++ b/src/os/exec_windows.go
@@ -45,16 +45,6 @@
return &ProcessState{p.Pid, syscall.WaitStatus{ExitCode: ec}, &u}, nil
}
-func terminateProcess(pid, exitcode int) error {
- h, e := syscall.OpenProcess(syscall.PROCESS_TERMINATE, false, uint32(pid))
- if e != nil {
- return NewSyscallError("OpenProcess", e)
- }
- defer syscall.CloseHandle(h)
- e = syscall.TerminateProcess(h, uint32(exitcode))
- return NewSyscallError("TerminateProcess", e)
-}
-
func (p *Process) signal(sig Signal) error {
handle := atomic.LoadUintptr(&p.handle)
if handle == uintptr(syscall.InvalidHandle) {
@@ -64,7 +54,8 @@
return ErrProcessDone
}
if sig == Kill {
- err := terminateProcess(p.Pid, 1)
+ e := syscall.TerminateProcess(syscall.Handle(handle), 1)
+ err := NewSyscallError("TerminateProcess", e)
runtime.KeepAlive(p)
return err
}
To view, visit change 322509. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Attention is currently required from: Alex Brainman, Brad Fitzpatrick.
Patch set 1:Run-TryBot +1Trust +1
Jason A. Donenfeld (Gerrit)
Attention is currently required from: Jason A. Donenfeld, Alex Brainman, Brad Fitzpatrick.
Jason A. Donenfeld uploaded patch set #2 to this change.
os: terminate windows processes via handle directly
We already have a handle to the process, so use that for termination,
rather than doing a new lookup based on the PID.
Change-Id: I2958c1817f12f3dd783412baacbf629049f6956a
---
M src/os/exec_windows.go
1 file changed, 8 insertions(+), 12 deletions(-)
To view, visit change 322509. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Attention is currently required from: Alex Brainman, Brad Fitzpatrick.
Patch set 2:Run-TryBot +1Trust +1
To view, visit change 322509. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Attention is currently required from: Alex Brainman, Brad Fitzpatrick.
Jason A. Donenfeld uploaded patch set #3 to this change.
os: terminate windows processes via handle directly
We already have a handle to the process, so use that for termination,
rather than doing a new lookup based on the PID.
Change-Id: I2958c1817f12f3dd783412baacbf629049f6956a
---
M src/os/exec_windows.go
1 file changed, 8 insertions(+), 12 deletions(-)
To view, visit change 322509. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Attention is currently required from: Alex Brainman, Brad Fitzpatrick.
Patch set 3:Run-TryBot +1Trust +1
To view, visit change 322509. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Attention is currently required from: Jason A. Donenfeld, Alex Brainman, Brad Fitzpatrick.
Jason A. Donenfeld uploaded patch set #4 to this change.
os: terminate windows processes via handle directly
We already have a handle to the process, so use that for termination,
rather than doing a new lookup based on the PID.
Change-Id: I2958c1817f12f3dd783412baacbf629049f6956a
---
M src/os/exec_windows.go
1 file changed, 8 insertions(+), 12 deletions(-)
To view, visit change 322509. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Attention is currently required from: Alex Brainman, Brad Fitzpatrick.
Patch set 4:Run-TryBot +1Trust +1
To view, visit change 322509. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Attention is currently required from: Alex Brainman, Brad Fitzpatrick.
1 comment:
Patchset:
Alex/Brad - could one of you review this commit and the subsequent one? It's for 1.17.
To view, visit change 322509. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Jason A. Donenfeld, Alex Brainman, Brad Fitzpatrick.
1 comment:
File src/os/exec_windows.go:
Patch Set #4, Line 58:
^syscall.Handle(0)Should we be using GetCurrentProcess here? That seems to be what the MSDN examples use.
To view, visit change 322509. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Attention is currently required from: Alex Brainman, Brad Fitzpatrick, Ian Lance Taylor.
1 comment:
File src/os/exec_windows.go:
Patch Set #4, Line 58:
^syscall.Handle(0)Should we be using GetCurrentProcess here? That seems to be what the MSDN examples use.
`GetCurrentProcess` just returns -1. The error handling of that function makes it annoying to use. That's why we replaced it with CurrentProcess in x/sys.
To view, visit change 322509. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Jason A. Donenfeld, Alex Brainman, Brad Fitzpatrick.
Patch set 4:Code-Review +2
Jason A. Donenfeld (Gerrit)
Jason A. Donenfeld submitted this change.
os: terminate windows processes via handle directly
We already have a handle to the process, so use that for termination,
rather than doing a new lookup based on the PID.
Change-Id: I2958c1817f12f3dd783412baacbf629049f6956a
Reviewed-on: https://go-review.googlesource.com/c/go/+/322509
Trust: Jason A. Donenfeld <Ja...@zx2c4.com>
Run-TryBot: Jason A. Donenfeld <Ja...@zx2c4.com>
TryBot-Result: Go Bot <go...@golang.org>
Reviewed-by: Ian Lance Taylor <ia...@golang.org>
---
M src/os/exec_windows.go
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/src/os/exec_windows.go b/src/os/exec_windows.go
index 5710401..b59a01a 100644
--- a/src/os/exec_windows.go
+++ b/src/os/exec_windows.go
@@ -45,16 +45,6 @@
return &ProcessState{p.Pid, syscall.WaitStatus{ExitCode: ec}, &u}, nil
}
-func terminateProcess(pid, exitcode int) error {
- h, e := syscall.OpenProcess(syscall.PROCESS_TERMINATE, false, uint32(pid))
- if e != nil {
- return NewSyscallError("OpenProcess", e)
- }
- defer syscall.CloseHandle(h)
- e = syscall.TerminateProcess(h, uint32(exitcode))
- return NewSyscallError("TerminateProcess", e)
-}
-
func (p *Process) signal(sig Signal) error {
handle := atomic.LoadUintptr(&p.handle)
if handle == uintptr(syscall.InvalidHandle) {
@@ -64,9 +54,15 @@
return ErrProcessDone
}
if sig == Kill {
- err := terminateProcess(p.Pid, 1)
+ 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)
- return err
+ defer syscall.CloseHandle(terminationHandle)
+ e = syscall.TerminateProcess(syscall.Handle(terminationHandle), 1)
+ return NewSyscallError("TerminateProcess", e)
}
// TODO(rsc): Handle Interrupt too?
return syscall.Errno(syscall.EWINDOWS)
To view, visit change 322509. To unsubscribe, or for help writing mail filters, visit settings.
Alex Brainman (Gerrit)
Patch set 5:Code-Review +2
1 comment:
Patchset:
Nice. Thank you.
Alex
To view, visit change 322509. To unsubscribe, or for help writing mail filters, visit settings.