From: Andrew M. Kuchling Date: Fri, 10 Nov 2006 13:08:03 +0000 (+0000) Subject: [Patch #1514544 by David Watson] use fsync() to ensure data is really on disk X-Git-Tag: v2.5.1c1~259 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=bb876b9c69f80c253621937ad734eafef3086e5f;p=python [Patch #1514544 by David Watson] use fsync() to ensure data is really on disk --- diff --git a/Lib/mailbox.py b/Lib/mailbox.py index e66dddec55..51c2ccc80f 100755 --- a/Lib/mailbox.py +++ b/Lib/mailbox.py @@ -2,6 +2,12 @@ """Read/write support for Maildir, mbox, MH, Babyl, and MMDF mailboxes.""" +# Notes for authors of new mailbox subclasses: +# +# Remember to fsync() changes to disk before closing a modified file +# or returning from a flush() method. See functions _sync_flush() and +# _sync_close(). + import sys import os import time @@ -238,7 +244,7 @@ class Maildir(Mailbox): try: self._dump_message(message, tmp_file) finally: - tmp_file.close() + _sync_close(tmp_file) if isinstance(message, MaildirMessage): subdir = message.get_subdir() suffix = self.colon + message.get_info() @@ -565,7 +571,8 @@ class _singlefileMailbox(Mailbox): new_file.close() os.remove(new_file.name) raise - new_file.close() + _sync_close(new_file) + # self._file is about to get replaced, so no need to sync. self._file.close() try: os.rename(new_file.name, self._path) @@ -599,7 +606,7 @@ class _singlefileMailbox(Mailbox): self.flush() if self._locked: self.unlock() - self._file.close() + self._file.close() # Sync has been done by self.flush() above. def _lookup(self, key=None): """Return (start, stop) or raise KeyError.""" @@ -789,7 +796,7 @@ class MH(Mailbox): if self._locked: _unlock_file(f) finally: - f.close() + _sync_close(f) return new_key def remove(self, key): @@ -836,7 +843,7 @@ class MH(Mailbox): if self._locked: _unlock_file(f) finally: - f.close() + _sync_close(f) def get_message(self, key): """Return a Message representation or raise a KeyError.""" @@ -923,7 +930,7 @@ class MH(Mailbox): """Unlock the mailbox if it is locked.""" if self._locked: _unlock_file(self._file) - self._file.close() + _sync_close(self._file) del self._file self._locked = False @@ -1020,7 +1027,7 @@ class MH(Mailbox): else: f.write('\n') finally: - f.close() + _sync_close(f) def pack(self): """Re-name messages to eliminate numbering gaps. Invalidates keys.""" @@ -1874,6 +1881,15 @@ def _create_temporary(path): socket.gethostname(), os.getpid())) +def _sync_flush(f): + """Ensure changes to file f are physically on disk.""" + f.flush() + os.fsync(f.fileno()) + +def _sync_close(f): + """Close file f, ensuring all changes are physically on disk.""" + _sync_flush(f) + f.close() ## Start: classes from the original module (for backward compatibility). diff --git a/Misc/NEWS b/Misc/NEWS index 8e258a4109..d91e64dbc5 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -137,8 +137,12 @@ Library weren't passing the message factory on to newly created Maildir/MH objects. -- Bug #1575506: Single-file mailboxes didn't re-lock properly in - their flush() method. +- Bug #1575506: mailbox.py: Single-file mailboxes didn't re-lock + properly in their flush() method. + +- Patch #1514544: mailbox.py: Try to ensure that messages/indexes have + been physically written to disk after calling .flush() or + .close(). (Patch by David Watson.) - Bug #1576241: fix functools.wraps() to work on built-in functions.