-
Notifications
You must be signed in to change notification settings - Fork 18k
proposal: os: add (*Process).Handle method #70352
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
Comments
Related Issues Related Code Changes
(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.) |
Thanks @kolyshkin for tagging me. This proposal looks good me from user perspective. I don't need to check pidfd in my side. :) |
It should probably return an error instead of a boolean since the operation is (I think?) a system call and could in principle fail for more reasons than ENOTSUP. |
The pidfd is already used internally by Are you suggesting we need to save the error getting pidfd in I've added |
I see. Thanks for clarifying.
No, what you have seems fine.
That seems appropriate. |
Proposal
Add a new
Handle
method foros.Process
, which returns a process handle, if available, and a boolean flag telling if a handle is valid. On Linux, handle is a pidfd (file descriptor referring to the process). On Windows, handle is a handle to the process.Context
This is a continuation of #62654, in particular its item 6, described in there as:
Also, a similar thing was proposed earlier here by @prattmic:
Use cases
1. Check if pidfd is being used on Linux.
Since Go 1.23, pidfd is used for os.Process-related operations instead of pid, if supported by the Linux kernel. This includes
os.StartProcess
andos.FindProcess
(they obtain a pidfd), as well as(*Process).Wait
,(*Process).Signal
, and(*Process).Kill
(they use pidfd). The main benefit of pidfd in the use cases above is a guarantee we're referring to the same process (i.e. there's no pid reuse issue).However, since this is done in a fully transparent way, there is no way for a user to know if pidfd is being used or not. Some programs implement some protection against pid reuse (for example,
runc
andcri-o
obtain and check process start time from/proc/<pid>/stat
). They can benefit from being able to know if Go is using pidfd internally.Another example is containerd which relies on Go 1.23 using pidfd internally, but since there's no way to check they had to recreate all the functionality checking for pidfd support here (which is still not 100% correct since the checks are slightly different from those in Go's checkPidfd, and Go checks may change over time ). Cc @fuweid.
With the proposed interface, a user can easily check if pidfd is being used:
2. Obtain a pidfd for additional direct use.
Aside from use cases already covered by existing
os.Process
methods, pidfd can also be used to:Other use cases may emerge in the future.
Currently, the only way to obtain a pidfd on Linux is to execute a new process (via
os.StartProcess
oros/exec
) with process'Attr.SysAttr.PidFD
field set. This works if we're starting the process, but not in any other case (someone else starts a process for us, or it is already running).Questions / discussion points
1. What are (could be) the additional direct use cases of Windows process handle?
A few are listed here. Apparently some git grep (GetPriorityClass, SetPriorityClass, AssignProcessToJobObject) are implemented in golang.org/x/sys/windows.
2. Should a duplicate of a handle be returned, or the original handle?
Return the original one, ensuring that
Handle
documentation describes when pidfd may become invalid.Arguments against duplicated handle:
Release
method and a proper finalizer;Arguments for duplicated handle:
3. Should Handle return
*os.File
rather thanuintptr
?Raw handle makes more sense in this case, and the finalizer set by
NewFile
does not make sense if the original handle is returned. Also, this won't work for Windows handle is not a file.4. Should this be Linux-specific?
Probably not. Since we have a boolean flag returned, we can implement it for all platforms and return
0, false
for those that do not support process handle (other than Windows and Linux).The text was updated successfully, but these errors were encountered: