-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Parent process with "windows" subsystem can't properly create console child process #101645
Comments
I think this will need a T-libs-api decision as the current documentation for
I would guess that this "inherit by default" is why it's implemented the way it is. But perhaps there's some wiggle room with what it means to inherit handles that don't exist so we can change it. In either case it would be good to have some way to use platform default behaviour here. @rustbot label -T-libs +T-libs-api |
That's exactly my point. If there is nothing to inherit in the first place, just inherit nothing. The current behavior ( |
Basically, currently there are only two options when handling the stdout/stderr: inherit or pipe. I think there should be a third option: isolated - make the child handle its console by itself. For example, if user calls |
Not always. Parent process can still set those handles to something other than if si.hStdInput != 0 || si.hStdOutput != 0 || si.hStdError != 0 {
si.dwFlags |= c::STARTF_USESTDHANDLES;
} This solution would make behaviour different depending on how you launch your app. Also, you can explicitly ask to create new console with |
I can't verify your statements since I don't have IntelliJ atm, but even it's true, isn't it IntelliJ setting up the console with maybe some special debugging API? How to you modify the code of P to achieve the same? And realistically speaking, do people actually do that with GUI apps?
I think in this case, it's impossible to be 100% sure where the user wants the output at. New option might be the way to go. |
@kryptan Your prediction was dead-on, as it came true in #105203. While we can do this, as #103459 is proposing, it seems best here that we create a function, or set of functions, that explicitly handle this kind of situation rather than guessing. |
Even with new APIs, we still need reasonable default behaviour. And the Command API is documented as inheriting handles by default. However, currently the API neglects the "no handle set" case entirely as that's not really a thing in POSIX or ISO C (well, you can close stdio but that's usually considered a Bad Thing unless you're being very very careful). |
Pass on null handle values to child process Fixes rust-lang#101645 In Windows, stdio handles are (semantically speaking) `Option<Handle>` where `Handle` is a non-zero value. When spawning a process with `Stdio::Inherit`, Rust currently turns zero values into `-1` values. This has the unfortunate effect of breaking console subprocesses (which typically need stdio) that are spawned from gui applications (that lack stdio by default) because the console process won't be assigned handles from the newly created console (as they usually would in that situation). Worse, `-1` is actually [a valid handle](https://doc.rust-lang.org/std/os/windows/io/struct.OwnedHandle.html) which means "the current process". So if a console process, for example, waits on stdin and it has a `-1` value then the process will end up waiting on itself. This PR fixes it by propagating the nulls instead of converting them to `-1`. While I think the current behaviour is a mistake, changing it (however justified) is an API change so I think this PR should at least have some input from t-libs-api. So choosing at random... r? `@joshtriplett`
I tried this code:
App P:
App C:
Build this in Windows.
I expected to see this happen:
Since
P
has "windows" subsystem, it will create no console window (i.e. no conhost.exe process). SinceC
is a console app, when the process is created, I expect to see one console window that prints "Hello World" infinitely.Instead, this happened: explanation
App
C
creates its console window but no text is printed.What I suspect the cause of the issue
In Windows, spawn() eventually calls to
CreateProcessW
with aSTARTUPINFO
, whosedwFlags
is hard coded toSTARTF_USESTDHANDLES
. Additionally, it also get device handles of stdin, stdout and stderr withGetStdHandle
, and set them to theSTARTUPINFO
. Since appP
uses "windows" subsystem, it does not have console. stdout is alwaysINVALID_HANDLE_VALUE
. CombiningSTARTF_USESTDHANDLES
withINVALID_HANDLE_VALUE
inSTARTUPINFO
causes the unexpected behavior.If I make a call to
CreateProcessW
with same parameters except a "zeroed"STARTUPINFO
, it exhibits the correct behavior.Suggestion
spawn() should only conditionally set the
STARTF_USESTDHANDLES
flag if it can obtain those device handles, and never set any of those handlesINVALID_HANDLE_VALUE
. Like this (pseudo code):Meta
rustc --version --verbose
:The text was updated successfully, but these errors were encountered: