-
Notifications
You must be signed in to change notification settings - Fork 18.7k
Description
What version of Go are you using (go version)?
go version go1.17 windows/amd64
Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (go env)?
go env Output
$ go env set GO111MODULE= set GOARCH=amd64 set GOBIN= set GOCACHE=C:\Users\knutz\AppData\Local\go-build set GOENV=C:\Users\knutz\AppData\Roaming\go\env set GOEXE=.exe set GOEXPERIMENT= set GOFLAGS= set GOHOSTARCH=amd64 set GOHOSTOS=windows set GOINSECURE= set GOMODCACHE=C:\Users\knutz\go\pkg\mod set GONOPROXY= set GONOSUMDB= set GOOS=windows set GOPATH=C:\Users\knutz\go set GOPRIVATE= set GOPROXY=https://proxy.golang.org,direct set GOROOT=C:\Program Files\Go set GOSUMDB=sum.golang.org set GOTMPDIR= set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64 set GOVCS= set GOVERSION=go1.17 set GCCGO=gccgo set AR=ar set CC=gcc set CXX=g++ set CGO_ENABLED=1 set GOMOD=C:\code\fd\agent_svc\go.mod set CGO_CFLAGS=-g -O2 set CGO_CPPFLAGS= set CGO_CXXFLAGS=-g -O2 set CGO_FFLAGS=-g -O2 set CGO_LDFLAGS=-g -O2 set PKG_CONFIG=pkg-config set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\knutz\AppData\Local\Temp\go-build1048178528=/tmp/go-build -gno-record-gcc-switches
What did you do?
package main
import (
"log"
"os/exec"
"syscall"
)
func main() {
cmd := exec.Command("notepad.exe")
cmd.SysProcAttr = &syscall.SysProcAttr{
NoInheritHandles: true,
}
err := cmd.Start()
log.Printf("err: %v", err)
}
What did you expect to see?
2021/08/28 11:59:56 err: <nil>
What did you see instead?
2021/08/28 12:00:02 err: fork/exec C:\Windows\system32\notepad.exe: The parameter is incorrect.
The problem
NoInheritHandles was added in 1.16:
1e3b535#diff-ec673c10a75fe2d2faa9c788e03823294b263c68cc3de50f4a0865a53e28f3a3
Between that commit and 1.17 there was a series of commits by @zx2c4:
https://github.com/golang/go/commits/master/src/syscall/exec_windows.go
I don't fully understand the reasoning behind the changes but they are documented here: 2d76081#diff-ec673c10a75fe2d2faa9c788e03823294b263c68cc3de50f4a0865a53e28f3a3
These changes are not compatible with NoInheritHandles because they seem to add handles explicitly on this line:
go/src/syscall/exec_windows.go
Line 395 in f4cd001
| err = updateProcThreadAttribute(si.ProcThreadAttributeList, 0, _PROC_THREAD_ATTRIBUTE_HANDLE_LIST, unsafe.Pointer(&fd[0]), uintptr(len(fd))*unsafe.Sizeof(fd[0]), nil, nil) |
Windows doesn't accept adding handles + NoInheritHandles which I guess makes sense.
How to solve this?
I'm not sure what the new commits do by @zx2c4 or how they are expected to act when NoInheritHandles is true. I would assume that no manipulation of handles/adding handles should happen. You can toggle between a non-working/working state by changing this:
// Do not accidentally inherit more than these handles.
if len(fd) > 0 {
err = updateProcThreadAttribute(si.ProcThreadAttributeList, 0, _PROC_THREAD_ATTRIBUTE_HANDLE_LIST, unsafe.Pointer(&fd[0]), uintptr(len(fd))*unsafe.Sizeof(fd[0]), nil, nil)
if err != nil {
return 0, 0, err
}
}
to
// Do not accidentally inherit more than these handles.
if len(fd) > 0 && !sys.NoInheritHandles {
err = updateProcThreadAttribute(si.ProcThreadAttributeList, 0, _PROC_THREAD_ATTRIBUTE_HANDLE_LIST, unsafe.Pointer(&fd[0]), uintptr(len(fd))*unsafe.Sizeof(fd[0]), nil, nil)
if err != nil {
return 0, 0, err
}
}
in this file:
go/src/syscall/exec_windows.go
Line 393 in f4cd001
| // Do not accidentally inherit more than these handles. |
@zx2c4 - What do you think? Would it be possible for you to fix this? Otherwise I can submit a PR with the above quick fix but I'm not sure how that influences the rest.