Skip to content

Commit e2fd2df

Browse files
committed
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
1 parent 5520801 commit e2fd2df

File tree

3 files changed

+52
-11
lines changed

3 files changed

+52
-11
lines changed

src/libstd/io/stdio.rs

+13-11
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
use prelude::v1::*;
1212
use io::prelude::*;
1313

14-
use cell::RefCell;
14+
use cell::{RefCell, BorrowState};
1515
use cmp;
1616
use fmt;
1717
use io::lazy::Lazy;
@@ -264,9 +264,8 @@ impl Write for Stdout {
264264
fn write_all(&mut self, buf: &[u8]) -> io::Result<()> {
265265
self.lock().write_all(buf)
266266
}
267-
fn write_fmt(&mut self, fmt: fmt::Arguments) -> io::Result<()> {
268-
self.lock().write_fmt(fmt)
269-
}
267+
// Don't override write_fmt as it's possible to run arbitrary code during a
268+
// write_fmt, allowing the possibility of a recursive lock (aka deadlock)
270269
}
271270
#[stable(feature = "rust1", since = "1.0.0")]
272271
impl<'a> Write for StdoutLock<'a> {
@@ -334,9 +333,7 @@ impl Write for Stderr {
334333
fn write_all(&mut self, buf: &[u8]) -> io::Result<()> {
335334
self.lock().write_all(buf)
336335
}
337-
fn write_fmt(&mut self, fmt: fmt::Arguments) -> io::Result<()> {
338-
self.lock().write_fmt(fmt)
339-
}
336+
// Don't override write_fmt for the same reasons as Stdout
340337
}
341338
#[stable(feature = "rust1", since = "1.0.0")]
342339
impl<'a> Write for StderrLock<'a> {
@@ -395,10 +392,15 @@ pub fn set_print(sink: Box<Write + Send>) -> Option<Box<Write + Send>> {
395392
reason = "implementation detail which may disappear or be replaced at any time")]
396393
#[doc(hidden)]
397394
pub fn _print(args: fmt::Arguments) {
398-
if let Err(e) = LOCAL_STDOUT.with(|s| match s.borrow_mut().as_mut() {
399-
Some(w) => w.write_fmt(args),
400-
None => stdout().write_fmt(args)
401-
}) {
395+
let result = LOCAL_STDOUT.with(|s| {
396+
if s.borrow_state() == BorrowState::Unused {
397+
if let Some(w) = s.borrow_mut().as_mut() {
398+
return w.write_fmt(args);
399+
}
400+
}
401+
stdout().write_fmt(args)
402+
});
403+
if let Err(e) = result {
402404
panic!("failed printing to stdout: {}", e);
403405
}
404406
}

src/libstd/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@
127127
#![feature(str_char)]
128128
#![feature(into_cow)]
129129
#![feature(slice_patterns)]
130+
#![feature(std_misc)]
130131
#![cfg_attr(test, feature(test, rustc_private, std_misc))]
131132

132133
// Don't link to std. We are std.

src/test/run-pass/issue-23781.rs

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
use std::fmt;
12+
13+
struct Foo;
14+
impl fmt::Debug for Foo {
15+
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
16+
println!("<Foo as Debug>::fmt()");
17+
18+
write!(fmt, "")
19+
}
20+
}
21+
22+
fn test1() {
23+
let foo_str = format!("{:?}", Foo);
24+
25+
println!("{}", foo_str);
26+
}
27+
28+
fn test2() {
29+
println!("{:?}", Foo);
30+
}
31+
32+
fn main() {
33+
// This works fine
34+
test1();
35+
36+
// This fails
37+
test2();
38+
}

0 commit comments

Comments
 (0)