Author baikie
Recipients Jim.Jewett, ajaksu2, akuchling, baikie, barry, loewis, nnorwitz, peter.ll, petri.lehtinen, r.david.murray, terry.reedy
Date 2014-03-23.22:57:20
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <20140323225502.GA10247@dbwatson.uk7.net>
In-reply-to <1395169728.29.0.243584984813.issue1599254@psf.upfronthosting.co.za>
Content
On Tue 18 Mar 2014, A.M. Kuchling wrote:
> I suggest we apply the fix for #1, and for #2 just discard and
> update the ToC when we lock the mailbox, ignoring other
> possible routes to corruption (so no detecting the problem and
> raising an ExternalClash exception).  Since 2007 the docs have
> said "If you’re modifying a mailbox, you must lock it by
> calling the lock() and unlock() methods before reading any
> messages in the file or making any changes".

Well, the warning you're referring to begins "Be very cautious
when modifying mailboxes that might be simultaneously changed by
some other process. The safest mailbox format to use for such
tasks is Maildir...", suggesting that it only applies to
mailboxes that might in fact be modified by another process.

But if a reread is forced *simply by discarding the ToC*, then
the application's own keys become invalid if it has,
e.g. previously deleted a message, even if it's a private mailbox
that no other process ever touches.  So such a change would make
the warning apply to such mailboxes, whereas previously it (in
effect) did not.

(That does raise the question of why the application is calling
.lock() at all on a mailbox that won't be modified by another
process, but if the programmer thought the warning didn't apply
to their situation, then they presumably wouldn't think that
calling .lock() after modifying might actually be dangerous - and
it currently isn't dangerous for such a mailbox.)

Anyway, I've rebased the mailbox-unified2 patch onto 2.7,
renaming it as mailbox-copy-back-2.7.diff; this fixes the
original (renaming and fcntl locking) issue and adds some
warnings about locking.  I've combined all the tests (I think)
into a separate patch, and ported them to the multiprocessing
module (possibly unwise as I'm not very familiar with it - it
would be nice if someone could test it on Windows).  I haven't
looked at the tests closely again, but they appear to check for
many of the ToC issues.

There actually isn't a test for the original locking issue, as
the tests all use the mailbox API, which doesn't provide a way to
turn off dot-locking.  Also, the module no longer rewrites the
mailbox if messages have only been appended to it - it just syncs
it instead.  However, I have verified by hand that the problem is
still there when the mailbox *is* rewritten due to deletions,
etc.
Files
File name Uploaded
mailbox-all-tests-2.7.diff baikie, 2014-03-23.22:57:18
mailbox-copy-back-2.7.diff baikie, 2014-03-23.22:57:17
History
Date User Action Args
2014-03-23 22:57:22baikiesetrecipients: + baikie, loewis, barry, akuchling, nnorwitz, terry.reedy, ajaksu2, peter.ll, r.david.murray, petri.lehtinen, Jim.Jewett
2014-03-23 22:57:20baikielinkissue1599254 messages
2014-03-23 22:57:20baikiecreate