-
Notifications
You must be signed in to change notification settings - Fork 355
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
Handle subprocess disallowing preexec during shutdown #4879
Conversation
In Python 3.12, you cannot pass a preexec_fn to subprocess.Popen during interpreter shutdown: python/cpython#104826 This avoids the problem by giving startProgram an arg to not use a preexec_fn, and passing that all the way from anaconda's exitHandler through execWithRedirect and _run_program. Signed-off-by: Adam Williamson <[email protected]>
This works OK in openQA testing. A possible alternative is just to have the exit handler call subprocess directly; I don't think all these wrappers around it are doing much of any use on this path. But that seemed a slightly bigger change than this, so I went with this to start with. |
This (or a different fix) must be merged before the next release, because Python 3.12 has been merged into Rawhide. If a new release is made without this bug fixed, it'll be built in Rawhide by packit, and that build will be broken. (Fortunately, now gating is enabled, it should be blocked by gating rather than getting in and breaking the compose). Also, today's compose testing indicates this needs extending to the VNC shutdown code: https://openqa.fedoraproject.org/tests/2005161#step/_boot_to_anaconda/5 I'm on vacation without a laptop, so if someone else can do that, it'd be great. |
Just a note, I thought of the following alternatives constrained to changes only in
So I think this is the only way. |
This is what we do elsewhere
Needs:
Working on that. |
d2868fc
to
7f30721
Compare
/kickstart-test --testtype smoke |
Thanks! What did you think of the 'just have the exit handlers call subprocess directly' idea? |
Ah, I missed that one. I think this way is better - we have all the logging and such in place for these calls, and will likely expect the logs to look the same for this. I also feel that worrying too much about this might be premature optimization. The exit-ing code paths need modularization, soonish, and the exec* family of functions is also one of the areas that are known to need rewriting for contemporary Python. So this might not live too long anyway... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Thanks!
In Python 3.12, you cannot pass a preexec_fn to subprocess.Popen during interpreter shutdown:
python/cpython#104826
This avoids the problem by giving startProgram an arg to not use a preexec_fn, and passing that all the way from anaconda's exitHandler through execWithRedirect and _run_program.