Sync pack data more carefully when writing

Otherwise a system failure might leave a broken repo.  Note that these
changes may not be comprehensive, but they're a start, and hopefully
deal with some of the most critical data.

In addition to syncing the pack data itself, sync the parent directory
after the final rename so that we don't risk losing the reference to
the new file.

See also:

  http://austingroupbugs.net/view.php?id=672
  https://www.sqlite.org/atomiccommit.html

Signed-off-by: Rob Browning <rlb@defaultvalue.org>
Tested-by: Rob Browning <rlb@defaultvalue.org>

diff --git a/lib/bup/git.py b/lib/bup/git.py
index 50e1d47f2605584f41b368892cc922cf30be643f..64b59bb8b0ff73c4f3825ba0bcb969c994ea3a44 100644 (file)
--- a/lib/bup/git.py
+++ b/lib/bup/git.py
@@ -9,6 +9,7 @@ from itertools import islice
 
 from bup import _helpers, path, midx, bloom, xstat
 from bup.helpers import (Sha1, add_error, chunkyreader, debug1, debug2,
+                         fdatasync,
                          hostname, log, merge_iter, mmap_read, mmap_readwrite,
                          progress, qprogress, unlink, username, userfullname,
                          utc_offset_str)
@@ -568,6 +569,7 @@ class PackWriter:
     """Writes Git objects inside a pack file."""
     def __init__(self, objcache_maker=_make_objcache, compression_level=1):
         self.file = None
+        self.parentfd = None
         self.count = 0
         self.outbytes = 0
         self.filename = None
@@ -581,12 +583,20 @@ class PackWriter:
 
     def _open(self):
         if not self.file:
-            (fd,name) = tempfile.mkstemp(suffix='.pack', dir=repo('objects'))
+            objdir = dir=repo('objects')
+            fd, name = tempfile.mkstemp(suffix='.pack', dir=objdir)
             try:
                 self.file = os.fdopen(fd, 'w+b')
             except:
                 os.close(fd)
                 raise
+            try:
+                self.parentfd = os.open(objdir, os.O_RDONLY)
+            except:
+                f = self.file
+                self.file = None
+                f.close()
+                raise
             assert(name.endswith('.pack'))
             self.filename = name[:-5]
             self.file.write('PACK\0\0\0\2\0\0\0\0')
@@ -687,12 +697,18 @@ class PackWriter:
         """Remove the pack file from disk."""
         f = self.file
         if f:
-            self.idx = None
+            pfd = self.parentfd
             self.file = None
+            self.parentfd = None
+            self.idx = None
             try:
-                os.unlink(self.filename + '.pack')
+                try:
+                    os.unlink(self.filename + '.pack')
+                finally:
+                    f.close()
             finally:
-                f.close()
+                if pfd is not None:
+                    os.close(pfd)
 
     def _end(self, run_midx=True):
         f = self.file
@@ -716,6 +732,7 @@ class PackWriter:
                 sum.update(b)
             packbin = sum.digest()
             f.write(packbin)
+            fdatasync(f.fileno())
         finally:
             f.close()
 
@@ -726,6 +743,10 @@ class PackWriter:
             os.unlink(self.filename + '.map')
         os.rename(self.filename + '.pack', nameprefix + '.pack')
         os.rename(self.filename + '.idx', nameprefix + '.idx')
+        try:
+            fdatasync(self.parentfd)
+        finally:
+            os.close(self.parentfd)
 
         if run_midx:
             auto_midx(repo('objects/pack'))
@@ -748,10 +769,12 @@ class PackWriter:
         idx_f = open(filename, 'w+b')
         try:
             idx_f.truncate(index_len)
+            fdatasync(idx_f.fileno())
             idx_map = mmap_readwrite(idx_f, close=False)
             try:
                 count = _helpers.write_idx(filename, idx_map, idx, self.count)
                 assert(count == self.count)
+                idx_map.flush()
             finally:
                 idx_map.close()
         finally:
@@ -774,6 +797,7 @@ class PackWriter:
             for b in chunkyreader(idx_f):
                 idx_sum.update(b)
             idx_f.write(idx_sum.digest())
+            fdatasync(idx_f.fileno())
             return namebase
         finally:
             idx_f.close()