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: FindProcess should document how to test if a process is alive since on UNIX it always returns a process #34396

Open
srowles opened this issue Sep 19, 2019 · 8 comments

Comments

@srowles
Copy link

@srowles srowles commented Sep 19, 2019

What version of Go are you using (go version)?

$go version go1.12.6 linux/amd64

Does this issue reproduce with the latest release?

Yes, Documentation issue, see live doc at:

https://golang.org/pkg/os/#FindProcess

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
$ go env
GOARCH="amd64"
GOHOSTARCH="amd64"
GOHOSTOS="linux"

What did you do?

Tried to find out if a process exists using os.FindProcess()

Read the documentation to find that the FindProcess call always returns a nil error on unix systems so it doesn't really "find" the process:

On Unix systems, FindProcess always succeeds and returns a Process for the given pid, regardless of whether the process exists.

Found closed documentation ticket saying you can send a signal to find if the process really exists or not:

#14146 (comment)

What did you expect to see?

More helpful information in the documentation about how someone might actually test for a process existing, for example as a starter on Linux:

	// error is always nil on Unix systems, just creates a process object
	p, _ := os.FindProcess(pid)
	// send SIGCONT (on Linux) which is a safe signal to the process to test if it exists
	err = p.Signal(syscall.SIGCONT)

What did you see instead?

No help without how the user might find out that a process really exists. Just a comment that it doesn't actually tell you if the process exists:

On Unix systems, FindProcess always succeeds and returns a Process for the given pid, regardless of whether the process exists.
@toothrot
Copy link
Contributor

@toothrot toothrot commented Sep 20, 2019

Hey @srowles, thanks for taking the time to file this.

I am not totally confident if the suggested example is within scope for os.FindProcess. /cc @bradfitz who chimed in last time this issue came up. #14146 (comment)

@toothrot toothrot added this to the Go1.14 milestone Sep 20, 2019
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Sep 20, 2019

I'm not sure SIGCONT is necessarily safe to send. It might be stopped on purpose, and then you just resumed it.

Maybe we need some sort of portable os.(*Process).State method? But then it'd be tempting to use ProcessState as its return type, which might not be a great fit, despite the name. Notably, its ExitCode method says:

ExitCode returns the exit code of the exited process, or -1 if the process hasn't exited or was terminated by a signal.

That doesn't really let you know if it's still running or not. Perhaps we could add a new Running() bool method.

Then what you could do:

    p, _ := os.FindProcess(123)
    if p != nil && p.State().Running() {
              ....
    }
}

/cc @ianlancetaylor

@srowles
Copy link
Author

@srowles srowles commented Sep 20, 2019

I'd be happy with a new method, that would be preferable to the workaround of sending signals to try and determine the process state.

@odeke-em odeke-em changed the title os: documentation for os.FindProcess could be more helpful os: FindProcess should document how to test if a process is alive since on UNIX it always returns a process Sep 25, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@odeke-em
Copy link
Member

@odeke-em odeke-em commented Dec 16, 2019

Given the number of times this has come up (here and on forums such as Stackoverflow), the suggestions for sending signal 0 which is in the user's way and requires them to go Googling, this issue is haunting me a little and I can't sleep thinking about it. I think that perhaps we could do more so os.FindProcess should do some more work on the UNIXes instead of blindly returning an unchecked process.

I noticed that the now defunct CL https://codereview.appspot.com/7392048/ by Akshat Kumar which timed out had some of the changes that I was just thinking.

My suggestions here would be that:
a) On Linux, FreeBSD and Plan 9, we stat the /proc/target_pid file as per Linux /proc FreeBSD /proc Plan9 /proc
b) On Darwin/NetBSD, we can do a sysctl(CTL_KERN, KERN_PROC, KERN_PROC_PID, target_pid)
c) For other UNIXes that we haven't covered, we can seek contributions

and with that, perhaps we can turn this issue instead into a change/proposal for Go1.15.

Kindly pinging @ianlancetaylor @mdempsky @randall77 as well to ask if this might seem like something viable.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Dec 16, 2019

To me it seems like kind of an impossible problem. The program could test that the process exists, and then the process could exit, and then the program could try to do something with the process, but it wouldn't work. Or it would affect an entirely different process that was reusing the process ID. In general doing arbitrary things with a PID isn't safe on Unix systems.

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Dec 16, 2019

Not portable but Linux has pidfd: https://lwn.net/Articles/794707/

@odeke-em
Copy link
Member

@odeke-em odeke-em commented Dec 18, 2019

To me it seems like kind of an impossible problem. The program could test that the process exists, and then the process could exit, and then the program could try to do something with the process, but it wouldn't work. Or it would affect an entirely different process that was reusing the process ID. In general doing arbitrary things with a PID isn't safe on Unix systems.

Indeed Ian, this is tricky if we are sending a signal to the process, but regardless of whether this might be racy or unsafe on UNIX systems, I think the same thing can happen on Windows but we strive hard to provide a FindProcess implementation for the user :) What we can do perhaps is document this unsafe behavior.

Not portable but Linux has pidfd: https://lwn.net/Articles/794707/

Cool, thanks for the reference, Brad!

@gopherbot
Copy link

@gopherbot gopherbot commented Dec 18, 2019

Change https://golang.org/cl/211801 mentions this issue: os: implement FindProcess on Darwin instead of noop

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.