oconnor663 / duct.rs

a Rust library for running child processes
MIT License
819 stars 34 forks source link

Redirect and forward to file in a Windows service #99

Open qrnch-jan opened 2 years ago

qrnch-jan commented 2 years ago

I want to do the shell equivalent of mycommand 2>&1 > /tmp/output.log with duct, and I accomplish this with:

  duct::cmd(program, args)
    .stderr_to_stdout()
    .unchecked()
    .stdout_file(logf)
    .run()

This works great, except that when I run it in a Windows service, I get an "invalid handle" error. I figure this stems from Windows services not having standard input or outputs.

However, I wonder if it's a fundamental limitation or if it can work? If I run std::process::Command and use the .output() method I can capture all the stdout and stderr and write them to a file, but I really would like to "preserve order" of the output, as is done when redirecting stderr to stdout.

oconnor663 commented 2 years ago

Hmm, I'm not aware of any Windows-specific limitations here, and I'd expect that expression to just work on Windows. Could you provide a small program that reproduces the error you're seeing, so that I can try it on my own box?

qrnch-jan commented 2 years ago

I'm having some trouble with the "small" qualifier because services require a fair amount of boiler-plate code, unfortunately. That said, I did what I could: https://github.com/qrnch-jan/svcexample

It's a service that you can register, run and unregister. The only thing it does when run is run a powershell script and then self-terminates. The idea is that the output of the script should be logged to a file (using duct).

I put this together very quickly, but I did a test run and it did trigger the problem.

Also: The resulting binary should not require vcredist when built with the msvc toolchain.

oconnor663 commented 2 years ago

Here's the sort of thing I was looking for:

const PYTHON_PROGRAM: &str = r#"
import sys
print("something for stdout")
print("something for stderr", file=sys.stderr)
"#;

fn main() {
    let logf = std::fs::File::create("logfile.txt").unwrap();
    duct::cmd!("python", "-c", PYTHON_PROGRAM)
        .stderr_to_stdout()
        .unchecked()
        .stdout_file(logf)
        .run()
        .unwrap();
}

When I run that on my Windows box, I don't get any errors, and I see both "something for stderr" and "something for stdout" in the log file. That makes me guess that whatever error you're running into is something wrong with your command, not with the way duct is running it.

qrnch-jan commented 2 years ago

The code you posted does not appear to be a Windows service. I can run duct with the redirection outside of a service without any issues, but I get the error when run within a service.

At this point I'm not sure if you're saying that from duct's perspective there are no differences between a Windows service and a regular program, or if you're lucky enough not to have to had to work with Windows services before. :)

I have to look at this through a debugger to make sure I haven't made any mistakes.

oconnor663 commented 2 years ago

Ah, thank you for repeating that. You're totally right, I'd missed that detail above, because I have no experience with Windows Services :) Duct definitely doesn't have any special logic built-in to deal with them.

I wonder if there could be an issue related to the service not having permission to open the log file? If it was a straight up error, presumably that would happen on an earlier line, but maybe the open succeeds but returns a weird handle for some reason?

woelper commented 1 year ago

I believe I have run into the same issue. I am using duct from a gui program. On windows, I use #![windows_subsystem = "windows"] to hide a console window popping up when I start the application. If I start the executable outside a terminal I also receive The handle is invalid. (os error 6). This does not happen if I run on other platforms, such as OSX.

Maybe this is easier to reproduce this way:

#![windows_subsystem = "windows"]

const PYTHON_PROGRAM: &str = r#"
import sys
print("something for stdout")
print("something for stderr", file=sys.stderr)
"#;

fn main() {

    let res = duct::cmd!("python", "-c", PYTHON_PROGRAM)
        .stderr_to_stdout()
        .run()
        .unwrap();

}
nazar-pc commented 9 months ago

Hitting the same issue without service just by adding #![windows_subsystem = "windows"] to the app, has anyone found a workaround? There was no such issue with std::process::Command, so there must be something that duct is doing that is special.

nazar-pc commented 9 months ago

I found a workaround, need to add .stdin_null().stdout_null().stderr_null(), apparently Windows apps with #![windows_subsystem = "windows"] do not really give stdout properly, so I guess duct is trying to read from it and errors out.

qrnch-jan commented 9 months ago

Hitting the same issue without service just by adding #![windows_subsystem = "windows"] to the app, has anyone found a workaround? There was no such issue with std::process::Command, so there must be something that duct is doing that is special.

