Page MenuHomePhabricator

treedirstate: ensure appended data is flushed to disk
ClosedPublic

Authored by mbthomas on Dec 11 2017, 10:12 AM.

Details

Reviewers
ikostia
Group Reviewers
Restricted Project
Commits
rFBHGX37bccca1426f: treedirstate: ensure appended data is flushed to disk
Summary

A dirstate update that appends data to the treedirstate tree file, followed by
a hard reboot before the filesystem cache is flushed, can result in a dirstate
tree root that is referred to by the dirstate file, but does not contain the
correct data. Ensure the appended data is synced to disk before returning from
Store.flush().

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mbthomas created this revision.Dec 11 2017, 10:12 AM
Herald added a reviewer: Restricted Project. · View Herald TranscriptDec 11 2017, 10:12 AM
ikostia accepted this revision.Dec 11 2017, 11:29 AM
This revision is now accepted and ready to land.Dec 11 2017, 11:29 AM
This revision was automatically updated to reflect the committed changes.
jsgf added a subscriber: jsgf.Dec 15 2017, 2:18 PM
jsgf added inline comments.
rust/treedirstate/src/filestore.rs
157

This should probably be sync_all(), since you do want to sync metadata as well (the file size). There's no guarantee that sync_data() will also sync the file size change, meaning that your data is safely written but inaccessible after the logical end of the file. (It is intended for overwrite cases where you want to write into the middle of a file and don't care whether the mtime update is durable.)