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

group_spawn() on Windows doesn't allow process creation flags to be set #15

Closed
crwpenn opened this issue Feb 9, 2023 · 6 comments
Closed

Comments

@crwpenn
Copy link

crwpenn commented Feb 9, 2023

When calling group_spawn() like this...

use std::process::Command;
use std::os::windows::process::CommandExt;
use command_group::CommandGroup;

let child = Command::new("app").creation_flags(CREATE_NO_WINDOW).group_spawn();

...I expect the new process to spawn with the CREATE_NO_WINDOW flag applied and to therefore be a background process with no visible window associated. When using spawn(), that's exactly how it happens, but when using group_spawn(), the CREATE_NO_WINDOW flag is ignored and the process spawns with a visible window.

The reason for this behavior is the call to creation_flags() inside group_spawn(), which overrides any process creation flags that were previously set:

self.creation_flags(CREATE_SUSPENDED);

If I remove that line from group_spawn(), and then call it like this (to preserve the CREATE_SUSPENDED flag)...

let child = Command::new("app").creation_flags(CREATE_SUSPENDED|CREATE_NO_WINDOW).group_spawn();

...I get the expected result (process spawns with no visible window).

Question: is the CREATE_SUSPENDED flag necessary here? In my use case, at least, it seems not to be - I don't notice any difference between .creation_flags(CREATE_SUSPENDED|CREATE_NO_WINDOW) and .creation_flags(CREATE_NO_WINDOW).

If CREATE_SUSPENDED isn't necessary, could the creation_flags() call be removed from group_spawn() to allow that function to behave the same as spawn() with respect to using creation_flags() to set process creation flags?

If CREATE_SUSPENDED is necessary here, is there a way that it could be added to any previously set flags, instead of overwriting them?

@passcod
Copy link
Member

passcod commented Feb 10, 2023

Yeah, it's there to avoid a race condition where the process ends before we assign it to the group.

@passcod
Copy link
Member

passcod commented Feb 10, 2023

I'm not really sure what to do, tbh. There's no particularly good way to fetch the flags before writing them. I think the two most viable options are:

  • Make a command_group::win::CommandGroupExt trait available only on Windows that provides a group_spawn_with_flags() method, or
  • Add an unstable-internals feature that exposes some of the internals of the crate, so you can "make your own" with the flags you want.
    Let me know which you'd prefer.

@crwpenn
Copy link
Author

crwpenn commented Feb 10, 2023

Thanks for the quick response! Given that CREATE_SUSPENDED is needed, I was actually going to suggest the group_spawn_with_flags() solution, so that would be my preference.

I think the best solution would be for std::os::windows::process::CommandExt to have an add_creation_flags() method, so that you could call that instead of creation_flags(), but it looks like that design was considered and rejected.

@passcod
Copy link
Member

passcod commented Feb 28, 2023

With the linked issue #17 there's a possibility for a different API which would allow this kind of thing more elegantly

@passcod
Copy link
Member

passcod commented Mar 3, 2023

@passcod passcod closed this as completed Mar 3, 2023
@crwpenn
Copy link
Author

crwpenn commented Mar 6, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants