Skip to content

Commit

Permalink
Rollup merge of #120781 - hniksic:master, r=m-ou-se
Browse files Browse the repository at this point in the history
Correct usage note on OpenOptions::append()

This PR aims to correct the following usage note in `OpenOptions::append()`, which currently contains misleading information:

> One maybe obvious note when using append-mode: make sure that all data that belongs together is written to the file in one operation. This can be done by concatenating strings before passing them to [write()](https://doc.rust-lang.org/std/io/trait.Write.html#tymethod.write), or using a buffered writer (with a buffer of adequate size), and calling [flush()](https://doc.rust-lang.org/std/io/trait.Write.html#tymethod.flush) when the message is complete.

The above is misleading because, despite appearances, neither concatenating data before passing it to `write()`, nor delaying writes using `BufWriter`, ensures atomicity. `File::write()`, as well as the underlying `write(2)` system call, makes no guarantees that the data passed to it will be written out in full. It is allowed to write out only a part of the data, and has a return value that tells you how much it has written, at which point it has already returned and modified the file with partial data. Given this limitation, the only way to ensure atomicity of appends is through external locking.

Attempting to ensure atomicity by issuing data in a single `write()` is a footgun often stumbled upon by beginners, which shouldn't be advertised in the docs. The worst thing about the footgun is that it *appears* to work at first, only failing when the string becomes sufficiently large, or when some internal properties of the output file descriptor change (e.g. it is switched from regular file to a special file that talks to a socket or TTY), making it accept smaller writes. Additionally, the suggestion to use `BufWriter` skims over the issue of buffer sizes, as well as the fact that `BufWriter::flush()` contains a *loop* that can happily issue multiple writes. This loop is completely opaque to the caller, so you can't even assert atomicity after-the-fact.

The PR makes the following changes:

* removes the paragraph that suggests concatenating strings to pass them to `write()` for atomicity or using `BufWriter`
* adds a paragraph explaining why attempting to use `write()` to append atomically is not a good idea.
  • Loading branch information
GuillaumeGomez authored Apr 16, 2024
2 parents ad18fe0 + d8745f9 commit 17d62bc
Showing 1 changed file with 18 additions and 9 deletions.
27 changes: 18 additions & 9 deletions library/std/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -980,15 +980,21 @@ impl OpenOptions {
/// Note that setting `.write(true).append(true)` has the same effect as
/// setting only `.append(true)`.
///
/// For most filesystems, the operating system guarantees that all writes are
/// atomic: no writes get mangled because another process writes at the same
/// time.
///
/// One maybe obvious note when using append-mode: make sure that all data
/// that belongs together is written to the file in one operation. This
/// can be done by concatenating strings before passing them to [`write()`],
/// or using a buffered writer (with a buffer of adequate size),
/// and calling [`flush()`] when the message is complete.
/// Append mode guarantees that writes will be positioned at the current end of file,
/// even when there are other processes or threads appending to the same file. This is
/// unlike <code>[seek]\([SeekFrom]::[End]\(0))</code> followed by `write()`, which
/// has a race between seeking and writing during which another writer can write, with
/// our `write()` overwriting their data.
///
/// Keep in mind that this does not necessarily guarantee that data appended by
/// different processes or threads does not interleave. The amount of data accepted a
/// single `write()` call depends on the operating system and file system. A
/// successful `write()` is allowed to write only part of the given data, so even if
/// you're careful to provide the whole message in a single call to `write()`, there
/// is no guarantee that it will be written out in full. If you rely on the filesystem
/// accepting the message in a single write, make sure that all data that belongs
/// together is written in one operation. This can be done by concatenating strings
/// before passing them to [`write()`].
///
/// If a file is opened with both read and append access, beware that after
/// opening, and after every write, the position for reading may be set at the
Expand All @@ -1003,6 +1009,9 @@ impl OpenOptions {
/// [`write()`]: Write::write "io::Write::write"
/// [`flush()`]: Write::flush "io::Write::flush"
/// [stream_position]: Seek::stream_position "io::Seek::stream_position"
/// [seek]: Seek::seek "io::Seek::seek"
/// [Current]: SeekFrom::Current "io::SeekFrom::Current"
/// [End]: SeekFrom::End "io::SeekFrom::End"
///
/// # Examples
///
Expand Down

0 comments on commit 17d62bc

Please sign in to comment.