Skip to content

Commit

Permalink
ImportantFileWriter: Mention non-guarantee of persistence in comments.
Browse files Browse the repository at this point in the history
Also remove the bulk of the existing commentary which over-promises the benefit
of ImportantFileWriter, provides unnecessary technical detail and contains an
outdated reference.

BUG=none

Review-Url: https://codereview.chromium.org/2201383002
Cr-Commit-Position: refs/heads/master@{#412345}
  • Loading branch information
thiemonagel authored and Commit bot committed Aug 16, 2016
1 parent a8ba91f commit c1d331a
Showing 1 changed file with 15 additions and 16 deletions.
31 changes: 15 additions & 16 deletions base/files/important_file_writer.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,22 +22,20 @@ namespace base {
class SequencedTaskRunner;
class Thread;

// Helper to ensure that a file won't be corrupted by the write (for example on
// application crash). Consider a naive way to save an important file F:
// Helper for atomically writing a file to ensure that it won't be corrupted by
// *application* crash during write (implemented as create, flush, rename).
//
// 1. Open F for writing, truncating it.
// 2. Write new data to F.
// As an added benefit, ImportantFileWriter makes it less likely that the file
// is corrupted by *system* crash, though even if the ImportantFileWriter call
// has already returned at the time of the crash it is not specified which
// version of the file (old or new) is preserved. And depending on system
// configuration (hardware and software) a significant likelihood of file
// corruption may remain, thus using ImportantFileWriter is not a valid
// substitute for file integrity checks and recovery codepaths for malformed
// files.
//
// It's good when it works, but it gets very bad if step 2. doesn't complete.
// It can be caused by a crash, a computer hang, or a weird I/O error. And you
// end up with a broken file.
//
// To be safe, we don't start with writing directly to F. Instead, we write to
// to a temporary file. Only after that write is successful, we rename the
// temporary file to target filename.
//
// If you want to know more about this approach and ext3/ext4 fsync issues, see
// http://blog.valerieaurora.org/2009/04/16/dont-panic-fsync-ext34-and-your-data/
// Also note that ImportantFileWriter can be *really* slow (cf. File::Flush()
// for details) and thus please don't block shutdown on ImportantFileWriter.
class BASE_EXPORT ImportantFileWriter : public NonThreadSafe {
public:
// Used by ScheduleSave to lazily provide the data to be saved. Allows us
Expand All @@ -53,8 +51,9 @@ class BASE_EXPORT ImportantFileWriter : public NonThreadSafe {
virtual ~DataSerializer() {}
};

// Save |data| to |path| in an atomic manner (see the class comment above).
// Blocks and writes data on the current thread.
// Save |data| to |path| in an atomic manner. Blocks and writes data on the
// current thread. Does not guarantee file integrity across system crash (see
// the class comment above).
static bool WriteFileAtomically(const FilePath& path, StringPiece data);

// Initialize the writer.
Expand Down

0 comments on commit c1d331a

Please sign in to comment.