The STDIO command creation flags still not working

[Pluto-Y]

Describe the bug
I create STDIO client using #243 example code, but it still not working in release mode

The code i use:

#[tauri::command]
async fn get_tools(command: &str, args: Vec<String>) -> Result<String, String> {
    let envs = get_login_shell_env()
        .await
        .map_or(HashMap::default(), |envs| envs);

    let mut cmd = process_wrap::tokio::TokioCommandWrap::with_new(command, |cmd| {
        cmd.args(args).envs(&envs);
    });

    cmd.wrap(process_wrap::tokio::KillOnDrop);
    #[cfg(target_os = "windows")]
    cmd.wrap(process_wrap::tokio::CreationFlags(
        windows::Win32::System::Threading::CREATE_NO_WINDOW,
    ));

    let transport = TokioChildProcess::new(cmd).unwrap();
    let service = ().serve(transport).await.unwrap();

    let tools = service.list_tools(Default::default()).await.unwrap();
    log::info!("Available tools: {tools:#?}");

    tokio::spawn(async move {
        // mock service still running for a while
        tokio::time::sleep(Duration::from_secs(60)).await;
        if let Err(err) = service.cancel().await {
            log::error!("Failed to cancel service: {err}");
            return;
        };

        log::info!("Service cancelled after timeout");
    });

    Ok(serde_json::to_string(&tools).unwrap_or_else(|_| {
        log::error!("Failed to serialize tools");
        "[]".to_string()
    }))
}

When the running service is created, the system terminal window still be created

PS: when i try to comment the code command_wrap.wrap(process_wrap::tokio::JobObject); in TokioChildProcess, it can be work

To Reproduce
Steps to reproduce the behavior:

1. using the above codes and run in release mode

Expected behavior
A clear and concise description of what you expected to happen.

DO NOT create a terminal window

Logs
If applicable, add logs to help explain your problem.

Additional context
Add any other context about the problem here.

If you need demo, i can supply it

[Pluto-Y]

@jokemanfire any suggestion or plan?

[jokemanfire]

I will see it this weekend.

[4t145]

Just have some experiment and find out that it about the wrap order:

Let's check this minimal example:

#![cfg_attr(not(debug_assertions), windows_subsystem = "windows")]

use std::error::Error;

use winit::application::ApplicationHandler;
use winit::event::{ElementState, MouseButton, WindowEvent};
use winit::event_loop::{ActiveEventLoop, EventLoop};
use winit::window::{Window, WindowId};

#[derive(Default, Debug)]
struct App {
    window: Option<Window>,
}

impl ApplicationHandler for App {
    fn resumed(&mut self, event_loop: &ActiveEventLoop) {
        if let Ok(window) = event_loop.create_window(Default::default()) {
            self.window.replace(window);
        }
    }

    fn window_event(&mut self, event_loop: &ActiveEventLoop, _: WindowId, event: WindowEvent) {
        match event {
            WindowEvent::CloseRequested => {
                event_loop.exit();
            }
            WindowEvent::RedrawRequested => {
                let window = self
                    .window
                    .as_ref()
                    .expect("redraw request without a window");
                window.pre_present_notify();
            }
            WindowEvent::MouseInput {
                button: MouseButton::Left,
                state: ElementState::Released,
                ..
            } => {
                let cmd = process_wrap::tokio::TokioCommandWrap::with_new("powershell", |cmd| {
                    cmd.arg("echo $env:path");
                })
                .wrap(process_wrap::tokio::CreationFlags(
                    windows::Win32::System::Threading::CREATE_NO_WINDOW,
                ))
                .wrap(process_wrap::tokio::JobObject)
                .spawn()
                .unwrap();
                tokio::spawn(async move {
                    let output = Box::<(dyn Future<Output = _> + Send + 'static)>::into_pin(
                        cmd.wait_with_output(),
                    )
                    .await
                    .unwrap();
                    let usage = std::str::from_utf8(&output.stdout).unwrap();
                    println!("path:\n{}", usage);
                });
            }
            _ => (),
        }
    }
}
#[tokio::main]
async fn main() -> Result<(), Box<dyn Error>> {
    let event_loop = EventLoop::new()?;
    let mut app = App::default();
    event_loop.run_app(&mut app)?;
    Ok(())
}

notice those two line

                .wrap(process_wrap::tokio::CreationFlags(
                    windows::Win32::System::Threading::CREATE_NO_WINDOW,
                ))
                .wrap(process_wrap::tokio::JobObject)

If you wrap CREATE_NO_WINDOW first, child process will create terminal window. Then if you exchange their position, it will not.

And I had a look on the document of CreationFlag, which says:

Shim wrapper which sets Windows process creation flags.

This wrapper is only available on Windows.

It exists to be able to set creation flags on a Command and also store them in the wrapper, so that they're no overwritten by other wrappers. Notably this is the only way to use creation flags and the JobObject wrapper together.

When both CreationFlags and JobObject are used together, CreationFlags must come first.

Solution

So, may we should remove those two line and let users determine what to wrap in.

        #[cfg(unix)]
        command_wrap.wrap(process_wrap::tokio::ProcessGroup::leader());
        #[cfg(windows)]
        command_wrap.wrap(process_wrap::tokio::JobObject);
        let (child, (child_stdout, child_stdin)) = child_proces

cc @lucasfernog-crabnebula

[Pluto-Y]

Yes, that it's what i try to fix. i just add command_wrap.wrap(process_wrap::tokio::JobObject); this line in the outside of the rmcp and then use rmcp TokioChildProcess constructor, it alsow works.

And I check the documents of the progress_wrap

Child wraps are in order: you may end up with a Foo(Bar(Child)) or a Bar(Foo(Child)) depending on if .wrap(Foo).wrap(Bar) or .wrap(Bar).wrap(Foo) was called. If your functionality is order-dependent, make sure to specify so in your documentation! By default does nothing: no wrapping is performed and the input child is returned as-is.

It seems rmcp should wrap process_wrap::tokio::JobObject at first? Not at end.

It needs your review for this. @4t145

[lucasfernog-crabnebula]

perhaps we should instruct users to add their own .wrap(process_wrap::tokio::JobObject) on their end when they want to add the creation flags? it's either that or we need to redesign the API a bit