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

Spawn new process with creation_flag(0x00000010) to get a new console on win7, the new process get IO.Exception. #105203

Closed
TYPEmber opened this issue Dec 3, 2022 · 7 comments
Labels
C-bug Category: This is a bug. O-windows Operating system: Windows

Comments

@TYPEmber
Copy link

TYPEmber commented Dec 3, 2022

I tried this code:

use std::os::windows::process::CommandExt;

    let mut res = std::process::Command::new(args.path)
        // create a new console for child process
        .creation_flags(0x00000010)
        .current_dir(dirp)
        .spawn()
        .unwrap();

It works well on win10, the new process has an individual console for stdout & stderr, keyboard for stdin.
But the same binary can't get the same result on win7, the new process throw a IO.Exception.

I tried to delete a line in std/src/sys/windows/process.rs line: 258 fn spawn()

// si.dwFlags = c::STARTF_USESTDHANDLES;

Which let the windows to set a default stdio for new process.

In this case, the binary works well on both win7 & win10.

Meta

Both stable and nightly version of the compiler has the same situation.

rustc --version --verbose:

rustc 1.65.0 (897e37553 2022-11-02)
binary: rustc
commit-hash: 897e37553bba8b42751c67658967889d11ecd120
commit-date: 2022-11-02
host: x86_64-pc-windows-msvc
release: 1.65.0
LLVM version: 15.0.0

rustc 1.67.0-nightly (c090c6880 2022-12-01)
binary: rustc
commit-hash: c090c6880c0183ba248bde4a16e29ba29ac4fbba
commit-date: 2022-12-01
host: x86_64-pc-windows-msvc
release: 1.67.0-nightly
LLVM version: 15.0.4
Backtrace

<backtrace>

@TYPEmber TYPEmber added the C-bug Category: This is a bug. label Dec 3, 2022
@workingjubilee workingjubilee added the O-windows Operating system: Windows label Dec 5, 2022
@workingjubilee
Copy link
Member

But the same binary can't get the same result on win7, the new process throw a IO.Exception.

Can you describe the error more fully and also the intended use-case? The behavior you describe is at conflict with the behavior of Command in general, as Command::spawn() tends to inherit from the parent so that everything else it does is functional, so removing that flag unconditionally is not the right fix. It may make sense to remove it conditionally, but that requires understanding what you're trying to do here.

@TYPEmber
Copy link
Author

TYPEmber commented Dec 5, 2022

But the same binary can't get the same result on win7, the new process throw a IO.Exception.

Can you describe the error more fully and also the intended use-case? The behavior you describe is at conflict with the behavior of Command in general, as Command::spawn() tends to inherit from the parent so that everything else it does is functional, so removing that flag unconditionally is not the right fix. It may make sense to remove it conditionally, but that requires understanding what you're trying to do here.

Thanks for your replying, I want to make a process supervisor, which needs to spawn child process that has an individual console for input and output. As a result, I set the creation_flag as 0x00000010 [https://learn.microsoft.com/en-us/windows/win32/procthread/process-creation-flags].

The error is the child process will throw an IO.Exception (when my csharp console application try to console.write("sth.")) on win7, but works well on win10, with that creation_flag. And it works well on both win7 & win10 without that creation_flag, but the parent and child process share the parent's console.

I checked the windows api's document and the source code of Command::spawn(), It seems like, if I do not need pipe output, Command::spawn() just inherit the parent's setting of stdio, which in my case, just the default stdio. So, I removed that flag, let windows set the default stdio for child process, for a temporarily fix in my use-case.

@workingjubilee
Copy link
Member

workingjubilee commented Dec 5, 2022

Ah, so the IO.Exception is not immediate if the process spawns without doing anything (just looping for a bit, say) but is caused by the application then trying to write to the console, and the issue bubbles up through the C# runtime? What's the error message? (at this point I don't think it necessarily matters, but I am curious)

It seems there are other issues touching on adjacent concerns, where the implicit defaults of Command::spawn are a bit of a mismatch and messing around here can get a bit confusing:

The behavior you want seems reasonable, but it may be more correct to expose an explicit API for the desired behavior instead of implicitly reasoning based on the content of the creation flags.

@TYPEmber
Copy link
Author

TYPEmber commented Dec 5, 2022

The error message is System.UnauthorizedAccessException: Access is denied. (Exception from HRESULT: 0x80070005 (E_ACCESSDENIED)).

Ah, so the IO.Exception is not immediate if the process spawns without doing anything (just looping for a bit, say)

Yes.

but is caused by the application then trying to write to the console

I'm really sorry, after further test, I get this error from code Console.ReadLine();. And Console.WriteLine("sth.") just outputs nothing to the console, it won't throw any error.

the issue bubbles up through the C# runtime?

Yes.

Thank you very much for these replies, I do agree there needs to have more explicit API for spawn a new process.

@ChrisDenton
Copy link
Member

I think a full fix here is to have a special new std::process::Stdio option. Currently there's inherit (use the parent's stdio), null (the null device) and pipe. We'd need something like a None option which allows not setting any stdio. However, this would need to be Windows only as there's not really a cross-platform equivalent.

@ChrisDenton
Copy link
Member

Ok, now that #103459 has merged, you can do this on the latest nightly but it isn't pretty because there's not yet a proper API for it. I've adapted the code in the OP to add a default_stdio function that should hopefully allow the child process to use its default stdio handles.

use std::os::windows::io::FromRawHandle;
use std::os::windows::process::CommandExt;
use std::process::{Command, Stdio};

fn default_stdio() -> Stdio {
    unsafe { Stdio::from_raw_handle(std::ptr::null_mut()) }
}

    let mut res = Command::new(args.path)
        // create a new console for child process
        .creation_flags(0x00000010)
        .current_dir(dirp)
        // use the default handles
        .stdin(default_stdio())
        .stdout(default_stdio())
        .stderr(default_stdio())
        .spawn()
        .unwrap();

In case you aren't aware, you can use rustup install nightly to get or update to the latest nightly version and cargo +nightly run to run the program.

@TYPEmber
Copy link
Author

wow, thank you very much!

It do works well on both win7 & win10.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. O-windows Operating system: Windows
Projects
None yet
Development

No branches or pull requests

3 participants