diff options
| author | Alex Crichton <alex@alexcrichton.com> | 2015-03-27 16:25:49 -0700 |
|---|---|---|
| committer | Alex Crichton <alex@alexcrichton.com> | 2015-03-27 19:03:18 -0700 |
| commit | e2fd2dffde52a59f7d59d67460aeb2ebf33f77dd (patch) | |
| tree | 1097838bfff9b82a5d1547e4d442316a3b08d747 /src | |
| parent | 552080181c58beef03493a110b4a38b20b6b5da5 (diff) | |
| download | rust-e2fd2dffde52a59f7d59d67460aeb2ebf33f77dd.tar.gz rust-e2fd2dffde52a59f7d59d67460aeb2ebf33f77dd.zip | |
std: Don't deadlock/panic on recursive prints
Previously a panic was generated for recursive prints due to a double-borrow of a `RefCell`. This was solved by the second borrow's output being directed towards the global stdout instead of the per-thread stdout (still experimental functionality). After this functionality was altered, however, recursive prints still deadlocked due to the overridden `write_fmt` method which locked itself first and then wrote all the data. This was fixed by removing the override of the `write_fmt` method. This means that unlocked usage of `write!` on a `Stdout`/`Stderr` may be slower due to acquiring more locks, but it's easy to make more performant with a call to `.lock()`. Closes #23781
Diffstat (limited to 'src')
| -rw-r--r-- | src/libstd/io/stdio.rs | 24 | ||||
| -rw-r--r-- | src/libstd/lib.rs | 1 | ||||
| -rw-r--r-- | src/test/run-pass/issue-23781.rs | 38 |
3 files changed, 52 insertions, 11 deletions
diff --git a/src/libstd/io/stdio.rs b/src/libstd/io/stdio.rs index 53f67766ea6..d361f17cbe4 100644 --- a/src/libstd/io/stdio.rs +++ b/src/libstd/io/stdio.rs @@ -11,7 +11,7 @@ use prelude::v1::*; use io::prelude::*; -use cell::RefCell; +use cell::{RefCell, BorrowState}; use cmp; use fmt; use io::lazy::Lazy; @@ -264,9 +264,8 @@ impl Write for Stdout { fn write_all(&mut self, buf: &[u8]) -> io::Result<()> { self.lock().write_all(buf) } - fn write_fmt(&mut self, fmt: fmt::Arguments) -> io::Result<()> { - self.lock().write_fmt(fmt) - } + // Don't override write_fmt as it's possible to run arbitrary code during a + // write_fmt, allowing the possibility of a recursive lock (aka deadlock) } #[stable(feature = "rust1", since = "1.0.0")] impl<'a> Write for StdoutLock<'a> { @@ -334,9 +333,7 @@ impl Write for Stderr { fn write_all(&mut self, buf: &[u8]) -> io::Result<()> { self.lock().write_all(buf) } - fn write_fmt(&mut self, fmt: fmt::Arguments) -> io::Result<()> { - self.lock().write_fmt(fmt) - } + // Don't override write_fmt for the same reasons as Stdout } #[stable(feature = "rust1", since = "1.0.0")] impl<'a> Write for StderrLock<'a> { @@ -395,10 +392,15 @@ pub fn set_print(sink: Box<Write + Send>) -> Option<Box<Write + Send>> { reason = "implementation detail which may disappear or be replaced at any time")] #[doc(hidden)] pub fn _print(args: fmt::Arguments) { - if let Err(e) = LOCAL_STDOUT.with(|s| match s.borrow_mut().as_mut() { - Some(w) => w.write_fmt(args), - None => stdout().write_fmt(args) - }) { + let result = LOCAL_STDOUT.with(|s| { + if s.borrow_state() == BorrowState::Unused { + if let Some(w) = s.borrow_mut().as_mut() { + return w.write_fmt(args); + } + } + stdout().write_fmt(args) + }); + if let Err(e) = result { panic!("failed printing to stdout: {}", e); } } diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index 8de6e5257ec..5f5f2fed567 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -127,6 +127,7 @@ #![feature(str_char)] #![feature(into_cow)] #![feature(slice_patterns)] +#![feature(std_misc)] #![cfg_attr(test, feature(test, rustc_private, std_misc))] // Don't link to std. We are std. diff --git a/src/test/run-pass/issue-23781.rs b/src/test/run-pass/issue-23781.rs new file mode 100644 index 00000000000..23ac8d2b782 --- /dev/null +++ b/src/test/run-pass/issue-23781.rs @@ -0,0 +1,38 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or +// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license +// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use std::fmt; + +struct Foo; +impl fmt::Debug for Foo { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + println!("<Foo as Debug>::fmt()"); + + write!(fmt, "") + } +} + +fn test1() { + let foo_str = format!("{:?}", Foo); + + println!("{}", foo_str); +} + +fn test2() { + println!("{:?}", Foo); +} + +fn main() { + // This works fine + test1(); + + // This fails + test2(); +} |
