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

panics when log is called after pty closes with EIO #53

Closed
Allen-Webb opened this issue Dec 7, 2022 · 1 comment
Closed

panics when log is called after pty closes with EIO #53

Allen-Webb opened this issue Dec 7, 2022 · 1 comment

Comments

@Allen-Webb
Copy link
Contributor

I am running into the following error:
failed to set color: Os { code: 5, kind: Uncategorized, message: "Input/output error" }', <registry path>/stderrlog-0.5.3/src/lib.rs:333:18

When I look at lib.rs, it looks inconsistent in that we ignore errors for write!() but panic on error for .set_color(). I don't believe panicking in this scenario makes sense. I am working on getting a repro example.

@Allen-Webb
Copy link
Contributor Author

This is really ugly, but I have a minimalistic repro case:

Cargo.toml

[package]
name = "crash-demo"
version = "0.1.0"
edition = "2021"

[dependencies]
libc = "0.2"
log = "0.4"
stderrlog = "0.5"

src/main.rs

fn check_ret(ret: std::os::raw::c_int, msg: &str) {
    if ret < 0 {
        panic!("{}: {}", msg, std::io::Error::last_os_error());
    }
}

fn main() {
    println!("Setting up PTY");
    // Set up a pseudo terminal.
    let pt = unsafe { libc::getpt() };
    check_ret(pt, "getpt");
    check_ret(unsafe { libc::grantpt(pt) }, "grantpt");
    check_ret(unsafe { libc::unlockpt(pt) }, "unlockpt");

    let name = unsafe { libc::ptsname(pt) };
    if name.is_null() {
        panic!("ptsname: {}", std::io::Error::last_os_error());
    }

    let client = unsafe { libc::open(name, libc::O_RDWR) };
    check_ret(client, "open client pty");

    // Back up stderr
    let dup_stderr = unsafe { libc::dup(2) };
    check_ret(dup_stderr, "dup stderr");

    // Set up the panic handler to restore stderr.
    let default_panic = std::panic::take_hook();
    std::panic::set_hook(Box::new(move |info| {
        check_ret(unsafe { libc::dup2(dup_stderr, 2) }, "dup2 restore stderr");
        default_panic(info);
        std::process::abort();
    }));

    // Replace stderr with pty
    check_ret(unsafe { libc::dup2(client, 2) }, "dup2 restore stderr");

    println!("Setting up stderrlog");
    stderrlog::new().verbosity(50).init().unwrap();
    log::info!("This should work.");

    println!("Closing PTY");
    check_ret(unsafe { libc::close(pt) }, "close pt");

    println!("Sending log after PTY closed");
    log::info!("This should trigger an EIO.");

    // Restore stderr
    check_ret(unsafe { libc::dup2(dup_stderr, 2) }, "dup2 restore stderr");

    println!("Done!");

    check_ret(unsafe { libc::close(client) }, "close client");
    check_ret(unsafe { libc::close(dup_stderr) }, "close dup_stderr");
}

@Allen-Webb Allen-Webb changed the title panics for io panics when log is called after pty closes with EIO Dec 7, 2022
Allen-Webb added a commit to Allen-Webb/stderrlog-rs that referenced this issue Dec 8, 2022
This makes sure stderrlog is well behaved after a pty closes.

cardoe#53
Allen-Webb added a commit to Allen-Webb/stderrlog-rs that referenced this issue Dec 8, 2022
cardoe pushed a commit to Allen-Webb/stderrlog-rs that referenced this issue Jan 31, 2024
This makes sure stderrlog is well behaved after a pty closes.

cardoe#53
@cardoe cardoe closed this as completed in b5b515e Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant