classification
Title: [mailbox] race: mbox may lose data with concurrent access
Type: Stage: resolved
Components: Library (Lib) Versions: Python 2.6
process
Status: closed Resolution: wont fix
Dependencies: Superseder: Always rewrite single-file mailboxes in-place
View: 15122
Assigned To: Nosy List: akuchling, doko, petri.lehtinen, pitrou, r.david.murray
Priority: normal Keywords:

Created on 2009-11-19 16:20 by doko, last changed 2012-06-26 06:39 by petri.lehtinen. This issue is now closed.

Files
File name Uploaded Description Edit
mailbox-race.py doko, 2009-11-19 16:20 test case
Messages (7)
msg95487 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2009-11-19 16:20
[forwarded from http://bugs.debian.org/451733]

the mailbox._singlefileMailbox class is not safe with concurrent access,
because mailbox._singlefileMailbox.flush() replaces the underlying file
with a new copy by constructing a temporary file and then renaming it.
This breaks all other class instances which have this mailbox open.  I'm
attaching a script demonstrating the problem. 

I think it's a bad idea to use rename(2) here; overwriting the file
content would fix the race condition, and #451274 too[1].
msg95491 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-11-19 16:52
> the mailbox._singlefileMailbox class is not safe with concurrent access,
> because mailbox._singlefileMailbox.flush() replaces the underlying file
> with a new copy by constructing a temporary file and then renaming it.
> This breaks all other class instances which have this mailbox open.

I don't think this class aims at being safe against concurrent access,
so having it fail loudly is a good thing.
Besides, the proposed cure (overwriting instead of renaming) looks worse
than the illness. The virtue of renaming is that it is atomic (on POSIX
systems at least), so you can't end up with a half-written mailbox if
there's a crash or an IO problem in the middle.
msg95494 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-11-19 17:06
Oops, sorry:
> I don't think this class aims at being safe against concurrent access,
> so having it fail loudly is a good thing.

I now understand that the problem is that it doesn't fail loudly. That's
what I get for replying too quickly.
Still, I don't think the suggested fix is ok. Perhaps we should simply
state in the documentation that flush() discards the old file, so that
other processes accessing it may get a surprise.
msg95496 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-11-19 17:18
Actually, the doc is quite clear about it:

« 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; try to avoid using single-file formats such as mbox
for concurrent writing. »
msg163109 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2012-06-18 17:40
Can this be closed?
msg163111 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-06-18 18:19
Probably.  Unless I'm mistaken, the issue with concurrent rewrite (as opposed to append) exists for single-file-mailboxes in the general case, not just in Python.  And as far as I know failure is never noisy, the last writer wins.
msg164048 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2012-06-26 06:38
I created #15122 for adding an option to rewrite the contents of single-file mailboxes in-place. Closing this issue as wontfix.
History
Date User Action Args
2012-06-26 06:39:01petri.lehtinensetstatus: open -> closed
superseder: Always rewrite single-file mailboxes in-place
messages: + msg164048

resolution: wont fix
stage: resolved
2012-06-18 18:19:31r.david.murraysetnosy: + r.david.murray
messages: + msg163111
2012-06-18 17:40:47petri.lehtinensetnosy: + petri.lehtinen
messages: + msg163109
2009-11-20 01:34:56pitrousetnosy: + akuchling
2009-11-19 17:18:28pitrousetmessages: + msg95496
2009-11-19 17:06:45pitrousetmessages: + msg95494
2009-11-19 16:52:18pitrousetnosy: + pitrou
messages: + msg95491
2009-11-19 16:20:41dokosetfiles: + mailbox-race.py
2009-11-19 16:20:21dokocreate