(Extracted from #60942 (comment) by @bcmills)
Background
On Windows, the NoInheritHandles property of syscall.SysProcAttr makes that child processes created via syscall.StartProcess (and the corresponding os and os/exec wrappers) to not inherit any handle, even those marked as inheritable.
This knob was added in #42098, in a time where all inheritable handles were implicitly inherited. A bit latter, #44011 and CL 288298 refactored how handles were inherited by child processes so that all inheritable handles that needed to be inherited would have to be explicitly listed as such.
With this last change, NoInheritHandles became obsolete and redundant, as one can achieve the same behavior by setting all standard handles (stored in ProcAttr.Files) to os.DevNull and not adding any valid handle into SysProcAttr.AdditionalInheritedHandles. Note that this is the os/exec default's behavior.
The new behavior can led to confusion, like in the following gist, where cmd.StdOut is not inherited even when explicitly marked as such.
cmd := exec.Command("./foo.exe")
cmd.Stdout = os.Stdout
cmd.SysProcAttr = &syscall.SysProcAttr{
NoInheritHandles: false,
}
cmd.Run()
I've also seen the following redundant pattern in the wild:
cmd := exec.Command(os.Args[0], rebuiltArgs...)
cmd.Stdin = nil
cmd.Stdout = nil
cmd.Stderr = nil
cmd.SysProcAttr = &windows.SysProcAttr{
NoInheritHandles: true,
}
cmd.Run()
Proposal
Given that NoInheritHandles is redundant and adds cognitive load to the os/exec API, mark it as deprecated and better document the alternative solution.
The next step after deprecating it would be to error out in case NoInheritHandles is true and a non-null handle is set to be inherited via the aforementioned APIs. I prefer to leave this for a future proposal, as doing so might break some currently valid usages.
(Extracted from #60942 (comment) by @bcmills)
Background
On Windows, the
NoInheritHandlesproperty of syscall.SysProcAttr makes that child processes created viasyscall.StartProcess(and the correspondingosandos/execwrappers) to not inherit any handle, even those marked as inheritable.This knob was added in #42098, in a time where all inheritable handles were implicitly inherited. A bit latter, #44011 and CL 288298 refactored how handles were inherited by child processes so that all inheritable handles that needed to be inherited would have to be explicitly listed as such.
With this last change,
NoInheritHandlesbecame obsolete and redundant, as one can achieve the same behavior by setting all standard handles (stored inProcAttr.Files) toos.DevNulland not adding any valid handle intoSysProcAttr.AdditionalInheritedHandles. Note that this is theos/execdefault's behavior.The new behavior can led to confusion, like in the following gist, where
cmd.StdOutis not inherited even when explicitly marked as such.I've also seen the following redundant pattern in the wild:
Proposal
Given that
NoInheritHandlesis redundant and adds cognitive load to theos/execAPI, mark it as deprecated and better document the alternative solution.The next step after deprecating it would be to error out in case
NoInheritHandlesis true and a non-null handle is set to be inherited via the aforementioned APIs. I prefer to leave this for a future proposal, as doing so might break some currently valid usages.