Skip to content

BufWriter documentation should warn about ignored errors on drop #37045

Closed
@jgallag88

Description

@jgallag88

BufWriter calls flush_buf() in its destructor, which writes out the contents of its internal buffer to the underlying writer. The destructor ignores any errors that happen when flushing the buffer. This makes it easy to accidentally forget to handle such errors:

use std::io::BufWriter;
use std::io::Result;
use std::io::Write;
use std::process::{ChildStdin, Command, Stdio};
use std::time::Duration;
use std::thread;

fn mk_broken_pipe() -> ChildStdin {
    let child = Command::new("true").stdin(Stdio::piped()).spawn().unwrap();
    // Wait for child to exit and close it's end of the pipe
    thread::sleep(Duration::from_secs(1));
    child.stdin.unwrap() // All writes to this pipe should now fail
}

fn f1() -> Result<()> {
    let mut child_stdin = mk_broken_pipe();
    child_stdin.write_all(b"Some bytes")
}

fn f2() -> Result<()> {
    let mut child_stdin = BufWriter::new(mk_broken_pipe());
    child_stdin.write_all(b"Some bytes")
    // Oops, forgot to call flush()
}

fn f3() -> Result<()> {
    let mut child_stdin = BufWriter::new(mk_broken_pipe());
    try!(child_stdin.write_all(b"Some bytes"));
    child_stdin.flush()
}

fn main() {
    println!("Without BufWriter: {:?}", f1());
    println!("With BufWriter, but forgot to call flush: {:?}", f2());
    println!("With BufWriter, with call to flush: {:?}", f3());
}

The code above outputs

Without BufWriter: Err(Error { repr: Os { code: 32, message: "Broken pipe" } })
With BufWriter, but forgot to call flush: Ok(())
With BufWriter, with call to flush: Err(Error { repr: Os { code: 32, message: "Broken pipe" } })

It's tempting (at least to me) to think of BufWriter as preserving the semantics of the underlying writer, while offering possible performance improvements. It doesn't quite though, and it's unfortunately easy to forget to call flush(), and to ignore errors when using BufWriter, as in function f2() above.

It seems like a warning in the BufWriter documentation could be helpful here. Right now it says

The buffer will be written out when the writer is dropped.

Perhaps it could be changed to something like

When the writer is dropped, the BufWriter will be flushed and the contents of its buffer will be written out. However, any errors that happen when the writer is dropped will be ignored. Code that wishes to handle such errors must manually call flush() before the writer is dropped.

If a change along these lines is of interest, I'll make a pull request.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions