Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os: kill process failed in the windows #5615

Closed
gopherbot opened this issue Jun 3, 2013 · 7 comments
Closed

os: kill process failed in the windows #5615

gopherbot opened this issue Jun 3, 2013 · 7 comments

Comments

@gopherbot
Copy link
Contributor

by runner.mei:

What steps will reproduce the problem?

kill process failed in the windows

What is the expected output?

"TerminateProcess: Access is denied."

What do you see instead?

kill process success in the windows

Which operating system are you using?

windows

Which version are you using?  (run 'go version')

go version go1.1 windows/amd64

Please provide any additional information below.

// unit test
func TestKillProcess(t *testing.T) {
    pr := exec.Command("ping", "127.0.0.1", "-t")
    e := pr.Start()
    if nil != e {
        t.Error(e)
        return
    }
    kp, _ := os.FindProcess(pr.Process.Pid)
    e = kp.Kill()

    if nil != e {
        t.Error(e)
        pr.Process.Kill()
        return
    }
}
/// --- FAIL: TestKillProcess (0.06 seconds)
///        kill_windows_test.go:20: TerminateProcess: Access is denied.
/// FAIL
/// exit status 1
/// FAIL    daemon  45.852s


PATCH
// src/pkg/os/exec_windows.go
71a72
>    const PROCESS_TERMINATE = 0x0001
73c74
<        syscall.PROCESS_QUERY_INFORMATION | syscall.SYNCHRONIZE

---
>        syscall.PROCESS_QUERY_INFORMATION | syscall.SYNCHRONIZE | PROCESS_TERMINATE
@alexbrainman
Copy link
Member

Comment 1:

I am not sure about changing this. The fix will require additional authority (ability to
kill the process) for everyone who uses this api. So the api calls that use to work in
the past might fall after we change that. It is unfortunate.
Perhaps we could change (*Process).signal to try and open process again with ... +
PROCESS_TERMINATE attribute just for the syscall.TerminateProcess.
Leaving for others to decide.
Alex

Labels changed: added os-windows.

@rsc
Copy link
Contributor

rsc commented Jun 4, 2013

Comment 3:

Does it work if you get the handle from StartProcess instead of FindProcess?

@gopherbot
Copy link
Contributor Author

Comment 4 by runner.mei:

if I get the handle from StartProcess instead of FindProcess, it should work.

@gopherbot
Copy link
Contributor Author

Comment 5 by runner.mei:

if I get the handle from StartProcess instead of FindProcess, it is ok

@alexbrainman
Copy link
Member

Comment 6:

That is expected - if you start process, you get permission to kill it by default.
I think, if there are no objections, I will try to do what I suggested in #1. I won't be
able to try for a few days.
Alex

Status changed to Accepted.

@alexbrainman
Copy link
Member

Comment 7:

How about this https://golang.org/cl/9651047/ ?
Alex

Status changed to Started.

@alexbrainman
Copy link
Member

Comment 8:

This issue was closed by revision 9b2561e.

Status changed to Fixed.

@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants