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

kernel: change SyExecuteProcess to use posix_spawn #4799

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fingolfin
Copy link
Member

@ChrisJefferson does this work under cygwin? If so, does it fix the bug where Process is changing the working dir for the parent process?

The patch is not ready for merging in any case:

  • handling of SIGCHLD not integreated
  • moving SyExecuteProcess to iostream.c is kind of a hack, but for the sake of quickly experimenting I didn't want to bother with trying to come up with a "clean" solution.

@fingolfin fingolfin added topic: kernel release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels Feb 27, 2022
@fingolfin fingolfin added the os: windows Issues and PRs that are (at least partially) specific to Windows label Feb 28, 2022
@ChrisJefferson
Copy link
Contributor

This works on cygwin, and stops the directory changing behaviour

@fingolfin fingolfin force-pushed the mh/SyExecuteProcess-posix_spawn branch from 2f788fa to e1079b6 Compare April 14, 2022 21:30
@fingolfin fingolfin changed the title WIP kernel: change SyExecuteProcess to use posix_spawn kernel: change SyExecuteProcess to use posix_spawn Apr 14, 2022
@fingolfin fingolfin marked this pull request as ready for review April 14, 2022 21:30
@fingolfin
Copy link
Member Author

I decided to just make the move of SyExecuteProcess to iostream.c permanent, there seems little point in trying to abstract this away and it makes code sharing much easier.

return -1;
}

// wait for some action
Copy link
Member Author

@fingolfin fingolfin Apr 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: the "classic" code using fork also has some logic to "ignore a CTRL-C". And it disables our SIGCHLD handler temporarily. We may need to add one or both back here.

@fingolfin fingolfin force-pushed the mh/SyExecuteProcess-posix_spawn branch from 3b4905d to 8c0b2d8 Compare February 15, 2024 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
os: windows Issues and PRs that are (at least partially) specific to Windows release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes topic: kernel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants