Skip to content

Commit

Permalink
Rollup merge of rust-lang#127659 - saethlin:manually-drop-bufwriter, …
Browse files Browse the repository at this point in the history
…r=joboet

Use ManuallyDrop in BufWriter::into_parts

The fact that `mem::forget` takes by value means that it interacts very poorly with Stacked Borrows; generally users think of calling it as a no-op, but in Stacked Borrows, the field retagging tends to cause surprise tag invalidation.
  • Loading branch information
workingjubilee authored Jul 14, 2024
2 parents 6b67c66 + c8b79dd commit 3033120
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 7 deletions.
14 changes: 7 additions & 7 deletions std/src/io/buffered/bufwriter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::fmt;
use crate::io::{
self, ErrorKind, IntoInnerError, IoSlice, Seek, SeekFrom, Write, DEFAULT_BUF_SIZE,
};
use crate::mem;
use crate::mem::{self, ManuallyDrop};
use crate::ptr;

/// Wraps a writer and buffers its output.
Expand Down Expand Up @@ -164,13 +164,13 @@ impl<W: Write> BufWriter<W> {
/// assert_eq!(&buffered_data.unwrap(), b"ata");
/// ```
#[stable(feature = "bufwriter_into_parts", since = "1.56.0")]
pub fn into_parts(mut self) -> (W, Result<Vec<u8>, WriterPanicked>) {
let buf = mem::take(&mut self.buf);
let buf = if !self.panicked { Ok(buf) } else { Err(WriterPanicked { buf }) };
pub fn into_parts(self) -> (W, Result<Vec<u8>, WriterPanicked>) {
let mut this = ManuallyDrop::new(self);
let buf = mem::take(&mut this.buf);
let buf = if !this.panicked { Ok(buf) } else { Err(WriterPanicked { buf }) };

// SAFETY: forget(self) prevents double dropping inner
let inner = unsafe { ptr::read(&self.inner) };
mem::forget(self);
// SAFETY: double-drops are prevented by putting `this` in a ManuallyDrop that is never dropped
let inner = unsafe { ptr::read(&this.inner) };

(inner, buf)
}
Expand Down
10 changes: 10 additions & 0 deletions std/src/io/buffered/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1067,3 +1067,13 @@ fn bufreader_full_initialize() {
// But we initialized the whole buffer!
assert_eq!(reader.initialized(), reader.capacity());
}

/// This is a regression test for https://github.com/rust-lang/rust/issues/127584.
#[test]
fn bufwriter_aliasing() {
use crate::io::{BufWriter, Cursor};
let mut v = vec![0; 1024];
let c = Cursor::new(&mut v);
let w = BufWriter::new(Box::new(c));
let _ = w.into_parts();
}

0 comments on commit 3033120

Please sign in to comment.