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

syscall: add SysProcAttr.NoInheritHandles on Windows #42098

Closed
ianlancetaylor opened this issue Oct 20, 2020 · 10 comments
Closed

syscall: add SysProcAttr.NoInheritHandles on Windows #42098

ianlancetaylor opened this issue Oct 20, 2020 · 10 comments

Comments

@ianlancetaylor
Copy link
Contributor

ianlancetaylor commented Oct 20, 2020

This is a proposal for the API change in the existing CL https://golang.org/cl/261917.

The proposal is to add a new field DontInheritHandles to syscall.SysProcAttr on Windows. Setting this field will cause handles to not be inherited when creating a child process.

The negative field name is unfortunate but it's hard to see how to avoid it while preserving the desired semantics of the zero value of SysProcAttr.

@JohanKnutzen
Copy link

A bit of background to this CL

Currently, CreateProcess/CreateProcessAsUser is called with bInheritHandles set to true.

In my use case on Windows, I have a service running in session 0 that spawns a process in another session.

As per the documentation here:
https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessa

"Terminal Services: You cannot inherit handles across sessions. Additionally, if this parameter is TRUE, you must create the process in the same session as the caller."

@rsc
Copy link
Contributor

rsc commented Oct 21, 2020

We don't have any fields beginning with Dont, which is a bit difficult because of the missing apostrophe, among other things.
But we do have a bunch of fields and other names that begin with the word "No".
So maybe NoInheritHandles would be better.
(It can't be InheritHandles because we want inherit to be the default, but the default will be the zero value for this struct field.)

With NoInheritHandles, this seems fine to me.

@rsc rsc changed the title proposal: syscall: add SysProcAttr.DontInheritHandles on Windows proposal: syscall: add SysProcAttr.NoInheritHandles on Windows Oct 21, 2020
@rsc
Copy link
Contributor

rsc commented Oct 21, 2020

Does anyone object to this change?

@rsc rsc moved this from Incoming to Active in Proposals (old) Oct 21, 2020
JohanKnutzen pushed a commit to JohanKnutzen/go that referenced this issue Oct 21, 2020
@JohanKnutzen
Copy link

Thanks @rsc

I've updated the CL to rename DontInheritHandles->NoInheritHandles

@gopherbot
Copy link

Change https://golang.org/cl/261917 mentions this issue: syscall: expose bInheritHandles of CreateProcess

@alexbrainman
Copy link
Member

DontInheritHandles->NoInheritHandles SGTM.

Thank you.

Alex

@rsc
Copy link
Contributor

rsc commented Oct 28, 2020

Based on the discussion above, this seems like a likely accept.

@rsc rsc moved this from Active to Likely Accept in Proposals (old) Oct 29, 2020
@rsc
Copy link
Contributor

rsc commented Nov 4, 2020

No change in consensus, so accepted.

@rsc rsc moved this from Likely Accept to Accepted in Proposals (old) Nov 4, 2020
@rsc rsc changed the title proposal: syscall: add SysProcAttr.NoInheritHandles on Windows syscall: add SysProcAttr.NoInheritHandles on Windows Nov 4, 2020
@rsc rsc modified the milestones: Proposal, Backlog Nov 4, 2020
@alexbrainman
Copy link
Member

No change in consensus, so accepted.

@rsc does that mean we can submit

https://go-review.googlesource.com/c/go/+/261917/

?

Thank you.

Alex

@ianlancetaylor
Copy link
Contributor Author

Yes.

@golang golang locked and limited conversation to collaborators Nov 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants