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

os: follow-up pidfd fixes [freeze exception] #67826

Closed
prattmic opened this issue Jun 4, 2024 · 4 comments
Closed

os: follow-up pidfd fixes [freeze exception] #67826

prattmic opened this issue Jun 4, 2024 · 4 comments
Labels
NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone

Comments

@prattmic
Copy link
Member

prattmic commented Jun 4, 2024

This is a freeze exception request for feedback for https://go.dev/cl/588675. This CL is fixing #67634, #67640, and #67641, which are issues with the new pidfd support added in https://go.dev/cl/570036 and https://go.dev/cl/570681.

Since these are regressions, I don't think a freeze exception is strictly necessary, however the fix is a fairly invasive overhaul of the internals of os.Process, which I'd say has a medium risk of introducing new problems. The alternative to submitting this fix now would be to revert the pidfd changes and try again when the dev cycles opens again.

cc @golang/release @kolyshkin

@prattmic prattmic added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jun 4, 2024
@kolyshkin
Copy link
Contributor

I'm still wrapping my head around CL 588675 but overall it makes sense. At the very least #67634 is definitely a bad regression that could hit users hard.

@dmitshur dmitshur added this to the Go1.23 milestone Jun 5, 2024
@prattmic
Copy link
Member Author

prattmic commented Jun 5, 2024

At the very least #67634 is definitely a bad regression that could hit users hard.

Yes, this is the one issue where I am aware of actual failures (some tests inside Google fail). The others are just problems I noticed when fixing the first.

@dmitshur
Copy link
Contributor

dmitshur commented Jun 5, 2024

We reviewed this in a release meeting, and agreed to approve its "freeze exception" bit. Thanks for letting us know.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Jun 11, 2024
@prattmic
Copy link
Member Author

https://go.dev/cl/588675 is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
Status: Done
Development

No branches or pull requests

4 participants