termcolor panics when used from the Windows GUI application compiled with Rust 1.56

[ancwrd1]

Starting with Rust 1.56 an attempt to use the termcolor library (for example indirectly via env_logger or pretty_env_logger crate) panics when it runs from the Windows application compiled into GUI subsystem.

Code example:

#![windows_subsystem = "windows"]

use std::io::{self, Write};
use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor};

use std::panic;

fn write_green() -> io::Result<()> {
    let mut stdout = StandardStream::stdout(ColorChoice::Always);
    stdout.set_color(ColorSpec::new().set_fg(Some(Color::Green)))?;
    writeln!(&mut stdout, "green text!")
}

fn main() {
    panic::set_hook(Box::new(|panic_info| {
        let _ = std::fs::write("panic.txt", format!("{:#?}", panic_info));
    }));

    let _ = write_green();
}

Dumps:

PanicInfo {
    payload: Any { .. },
    message: Some(
        assertion failed: !handle.is_null(),
    ),
    location: Location {
        file: "/rustc/09c42c45858d5f3aedfa670698275303a3d19afa\\library\\std\\src\\os\\.\\windows\\io\\handle.rs",
        line: 183,
        col: 9,
    },
}

[BurntSushi]

This doesn't appear to be a problem with this library. As you can see, the panic location comes from inside std.

I don't use Windows, so I don't really understand why std would be panicking here. I'm afraid you're on your own with respect to debugging that.

[BurntSushi]

This doesn't happen with Rust 1.55? If so, it would be good to find the change responsible in std for this.

[ancwrd1]

This essentially boils down to winapi-util code:

    pub fn stdout() -> HandleRef {
        unsafe { HandleRef::from_raw_handle(io::stdout().as_raw_handle()) }
    }

    pub unsafe fn from_raw_handle(handle: RawHandle) -> HandleRef {
        HandleRef(HandleRefInner(Some(File::from_raw_handle(handle))))
    }

which then panics because of std/src/os/windows/io/handle.rs, line 183:

    unsafe fn from_raw_handle(handle: RawHandle) -> Self {
        assert!(!handle.is_null());
        Self { handle: NonNull::new_unchecked(handle) }
    }

To my opinion if stdlib doesn't allow a null handle anymore the library must check for it.
Should I open a ticket in winapi-util (which happens to be yours also) or do you think this should be reported in Rust issuer tracker?

[BurntSushi]

We can talk about it here. What do you think this library should do? What does writing to stdout do in your program, without termcolor? I mean it looks like the problem here is that there is no stdout, but your program assumes there is one?

[ancwrd1]

I don't use it directly, this is used by env_logger and the code is just env_logger::init() which worked fine prior to upgrading toolchain to 1.56. And env_logger uses stderr obviously which now panics.

The application is cross-platform and runs under macOS, Windows and Linux. It's not a critical issue in a sense that I can simply put #[cfg(unix)] in front of the env_logger::init() but it 's my understanding that there should be no unexpected panics or they should be documented at least.

[BurntSushi]

The next step would be to find the PR that added this assert to std.

Could you please answer my question as to whether simply writing to stdout in your program works?

[BurntSushi]

And I would still like to know what you want termcolor to do here. If there's no panic, then what should the behavior be?

[ancwrd1]

Simply writing to stdout doesn't panic.

I am not much familiar with the code but I can see for example that termcolor checks the result of wincon::Console::stderr() so my suggestion would be to modify winapi-util so that HandleRef::stderr and HandleRef::stdout return a Result based on the is_null check of the raw handle.

[BurntSushi]

Simply writing to stdout doesn't panic.

Then more investigation is needed here because now I don't understand why termcolor panics.

so my suggestion would be to modify winapi-util so that HandleRef::stderr and HandleRef::stdout return a Result based on the is_null check of the raw handle.

This unfortunately doesn't answer my question. I'm not asking for API design. I'm asking for what you want the behavior of termcolor to be in your program.

If a simple line about panics in the docs is all you want, what should it say? I'm not sure I could completely characterize what you're observing. I don't even know what "windows subsystem" is for example.

[ancwrd1]

Then more investigation is needed here because now I don't understand why termcolor panics.

To my understanding it panics in winapi-util because first it calls io::stdout().as_raw_handle() which returns null handle on Windows (when app is compiled into "GUI" subsystem) which is then passed to File::from_raw_handle. When I write to stdout directly there is no from_raw_handle invocation (so no assertion I guess but I don't know the std internals).

I'm asking for what you want the behavior of termcolor to be in your program.

The old behavior which doesn't panic and which doesn't require platform-specific cfg guards.

I don't even know what "windows subsystem" is for example.

https://rust-lang.github.io/rfcs/1665-windows-subsystem.html
Windows executables can be marked as "console" or "gui" (or "windows").

If a simple line about panics in the docs is all you want, what should it say?

You can say that it may panic on Windows platform if the application doesn't have a console (so is compiled to "windows" subsystem).

[BurntSushi]

Okay sorry, but this comversation is getting too frustrating to me. IMO, you're focusing too much on the implementation details instead of the higher level picture. Like, yeah, obviously no raw handle is being returned. But why? And how is that possible if stdout printing works just fine? Shouldn't that indicate that a raw handle exists somewhere?

And how should previous behavior be restored? What precisely has changed? If there's no raw handle, then how exactly is previous behavior achieved? What is the previouw behavior? What changed in std? Is it a new assert or is it something about the raw handles that have changed?

I don't have time to unravel this right now, and probably won't for a while. So if someone wants action here, then someone else needs to unravel the complete mystery here.

[ancwrd1]

I didn't mean to bring any frustration, sorry if you feel that way. I can open an issue in Rust repository (I think it's probably a good idea to get a feedback from std team).

[BurntSushi]

It's not your fault. Really. It's mostly just the issue and me not having the proper time to dedicate to it.

[ancwrd1]

It was already reported and will likely be fixed soon: rust-lang/rust#88576

I think it can be probably closed now.