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

Fix Termination impl panic on closed stderr #97970

Merged
merged 1 commit into from
Jun 12, 2022
Merged

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented Jun 10, 2022

Repro:

#![feature(backtrace)]

use std::backtrace::Backtrace;
use std::io::{self, Write as _};
use std::panic::{self, PanicInfo};

#[derive(Debug)]
pub struct Error;

fn panic_hook(panic_info: &PanicInfo) {
    let backtrace = Backtrace::force_capture();
    let _ = write!(io::stdout(), "{}\n{}", panic_info, backtrace);
}

fn main() -> Result<(), Error> {
    panic::set_hook(Box::new(panic_hook));
    let stderr = io::stderr();
    let mut stderr = stderr.lock();
    while stderr.write_all(b".\n").is_ok() {}
    Err(Error)
}

Before:

$ target/debug/repro 3>&2 2>&1 1>&3 | head
.
.
.
.
.
.
.
.
.
.
panicked at 'failed printing to stderr: Broken pipe (os error 32)', library/std/src/io/stdio.rs:1016:9
   0: testing::panic_hook
             at ./src/main.rs:11:21
   1: core::ops::function::Fn::call
             at /git/rust/library/core/src/ops/function.rs:77:5
   2: std::panicking::rust_panic_with_hook
   3: std::panicking::begin_panic_handler::{{closure}}
   4: std::sys_common::backtrace::__rust_end_short_backtrace
   5: rust_begin_unwind
   6: core::panicking::panic_fmt
   7: std::io::stdio::_eprint
   8: <core::result::Result<!,E> as std::process::Termination>::report
             at /git/rust/library/std/src/process.rs:2164:9
   9: <core::result::Result<(),E> as std::process::Termination>::report
             at /git/rust/library/std/src/process.rs:2148:25
  10: std::rt::lang_start::{{closure}}
             at /git/rust/library/std/src/rt.rs:145:18
  11: std::rt::lang_start_internal
  12: std::rt::lang_start
             at /git/rust/library/std/src/rt.rs:144:17
  13: main
  14: __libc_start_main
             at /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16
  15: _start

After:

$ target/debug/repro 3>&2 2>&1 1>&3 | head
.
.
.
.
.
.
.
.
.
.

Repro:

    #![feature(backtrace)]

    use std::backtrace::Backtrace;
    use std::io::{self, Write as _};
    use std::panic::{self, PanicInfo};

    #[derive(Debug)]
    pub struct Error;

    fn panic_hook(panic_info: &PanicInfo) {
        let backtrace = Backtrace::force_capture();
        let _ = write!(io::stdout(), "{}\n{}", panic_info, backtrace);
    }

    fn main() -> Result<(), Error> {
        panic::set_hook(Box::new(panic_hook));
        let stderr = io::stderr();
        let mut stderr = stderr.lock();
        while stderr.write_all(b".\n").is_ok() {}
        Err(Error)
    }

Before:

    $ RUST_BACKTRACE=1 target/debug/testing 3>&2 2>&1 1>&3 | head
    .
    .
    .
    .
    .
    .
    .
    .
    .
    .
    panicked at 'failed printing to stderr: Broken pipe (os error 32)', library/std/src/io/stdio.rs:1016:9
       0: testing::panic_hook
                 at ./src/main.rs:11:21
       1: core::ops::function::Fn::call
                 at /git/rust/library/core/src/ops/function.rs:77:5
       2: std::panicking::rust_panic_with_hook
       3: std::panicking::begin_panic_handler::{{closure}}
       4: std::sys_common::backtrace::__rust_end_short_backtrace
       5: rust_begin_unwind
       6: core::panicking::panic_fmt
       7: std::io::stdio::_eprint
       8: <core::result::Result<!,E> as std::process::Termination>::report
                 at /git/rust/library/std/src/process.rs:2164:9
       9: <core::result::Result<(),E> as std::process::Termination>::report
                 at /git/rust/library/std/src/process.rs:2148:25
      10: std::rt::lang_start::{{closure}}
                 at /git/rust/library/std/src/rt.rs:145:18
      11: std::rt::lang_start_internal
      12: std::rt::lang_start
                 at /git/rust/library/std/src/rt.rs:144:17
      13: main
      14: __libc_start_main
                 at /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16
      15: _start

