diff --git a/Changelog.draft.rst b/Changelog.draft.rst index 238a592..83aa357 100644 --- a/Changelog.draft.rst +++ b/Changelog.draft.rst @@ -16,16 +16,19 @@ New Features Changes ------- +* Maildirs use less memory while syncing. + Bug Fixes --------- +* Saving to Maildirs now checks for file existence without race conditions. * A bug in the underlying imap library has been fixed that could potentially lead to data loss if the server interrupted responses with unexpected but legal server status responses. This would mainly occur in folders with many thousands of emails. Upgrading from the previous release is strongly recommended. -Peanding for the next major release +Pending for the next major release ================================== * UIs get shorter and nicer names. (API changing) diff --git a/offlineimap/folder/Maildir.py b/offlineimap/folder/Maildir.py index e360619..b576507 100644 --- a/offlineimap/folder/Maildir.py +++ b/offlineimap/folder/Maildir.py @@ -28,6 +28,8 @@ try: except ImportError: from md5 import md5 +from offlineimap import OfflineImapError + uidmatchre = re.compile(',U=(\d+)') flagmatchre = re.compile(':.*2,([A-Z]+)') timestampmatchre = re.compile('(\d+)'); @@ -63,12 +65,15 @@ class MaildirFolder(BaseFolder): self.accountname = accountname BaseFolder.__init__(self) #self.ui is set in BaseFolder.init() + # Cache the full folder path, as we use getfullname() very often + self._fullname = os.path.join(self.getroot(), self.getname()) def getaccountname(self): return self.accountname def getfullname(self): - return os.path.join(self.getroot(), self.getname()) + """Return the absolute file path to the Maildir folder (sans cur|new)""" + return self._fullname def getuidvalidity(self): """Maildirs have no notion of uidvalidity, so we just return a magic @@ -182,81 +187,73 @@ class MaildirFolder(BaseFolder): return self.messagelist def getmessage(self, uid): + """Return the content of the message""" filename = self.messagelist[uid]['filename'] - file = open(filename, 'rt') + filepath = os.path.join(self.getfullname(), filename) + file = open(filepath, 'rt') retval = file.read() file.close() + #TODO: WHY are we replacing \r\n with \n here? And why do we + # read it as text? return retval.replace("\r\n", "\n") def getmessagetime( self, uid ): filename = self.messagelist[uid]['filename'] - st = os.stat(filename) + filepath = os.path.join(self.getfullname(), filename) + st = os.stat(filepath) return st.st_mtime def savemessage(self, uid, content, flags, rtime): # This function only ever saves to tmp/, # but it calls savemessageflags() to actually save to cur/ or new/. - self.ui.debug('maildir', 'savemessage: called to write with flags %s and content %s' % \ - (repr(flags), repr(content))) + self.ui.debug('maildir', 'savemessage: called to write with flags %s ' + 'and content %s' % (repr(flags), repr(content))) if uid < 0: # We cannot assign a new uid. return uid if uid in self.messagelist: - # We already have it. + # We already have it, just update flags. self.savemessageflags(uid, flags) return uid # Otherwise, save the message in tmp/ and then call savemessageflags() # to give it a permanent home. 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() - messagename = '%d_%d.%d.%s,U=%d,FMD5=%s' % \ - (timeval, - timeseq, - os.getpid(), - socket.gethostname(), - uid, - md5(self.getvisiblename()).hexdigest()) - if os.path.exists(os.path.join(tmpdir, messagename)): - time.sleep(2) - attempts += 1 + timeval, timeseq = gettimeseq() + messagename = '%d_%d.%d.%s,U=%d,FMD5=%s' % \ + (timeval, + timeseq, + os.getpid(), + socket.gethostname(), + uid, + md5(self.getvisiblename()).hexdigest()) + # open file and write it out + try: + 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: - break - 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) + raise + file = os.fdopen(fd, 'wt') + file.write(content) # Make sure the data hits the disk file.flush() if self.dofsync: - os.fsync(file.fileno()) - + os.fsync(fd) file.close() - if rtime != None: - os.utime(os.path.join(tmpdir,tmpmessagename), (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 + if rtime != None: + os.utime(os.path.join(tmpdir, messagename), (rtime, rtime)) self.messagelist[uid] = {'uid': uid, 'flags': [], - 'filename': os.path.join(tmpdir, messagename)} + 'filename': os.path.join('tmp', messagename)} + # savemessageflags moves msg to 'cur' or 'new' as appropriate self.savemessageflags(uid, flags) self.ui.debug('maildir', 'savemessage: returning uid %d' % uid) return uid @@ -266,14 +263,14 @@ class MaildirFolder(BaseFolder): def savemessageflags(self, uid, flags): oldfilename = self.messagelist[uid]['filename'] - newpath, newname = os.path.split(oldfilename) + dir_prefix, newname = os.path.split(oldfilename) tmpdir = os.path.join(self.getfullname(), 'tmp') if 'S' in flags: # If a message has been seen, it goes into the cur - # directory. CR debian#152482, [complete.org #4] - newpath = os.path.join(self.getfullname(), 'cur') + # directory. CR debian#152482 + dir_prefix = 'cur' else: - newpath = os.path.join(self.getfullname(), 'new') + dir_prefix = 'new' infostr = ':' infomatch = re.search('(:.*)$', newname) if infomatch: # If the info string is present.. @@ -284,15 +281,16 @@ class MaildirFolder(BaseFolder): infostr += '2,' + ''.join(flags) newname += infostr - newfilename = os.path.join(newpath, newname) + newfilename = os.path.join(dir_prefix, newname) if (newfilename != oldfilename): - os.rename(oldfilename, newfilename) + os.rename(os.path.join(self.getfullname(), oldfilename), + os.path.join(self.getfullname(), newfilename)) self.messagelist[uid]['flags'] = flags self.messagelist[uid]['filename'] = newfilename # By now, the message had better not be in tmp/ land! final_dir, final_name = os.path.split(self.messagelist[uid]['filename']) - assert final_dir != tmpdir + assert final_dir != 'tmp' def deletemessage(self, uid): """Unlinks a message file from the Maildir. @@ -306,13 +304,16 @@ class MaildirFolder(BaseFolder): return filename = self.messagelist[uid]['filename'] + filepath = os.path.join(self.getfullname(), filename) try: - os.unlink(filename) + os.unlink(filepath) except OSError: # Can't find the file -- maybe already deleted? newmsglist = self._scanfolder() if uid in newmsglist: # Nope, try new filename. - os.unlink(newmsglist[uid]['filename']) + filename = newmsglist[uid]['filename'] + filepath = os.path.join(self.getfullname(), filename) + os.unlink(filepath) # Yep -- return. del(self.messagelist[uid])