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
This commit is contained in:
parent
552080181c
commit
e2fd2dffde
3 changed files with 52 additions and 11 deletions
|
@ -11,7 +11,7 @@
|
||||||
use prelude::v1::*;
|
use prelude::v1::*;
|
||||||
use io::prelude::*;
|
use io::prelude::*;
|
||||||
|
|
||||||
use cell::RefCell;
|
use cell::{RefCell, BorrowState};
|
||||||
use cmp;
|
use cmp;
|
||||||
use fmt;
|
use fmt;
|
||||||
use io::lazy::Lazy;
|
use io::lazy::Lazy;
|
||||||
|
@ -264,9 +264,8 @@ impl Write for Stdout {
|
||||||
fn write_all(&mut self, buf: &[u8]) -> io::Result<()> {
|
fn write_all(&mut self, buf: &[u8]) -> io::Result<()> {
|
||||||
self.lock().write_all(buf)
|
self.lock().write_all(buf)
|
||||||
}
|
}
|
||||||
fn write_fmt(&mut self, fmt: fmt::Arguments) -> io::Result<()> {
|
// Don't override write_fmt as it's possible to run arbitrary code during a
|
||||||
self.lock().write_fmt(fmt)
|
// write_fmt, allowing the possibility of a recursive lock (aka deadlock)
|
||||||
}
|
|
||||||
}
|
}
|
||||||
#[stable(feature = "rust1", since = "1.0.0")]
|
#[stable(feature = "rust1", since = "1.0.0")]
|
||||||
impl<'a> Write for StdoutLock<'a> {
|
impl<'a> Write for StdoutLock<'a> {
|
||||||
|
@ -334,9 +333,7 @@ impl Write for Stderr {
|
||||||
fn write_all(&mut self, buf: &[u8]) -> io::Result<()> {
|
fn write_all(&mut self, buf: &[u8]) -> io::Result<()> {
|
||||||
self.lock().write_all(buf)
|
self.lock().write_all(buf)
|
||||||
}
|
}
|
||||||
fn write_fmt(&mut self, fmt: fmt::Arguments) -> io::Result<()> {
|
// Don't override write_fmt for the same reasons as Stdout
|
||||||
self.lock().write_fmt(fmt)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
#[stable(feature = "rust1", since = "1.0.0")]
|
#[stable(feature = "rust1", since = "1.0.0")]
|
||||||
impl<'a> Write for StderrLock<'a> {
|
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")]
|
reason = "implementation detail which may disappear or be replaced at any time")]
|
||||||
#[doc(hidden)]
|
#[doc(hidden)]
|
||||||
pub fn _print(args: fmt::Arguments) {
|
pub fn _print(args: fmt::Arguments) {
|
||||||
if let Err(e) = LOCAL_STDOUT.with(|s| match s.borrow_mut().as_mut() {
|
let result = LOCAL_STDOUT.with(|s| {
|
||||||
Some(w) => w.write_fmt(args),
|
if s.borrow_state() == BorrowState::Unused {
|
||||||
None => stdout().write_fmt(args)
|
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);
|
panic!("failed printing to stdout: {}", e);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -127,6 +127,7 @@
|
||||||
#![feature(str_char)]
|
#![feature(str_char)]
|
||||||
#![feature(into_cow)]
|
#![feature(into_cow)]
|
||||||
#![feature(slice_patterns)]
|
#![feature(slice_patterns)]
|
||||||
|
#![feature(std_misc)]
|
||||||
#![cfg_attr(test, feature(test, rustc_private, std_misc))]
|
#![cfg_attr(test, feature(test, rustc_private, std_misc))]
|
||||||
|
|
||||||
// Don't link to std. We are std.
|
// Don't link to std. We are std.
|
||||||
|
|
38
src/test/run-pass/issue-23781.rs
Normal file
38
src/test/run-pass/issue-23781.rs
Normal file
|
@ -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();
|
||||||
|
}
|
Loading…
Add table
Add a link
Reference in a new issue