After:

    $ RUST_BACKTRACE=1 target/debug/testing 3>&2 2>&1 1>&3 | head
    .
    .
    .
    .
    .
    .
    .
    .
    .
    .
@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jun 10, 2022
@rust-highfive
Copy link
Collaborator

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 10, 2022
@joshtriplett
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jun 12, 2022

📌 Commit 563aa12 has been approved by joshtriplett

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 12, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jun 12, 2022
Fix Termination impl panic on closed stderr

Repro:

```rust
#![feature(backtrace)]

use std::backtrace::Backtrace;
use std::io::{self, Write as _};
use std::panic::{self, PanicInfo};

#[derive(Debug)]
pub struct Error;

fn panic_hook(panic_info: &PanicInfo) {
    let backtrace = Backtrace::force_capture();
    let _ = write!(io::stdout(), "{}\n{}", panic_info, backtrace);
}

fn main() -> Result<(), Error> {
    panic::set_hook(Box::new(panic_hook));
    let stderr = io::stderr();
    let mut stderr = stderr.lock();
    while stderr.write_all(b".\n").is_ok() {}
    Err(Error)
}
```

### Before:

```console
$ target/debug/repro 3>&2 2>&1 1>&3 | head
.
.
.
.
.
.
.
.
.
.
panicked at 'failed printing to stderr: Broken pipe (os error 32)', library/std/src/io/stdio.rs:1016:9
   0: testing::panic_hook
             at ./src/main.rs:11:21
   1: core::ops::function::Fn::call
             at /git/rust/library/core/src/ops/function.rs:77:5
   2: std::panicking::rust_panic_with_hook
   3: std::panicking::begin_panic_handler::{{closure}}
   4: std::sys_common::backtrace::__rust_end_short_backtrace
   5: rust_begin_unwind
   6: core::panicking::panic_fmt
   7: std::io::stdio::_eprint
   8: <core::result::Result<!,E> as std::process::Termination>::report
             at /git/rust/library/std/src/process.rs:2164:9
   9: <core::result::Result<(),E> as std::process::Termination>::report
             at /git/rust/library/std/src/process.rs:2148:25
  10: std::rt::lang_start::{{closure}}
             at /git/rust/library/std/src/rt.rs:145:18
  11: std::rt::lang_start_internal
  12: std::rt::lang_start
             at /git/rust/library/std/src/rt.rs:144:17
  13: main
  14: __libc_start_main
             at /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16
  15: _start
```

### After:

```console
$ target/debug/repro 3>&2 2>&1 1>&3 | head
.
.
.
.
.
.
.
.
.
.
```
@bors
Copy link
Contributor

bors commented Jun 12, 2022

⌛ Testing commit 563aa12 with merge 56db11c0496f579a182c562fcd5c46afce6f0b1c...

@bors
Copy link
Contributor

bors commented Jun 12, 2022

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 12, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 12, 2022
Rollup of 5 pull requests

Successful merges:

 - rust-lang#97921 (additional docs example for replace **all** of str)
 - rust-lang#97970 (Fix Termination impl panic on closed stderr)
 - rust-lang#97991 (Use safer `strip=symbols`-flag for dylibs on macOS)
 - rust-lang#97992 (Stabilize scoped threads.)
 - rust-lang#98012 (`ValuePairs::PolyTraitRefs` should be called "trait"s in type error diagnostics)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@bors bors merged commit cf3c41a into rust-lang:master Jun 12, 2022
@rustbot rustbot added this to the 1.63.0 milestone Jun 12, 2022
@dtolnay dtolnay deleted the terminate branch June 12, 2022 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants