mailbox.mbox creates new file when adding message to mbox

[chrisisbd]

BPO	9559
Nosy	@warsaw, @akuchling, @bitdancer, @akheron, @lilydjwg
Files	issue9559.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2012-06-29.11:15:52.753>
created_at = <Date 2010-08-10.16:40:07.358>
labels = ['type-bug', 'library', 'expert-email']
title = 'mailbox.mbox creates new file when adding message to mbox'
updated_at = <Date 2012-06-29.11:15:52.752>
user = 'https://bugs.python.org/chrisisbd'

bugs.python.org fields:

activity = <Date 2012-06-29.11:15:52.752>
actor = 'petri.lehtinen'
assignee = 'none'
closed = True
closed_date = <Date 2012-06-29.11:15:52.753>
closer = 'petri.lehtinen'
components = ['Library (Lib)', 'email']
creation = <Date 2010-08-10.16:40:07.358>
creator = 'chrisisbd'
dependencies = []
files = ['26167']
hgrepos = []
issue_num = 9559
keywords = ['patch']
message_count = 9.0
messages = ['113547', '163093', '164061', '164087', '164240', '164241', '164242', '164254', '164321']
nosy_count = 7.0
nosy_names = ['barry', 'akuchling', 'r.david.murray', 'chrisisbd', 'python-dev', 'petri.lehtinen', 'lilydjwg']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue9559'
versions = ['Python 2.7', 'Python 3.2', 'Python 3.3']

[chrisisbd]

When you call mailbox.mbox.add() the old mbox file is copied, the new file is modified and then renamed to the name of the'old' mbox file.

This breaks the way that many MUAs detect and manage new mail in an mbox, in particular I discovered this with mutt. If the python process writing the mbox and mutt are on the same system writing a local file then you get the message "Mailbox was externally modified. Flags may be wrong." from mutt (and various odd things can happen). If mutt is reading the mbox over NFS then you get a "Stale NFS file handle" error.

This should be strongly noted in the documentation for mailbox.mbox, in addition it would be really nice if there was a mailbox.mbox.append() method which *really* appends the data to the end of the mbox rather than changing it completely.

Most MDAs (all?) do just append new mail to the end of the mbox and I feel that python should really try and do the same.

[akheron]

This is actually not true. When calling add(), mbox (and MMDF and Babyl) append the message to the file without rewriting it.

It's the following flush() call that rewrites the whole mailbox contents. I think this could be changed to work correctly by not setting self._pending = True in _singlefileMailbox.add. This way, the file wouldn't be rewritten by flush() if messages are only appended.

OTOH, flush() should still fsync the mailbox file (if we want to ensure that the changes are really written to disk). This would probably require a new flag in addition to self._pending, to indicate that there are unsynced changes.

[akheron]

Attached a patch that doesn't rewrite+rename if messages have only been added. In this case, flush() only syncs the mailbox file to make sure all changes have been written to disk.

David & Barry: what do you think about including this on bugfix releases? Could someone depend on the file being rewritten in all situations?

[bitdancer]

Could someone be depending on it? Sure. Is that likely enough to block this as a bug fix? Personally I think not. Appending to the mailbox when adding messages is, I think, the expected behavior, and always rewriting it is the surprising behavior.

The patch looks good to me, though personally I'd eliminate the extra blank lines.

[python-dev]

New changeset c37cb11b546f by Petri Lehtinen in branch '2.7':
bpo-9559: Append data to single-file mailbox files if messages are only added
http://hg.python.org/cpython/rev/c37cb11b546f

New changeset 5f447a005d67 by Petri Lehtinen in branch '3.2':
bpo-9559: Append data to single-file mailbox files if messages are only added
http://hg.python.org/cpython/rev/5f447a005d67

New changeset 0bacbab678ed by Petri Lehtinen in branch 'default':
bpo-9559: Append data to single-file mailbox files if messages are only added
http://hg.python.org/cpython/rev/0bacbab678ed

[akheron]

Fixed. I removed the extra newlines.

[akheron]

See bpo-15122 for always modifying single-file mailboxes in-place.

[akheron]

The _pre_mailbox_hook may be called twice, like this:

babyl = mailbox.Babyl('new_file')
babyl.add('foo\n')
babyl.remove(0)
babyl.add('bar\n')

This only affects Babyl, that writes the mailbox header in _pre_mailbox_hook. The mailbox is corrupted on disk until flush() is called.

[python-dev]

New changeset 3d7a75e945ee by Petri Lehtinen in branch '2.7':
bpo-9559: Don't call _pre_mailbox_hook more than once
http://hg.python.org/cpython/rev/3d7a75e945ee

New changeset 7cf5a629fde2 by Petri Lehtinen in branch '3.2':
bpo-9559: Don't call _pre_mailbox_hook more than once
http://hg.python.org/cpython/rev/7cf5a629fde2

New changeset 5a0ec296b287 by Petri Lehtinen in branch 'default':
bpo-9559: Don't call _pre_mailbox_hook more than once
http://hg.python.org/cpython/rev/5a0ec296b287