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

Jump to bottom

syscall: SysProcAttr{ NoInheritHandles: true } broken in 1.17 on Windows [1.17 backport] #48075

Closed
gopherbot opened this issue Aug 30, 2021 · 4 comments
Closed

syscall: SysProcAttr{ NoInheritHandles: true } broken in 1.17 on Windows [1.17 backport] #48075

gopherbot opened this issue Aug 30, 2021 · 4 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge OS-Windows
Milestone
Go1.17.3

Comments

@gopherbot
Copy link

@bcmills requested issue #48040 to be considered for backport to the next 1.17 minor release.

@gopherbot, please backport to Go 1.17. This is a regression from Go 1.16.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Aug 30, 2021
@gopherbot gopherbot mentioned this issue Aug 30, 2021
Closed
@gopherbot gopherbot added this to the Go1.17.1 milestone Aug 30, 2021
@thanm thanm modified the milestones: Go1.17.1, Go1.17.2 Sep 9, 2021
@gopherbot
Copy link
Author

Change https://golang.org/cl/350416 mentions this issue: [release-branch.go1.17] syscall: do not use handle lists on windows when NoInheritHandles is true

@zx2c4
Copy link
Contributor

zx2c4 commented Sep 17, 2021

CC @JohanKnutzen in case you wanted to track its inclusion in 1.17.y.

@JohanKnutzen
Copy link

@zx2c4 Excellent. Thanks for the patch. Looking forward to this being rolled out.

@heschi heschi added CherryPickApproved Used during the release process for point releases and removed CherryPickCandidate Used during the release process for point releases labels Oct 6, 2021
@mknyszek mknyszek modified the milestones: Go1.17.2, Go1.17.3 Oct 7, 2021
@seankhliao seankhliao mentioned this issue Oct 18, 2021
Closed
@gopherbot
Copy link
Author

Closed by merging 11b64b4 to release-branch.go1.17.

gopherbot pushed a commit that referenced this issue Oct 25, 2021
@zx2c4 @dmitshur
[release-branch.go1.17] syscall: do not use handle lists on windows w…
11b64b4 
…hen NoInheritHandles is true

If NoInheritHandles is passed, then we shouldn't attempt to do anything
with handle lists. Otherwise CreateProcess fails with invalid param,
because it's being told both to not inherit handles and to inherit
certain handles.

This commit fixes that by using the same logic for handle lists as it
does for enabling or disabling handle inheritance. It also adds a test
to make sure this doesn't regress again.

Updates #48040
Fixes #48075

Change-Id: I507261baeec263091738ab90157a991d917dc92f
Reviewed-on: https://go-review.googlesource.com/c/go/+/350416
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Patrik Nyblom <pnyb@google.com>
@golang golang locked and limited conversation to collaborators Oct 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge OS-Windows
Projects
None yet
Milestone
Go1.17.3
Development

No branches or pull requests
7 participants
@zx2c4 @JohanKnutzen @mknyszek @dmitshur @gopherbot @thanm @heschi