Issue on the interaction between CreationFlags and JobObject wrapper

[agallardol]

Hi, thanks for the library. I'm here because I was fixing an issue on another library and besides the fix it wasn't working because I tried to use CreationFlags but it isn't working. If I try to add CREATE_NO_WINDOW it's never assigned to the process itself.

I think this issue is very important for apps that really requires spawn new processes with no visible windows like MCP Dockmaster which under the hood uses rmcp the official sdk for MCPs in rust, which under the hood uses this library.

Context
On the JobObject implementation, we try to merge any previously added CreationFlag with the CREATE_SUSPENDED flag. which makes sense, even with the documented rule about use CreationFlags wrapper before the JobObject wrapper.

Issue
The spawn method temporarily takes ownership of self.wrappers using std::mem::take, which means that from there any call to self.wrappers and/or core.wrappers will return an empty array.

process-wrap/src/generic_wrap.rs

Lines 117 to 127 in b83fec4

	pub fn spawn(&mut self) -> ::std::io::Result<Box<dyn $childer>> {
	let mut command = ::std::mem::replace(&mut self.command, <$command>::new(""));
	let mut wrappers = ::std::mem::take(&mut self.wrappers);
	let res = self.spawn_inner(&mut command, &mut wrappers);
	self.command = command;
	self.wrappers = wrappers;
	res
	}

After this mem take we call the self.spawn_inner() which uses the self instance. If at any point of this method and his sub functions we try to use core.has_wrapper and/or core.get_wrapper we will find no wrappers because we replaced temporarily the wrappers instance with an empty array.

process-wrap/src/tokio/job_object.rs

Lines 39 to 50 in b83fec4

	impl TokioCommandWrapper for JobObject {
	#[cfg_attr(feature = "tracing", instrument(level = "debug", skip(self)))]
	fn pre_spawn(&mut self, command: &mut Command, core: &TokioCommandWrap) -> Result<()> {
	let mut flags = CREATE_SUSPENDED;
	#[cfg(feature = "creation-flags")]
	if let Some(CreationFlags(user_flags)) = core.get_wrap::<CreationFlags>() {
	flags |= *user_flags;
	}
	command.creation_flags(flags.0);
	Ok(())
	}

How to reproduce it
Try to add the CREATE_NO_WINDOW flag following the docs instructions

    let child = TokioCommandWrap::with_new("echo", |command| {
        command.arg("hello world");
    })
    .wrap(CreationFlags(CREATE_NO_WINDOW))
    .wrap(JobObject)
    .spawn()?;

And then check the code never reachs this conditional

process-wrap/src/tokio/job_object.rs

Lines 44 to 46 in b83fec4

	if let Some(CreationFlags(user_flags)) = core.get_wrap::<CreationFlags>() {
	flags |= *user_flags;
	}

So the flags are never merged and your process ends having just the CREATE_SUSPENDED flag

I think is very important because to be able to use this library in apps where we need to spawn new processes silently, with no visible windows.

[passcod]

You're correct and I've got a fix in development, but the refactor needed isn't quite as straightforward as I thought. Ongoing...

[agallardol]

aaah yeah I though that. At the begining I tried to fix it myself to generate a PR in place of an issue but I did notice that the right fix requires some refactors deeply tied to how the library is built. Thanks for your help!

[TheLostLambda]

I'm also now running into this issue — are there any updates / workarounds that people have found?

EDIT: My current workaround is doing the exact opposite of what the docs say to do, and manually adding in that CREATE_SUSPENDED flag.

It should be noted, however, that I don't even remotely understand enough about Windows to know if this is correct or how to even check, but it's behaving as I'd expect in the immediate moment?

let mut child = StdCommandWrap::from(command)
    .wrap(JobObject)
    .wrap(CreationFlags(CREATE_NO_WINDOW | CREATE_SUSPENDED))
    .spawn()?;

[passcod]

Yeah as long as you add the suspended flag manually then you can use that. I'll add it to the docs as a workaround.

[bzld-acn]

Hi, is there any progress on #20?
The root cause also seems to break the JobObject let kill_on_drop = core.has_wrap::<KillOnDrop>() feature, but for that I don't see a workaround like for the creation flags...?

[passcod]

I'm still stuck on the refactor but will tackle it anew once #22 is done.

[TheLostLambda]

Sorry for holding that up! I should be done with the last bits in the next few days — I've mostly got the doc tests done.

[TheLostLambda]

Sorry for holding that up! I should be done with the last bits in the next few days — I've mostly got the doc tests done.

That was definitely more than a few days, but I think all of the PRs / issues I was holding up are ready for @passcod now!