Fix child process spawning on linux#773
Merged
cgutman merged 5 commits intoLizardByte:nightlyfrom Jan 19, 2023
FrogTheFrog:nightly
Merged
Fix child process spawning on linux#773cgutman merged 5 commits intoLizardByte:nightlyfrom FrogTheFrog:nightly
cgutman merged 5 commits intoLizardByte:nightlyfrom
FrogTheFrog:nightly
Conversation
FrogTheFrog
commented
Jan 14, 2023
src/process.cpp
Outdated
| boost::filesystem::path(proc.working_dir); | ||
| BOOST_LOG(info) << "Spawning ["sv << cmd << "] in ["sv << working_dir << ']'; | ||
| auto child = platf::run_unprivileged(cmd, working_dir, _env, _pipe.get(), ec); | ||
| bp::group child_group; |
Contributor
Author
There was a problem hiding this comment.
Note: I don't really like this myself that there is a need to pass a temporary group, but I don't know how to do it optionally without creating 2 function versions. The boost::child constructor is just meh...
Contributor
Author
|
Got an advice that temporary group could have had some strange behavior on Windows, so duplicating code instead :/ |
Contributor
Author
|
@cgutman Sorry to tag you like this, but could I ask for a review from you? Since I know you're more knowledgeable about |
Contributor
Author
|
It seems that ctor does not accept PID with other arguments. Since it was working this way on Windows before and it's only broken on linux, the child can be added to the group manually via method. |
cgutman
approved these changes
Jan 19, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This solves a bug introduced accidentally by #600.
When starting an attached command/process on linux, you are welcomed by this error:

It seems that for whatever reason, if your child is going to be in a process group, it has to be in it from the start boostorg/process#164 in order for the group methods to work properly.
Type of Change
.github/...)Checklist
Branch Updates
LizardByte requires that branches be up-to-date before merging. This means that after any PR is merged, this branch
must be updated before it can be merged. You must also
Allow edits from maintainers.