Skip to content

Commit 62c8256

Browse files
committed
Auto merge of #31068 - sfackler:bufwriter-panic, r=alexcrichton
We don't want to write the same data twice. Closes #30888 r? @alexcrichton
2 parents 5c1d5fc + 334bee3 commit 62c8256

File tree

1 file changed

+40
-3
lines changed

1 file changed

+40
-3
lines changed

src/libstd/io/buffered.rs

+40-3
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,10 @@ impl<R: Seek> Seek for BufReader<R> {
300300
pub struct BufWriter<W: Write> {
301301
inner: Option<W>,
302302
buf: Vec<u8>,
303+
// #30888: If the inner writer panics in a call to write, we don't want to
304+
// write the buffered data a second time in BufWriter's destructor. This
305+
// flag tells the Drop impl if it should skip the flush.
306+
panicked: bool,
303307
}
304308

305309
/// An error returned by `into_inner` which combines an error that
@@ -364,6 +368,7 @@ impl<W: Write> BufWriter<W> {
364368
BufWriter {
365369
inner: Some(inner),
366370
buf: Vec::with_capacity(cap),
371+
panicked: false,
367372
}
368373
}
369374

@@ -372,7 +377,11 @@ impl<W: Write> BufWriter<W> {
372377
let len = self.buf.len();
373378
let mut ret = Ok(());
374379
while written < len {
375-
match self.inner.as_mut().unwrap().write(&self.buf[written..]) {
380+
self.panicked = true;
381+
let r = self.inner.as_mut().unwrap().write(&self.buf[written..]);
382+
self.panicked = false;
383+
384+
match r {
376385
Ok(0) => {
377386
ret = Err(Error::new(ErrorKind::WriteZero,
378387
"failed to write the buffered data"));
@@ -455,7 +464,10 @@ impl<W: Write> Write for BufWriter<W> {
455464
try!(self.flush_buf());
456465
}
457466
if buf.len() >= self.buf.capacity() {
458-
self.inner.as_mut().unwrap().write(buf)
467+
self.panicked = true;
468+
let r = self.inner.as_mut().unwrap().write(buf);
469+
self.panicked = false;
470+
r
459471
} else {
460472
let amt = cmp::min(buf.len(), self.buf.capacity());
461473
Write::write(&mut self.buf, &buf[..amt])
@@ -489,7 +501,7 @@ impl<W: Write + Seek> Seek for BufWriter<W> {
489501
#[stable(feature = "rust1", since = "1.0.0")]
490502
impl<W: Write> Drop for BufWriter<W> {
491503
fn drop(&mut self) {
492-
if self.inner.is_some() {
504+
if self.inner.is_some() && !self.panicked {
493505
// dtors should not panic, so we ignore a failed flush
494506
let _r = self.flush_buf();
495507
}
@@ -777,6 +789,8 @@ mod tests {
777789
use prelude::v1::*;
778790
use io::prelude::*;
779791
use io::{self, BufReader, BufWriter, LineWriter, SeekFrom};
792+
use sync::atomic::{AtomicUsize, Ordering};
793+
use thread;
780794
use test;
781795

782796
/// A dummy reader intended at testing short-reads propagation.
@@ -1065,6 +1079,29 @@ mod tests {
10651079
panic!();
10661080
}
10671081

1082+
#[test]
1083+
fn panic_in_write_doesnt_flush_in_drop() {
1084+
static WRITES: AtomicUsize = AtomicUsize::new(0);
1085+
1086+
struct PanicWriter;
1087+
1088+
impl Write for PanicWriter {
1089+
fn write(&mut self, _: &[u8]) -> io::Result<usize> {
1090+
WRITES.fetch_add(1, Ordering::SeqCst);
1091+
panic!();
1092+
}
1093+
fn flush(&mut self) -> io::Result<()> { Ok(()) }
1094+
}
1095+
1096+
thread::spawn(|| {
1097+
let mut writer = BufWriter::new(PanicWriter);
1098+
writer.write(b"hello world");
1099+
writer.flush();
1100+
}).join().err().unwrap();
1101+
1102+
assert_eq!(WRITES.load(Ordering::SeqCst), 1);
1103+
}
1104+
10681105
#[bench]
10691106
fn bench_buffered_reader(b: &mut test::Bencher) {
10701107
b.iter(|| {

0 commit comments

Comments
 (0)