Simplify Maildir message saving

Previously we were attempting to save out mails according to
 http://www.qmail.org/man/man5/maildir.html in 4 steps:

1 Create a unique filename
2 Do stat(tmp/<filename>). If it found a file, wait 2 sec and go back to 1.
3 Create and write the message to the tmp/<filename>.
4 Link from tmp/* to new/*

(we did step 2 up to 15 times) But as stated by
http://wiki1.dovecot.org/MailboxFormat/Maildir (see section 'Issues with
the specification'), this is a pointless approach, e.g. there are race
issues between stating that the filename does not exist and the actual
moving (when it might exist).

So, we can simplify the steps as suggested in the dovecot wiki and
tighten up our safety at the same time.

One improvement that we do is to open the file, guaranteeing that it did
not exist before in an atomic manner, thus our simplified approach is
really more secure than what we had before.

Also, we throw an OfflineImapError at MESSAGE level when the supposedly
unique filename already exists, so that we can skip this message and
still continue with other messages.

Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Signed-off-by: Nicolas Sebrecht <nicolas.s-dev@laposte.net>
This commit is contained in:
Sebastian Spaeth 2011-06-13 15:22:37 +02:00 committed by Nicolas Sebrecht
parent e023f190b0
commit d8026c5308
2 changed files with 28 additions and 38 deletions

View File

@ -16,9 +16,12 @@ New Features
Changes Changes
------- -------
* Maildirs use less memory while syncing.
Bug Fixes Bug Fixes
--------- ---------
* Saving to Maildirs now checks for file existence without race conditions.
Pending for the next major release Pending for the next major release
================================== ==================================

View File

@ -28,6 +28,8 @@ try:
except ImportError: except ImportError:
from md5 import md5 from md5 import md5
from offlineimap import OfflineImapError
uidmatchre = re.compile(',U=(\d+)') uidmatchre = re.compile(',U=(\d+)')
flagmatchre = re.compile(':.*2,([A-Z]+)') flagmatchre = re.compile(':.*2,([A-Z]+)')
timestampmatchre = re.compile('(\d+)'); timestampmatchre = re.compile('(\d+)');
@ -217,11 +219,6 @@ class MaildirFolder(BaseFolder):
# Otherwise, save the message in tmp/ and then call savemessageflags() # Otherwise, save the message in tmp/ and then call savemessageflags()
# to give it a permanent home. # to give it a permanent home.
tmpdir = os.path.join(self.getfullname(), 'tmp') tmpdir = os.path.join(self.getfullname(), 'tmp')
messagename = None
attempts = 0
while 1:
if attempts > 15:
raise IOError, "Couldn't write to file %s" % messagename
timeval, timeseq = gettimeseq() timeval, timeseq = gettimeseq()
messagename = '%d_%d.%d.%s,U=%d,FMD5=%s' % \ messagename = '%d_%d.%d.%s,U=%d,FMD5=%s' % \
(timeval, (timeval,
@ -230,39 +227,29 @@ class MaildirFolder(BaseFolder):
socket.gethostname(), socket.gethostname(),
uid, uid,
md5(self.getvisiblename()).hexdigest()) md5(self.getvisiblename()).hexdigest())
if os.path.exists(os.path.join(tmpdir, messagename)): # open file and write it out
time.sleep(2) try:
attempts += 1 fd = os.open(os.path.join(tmpdir, messagename),
os.O_EXCL|os.O_CREAT|os.O_WRONLY)
except OSError, e:
if e.errno == 17:
#FILE EXISTS ALREADY
severity = OfflineImapError.ERROR.MESSAGE
raise OfflineImapError("Unique filename %s already existing." %\
messagename, severity)
else: else:
break raise
tmpmessagename = messagename.split(',')[0]
self.ui.debug('maildir', 'savemessage: using temporary name %s' %\
tmpmessagename)
file = open(os.path.join(tmpdir, tmpmessagename), "wt")
file.write(content)
file = os.fdopen(fd, 'wt')
file.write(content)
# Make sure the data hits the disk # Make sure the data hits the disk
file.flush() file.flush()
if self.dofsync: if self.dofsync:
os.fsync(file.fileno()) os.fsync(fd)
file.close() file.close()
if rtime != None: if rtime != None:
os.utime(os.path.join(tmpdir, tmpmessagename), (rtime, rtime)) os.utime(os.path.join(tmpdir, messagename), (rtime, rtime))
self.ui.debug('maildir', 'savemessage: moving from %s to %s' % \
(tmpmessagename, messagename))
if tmpmessagename != messagename: # then rename it
os.rename(os.path.join(tmpdir, tmpmessagename),
os.path.join(tmpdir, messagename))
if self.dofsync:
try:
# fsync the directory (safer semantics in Linux)
fd = os.open(tmpdir, os.O_RDONLY)
os.fsync(fd)
os.close(fd)
except:
pass
self.messagelist[uid] = {'uid': uid, 'flags': [], self.messagelist[uid] = {'uid': uid, 'flags': [],
'filename': os.path.join('tmp', messagename)} 'filename': os.path.join('tmp', messagename)}