Well it's sort of both a duct and Windows issue.

duct assumes that the process allocates the standard input/output handles, but there's nothing actually requiring a process to do that. In a Windows service, for instance, there's no console and there's no (obvious) way to redirect the input or output, so Microsoft chose to make services not allocate stdin/stdout/stderr by default.

It's perfectly valid for duct to document this as a "known issues": It simply will not work without the standard std i/o handles.

That said, there are other ways this could be handled, based on setting up its own stdin/stdout/stderr handles.

For instance, duct could detect that it's running in a "no std i/o" environment, and then create the stdout/stderr handles itself and redirect output to them to files. Or it could redirect its own stdout/stderr to stdin running in a client process which could be a console running on the desktop. I have some old C code somewhere to do all of this, and I've been meaning to see if there's a neat way to port and integrate these things into duct, but I unfortunately never found the time.

With regards to the "windows subsystem": The idea when you run the "window subsystem" is that there's no std CRT, so you have to rely on win32 for everything. When using std i/o redirections, it's fair to assume the Rust code assumes there's an initialized CRT it can rely on.

oconnor663 commented 9 months ago

I'd like to try to reproduce this myself, to see exactly where in Duct the error is occuring. Could anyone point me to a GitHub repo I could clone and run some command in to repro? I have a Windows box I can use but no experience writing Windows services.

However, this turns out, it sounds like a good candidate for an entry in https://github.com/oconnor663/duct.py/blob/master/gotchas.md.

nazar-pc commented 9 months ago

My project is probably too large to recommend, but essentially I have #![windows_subsystem = "windows"] and spawning the process itself again with duct to handle child process restarts and logging, including crashes, but only on Linux and macOS due to this issue (and generally how Windows works).

qrnch-jan commented 9 months ago

I'd like to try to reproduce this myself, to see exactly where in Duct the error is occuring. Could anyone point me to a GitHub repo I could clone and run some command in to repro? I have a Windows box I can use but no experience writing Windows services.

Setting up a Windows service is kind of non-trivial, unfortunately. And once you do, it can be a pain to inspect it (ironically my suggestion to allow duct to redirect to a file in no std i/o environments was partially inspired by this). The repo I posted earlier (see https://github.com/oconnor663/duct.rs/issues/99#issuecomment-966004729) solves the "service binary" part, however you still need a way to interact with it.

The way I do it is using Visual Studio (Community Edition is free for non-commercial use).

This is a rough guide to get to the point where you can run and debug the svcexample service:

  1. Check out the repo I referenced above, build and install it using the instructions in its readme. (Note: Windows Defender had a tendency to warn about services I've written in Rust, so you may need to take the resulting binary out of quarantine).
  2. Open up Visual Studio 2022 (you may need to start Visual Studio in elevated mode (right-click and choose to run it as Administrator)), and choose "Continue without code".
  3. Under the "Debug" menu, choose "Attach to Process"
  4. Switch to the Services manager in Windows, find svcexample in the list and select to Start the service.
  5. Switch back to Visual Studio, find the svcexample.exe in the list and Attach to it.
  6. You can now step through the service binary using the debugger.

IIRC you can compile the svcexample with dbgtools support, which will cause the service to stop and wait for a debugger to connect to it. This is useful for scenarios like this -- however it will cause the Windows Service subsystem to automatically terminate the process if it thinks the service has died/crashed/hung (which it technically is, I guess, while its waiting for the a debugger to attach), so you need to be fast .. ish from the point you tell the service manager to start the service.

However, this turns out, it sounds like a good candidate for an entry in https://github.com/oconnor663/duct.py/blob/master/gotchas.md.

Yeah, it definitely belongs there. I'll try to find some time to write something during the holidays.

oconnor663 commented 9 months ago

@qrnch-jan that worked! Thank you so much for walking me through it.

I think the cause of the error is that duct is unnecessarily dup'ing stdin. That's no big deal most of the time, but with a Windows service I think stdin doesn't exist, and the dup fails. The fix is pretty straightforward, and in fact there was a TODO next to the relevant lines saying I should probably fix it :) This branch seems to fix your service example above: https://github.com/oconnor663/duct.rs/tree/avoid_extra_dup. Could you try it and let me know whether it works for you?

oconnor663 commented 9 months ago

I've gone ahead and shipped this fix as v0.13.7, since it makes the code better anyway. But folks here, please let me know if you get a chance to test it.