| msg28902 - (view) |
Author: David Watson (baikie) |
Date: 2006-06-25 15:38 |
| fcntl/flock() locking of mailboxes is actually disabled due to a typo: --- mailbox.py.orig +++ mailbox.py @@ -15,7 +15,7 @@ import rfc822 import StringIO try: - import fnctl + import fcntl except ImportError: fcntl = None However, once enabled, it doesn't work on FreeBSD - the lock() method always raises ExternalClashError (tested on 5.3). This is because flock(), lockf() and fcntl locking share the same underlying mechanism on this system (and probably others), and so the second lock can never be acquired. |
|
|
| msg28903 - (view) |
Author: Neal Norwitz (nnorwitz) *  |
Date: 2006-06-25 19:52 |
| Logged In: YES user_id=33168 Andrew, are you helping maintain this module? David is correct that fcntl is mispelled in current svn. Not sure what to do about the locking issue. David, would it be possible for you to work on a patch to correct this problem? I'm not sure if any developer has access to a FreeBSD box similar to yours. (There are a couple that use FreeBSD though.) It would be great to come up with tests for this if the current ones don't stress this situation. |
|
|
| msg28904 - (view) |
Author: A.M. Kuchling (akuchling) * |
Date: 2006-06-26 12:56 |
| Logged In: YES user_id=11375 Interesting. The man page for flock() on my Linux system says of flock() in a certain version: "This yields true BSD semantics: there is no interaction between the types of lock placed by flock(2) and fcntl(2), and flock(2) does not detect deadlock." Apparently this is out of date, and placing two locks is harmful. I think it's best to just use one set of locking primitives, and that one should be lockf(); the flock() calls should be removed. |
|
|
| msg28905 - (view) |
Author: A.M. Kuchling (akuchling) * |
Date: 2006-06-26 13:27 |
| Logged In: YES user_id=11375 Typo fixed in rev. 47099; flock() calls removed in rev. 47100; a test for lock conflicts added in rev. 47101. David, please try out the current SVN head and let me know if matters have improved. I'll wait to see the buildbot reports before closing this bug. (Surprisingly, we don't seem to have a FreeBSD buildbot, though we do have an OpenBSD one that didn't seem to have a problem with the original version of the mailbox code.) |
|
|
| msg28906 - (view) |
Author: David Watson (baikie) |
Date: 2006-06-26 22:56 |
| Logged In: YES user_id=1504904 Yeah, it works fine now (locks successfully when the mailbox isn't locked by another process, and fails when it is locked), although I was reminded that the Python install process didn't work properly (I've submitted another bug report). However, I should point out that flock() locking is required when working with qmail on (for example) Linux - flock() is the only locking mechanism it uses, and as noted below, flock() on Linux is independent from fcntl/lockf(). |
|
|
| msg28907 - (view) |
Author: A.M. Kuchling (akuchling) * |
Date: 2006-06-26 23:24 |
| Logged In: YES user_id=11375 Hm, this looks messy; I doubt the module can know if you want lockf() or fcntl() locking. Maybe Mailbox.lock() should grow an optional parameter that lets you specify the locking mechanism to use -- lockf, fcntl, or both. (But defaulting to what?) On the other hand, maybe it's too late to go making API changes for beta2 (but I suspect it would be OK in this case). |
|
|
| msg28908 - (view) |
Author: Neal Norwitz (nnorwitz) * |
Date: 2006-06-27 03:21 |
| Logged In: YES user_id=33168 Is the optional parameter necessary if you use inheritance (assuming there's a class/object in there)? Maybe it would be better to have a subclass? Would that be better for us to provide or just to add something to the docs that a user could make a subclass to work around locking issues? I haven't looked at the issues, but if we really need an API change, I'm probably ok with it since it would seem to be quite small. If an API change is really required (doc isn't sufficient), I'd like it to go in ASAP. We could doc for 2.5 and if there's a problem fix for 2.6. |
|
|
| msg28909 - (view) |
Author: David Watson (baikie) |
Date: 2006-11-12 23:52 |
| Logged In: YES user_id=1504904 I'd forgotten about this for a while, but I see the docs didn't actually get changed for 2.5. I was going to submit a patch for them today and suggest using subclasses as described below, but then I realized that subclassing wouldn't work, since the code makes use of the module functions _lock_file() and _unlock_file() outside of the lock()/unlock() methods. A possible solution is attached; the Mailbox class is given methods _acquire_lock() and _release_lock() wrapping _lock_file() and _unlock_file(), and these are used instead (thus, they can be overridden as methods, but if someone had replaced _lock_file() in the module namespace instead, then their code would still work). Also attached is a minimal doc patch to describe the new (shorter) list of locking methods. I've called the system-call locking "fcntl()" since that's the usual term when dealing with mailboxes, and the name fcntl.lockf() is misleading (it calls fcntl() directly, not lockf(), and the Linux man pages say that "In general, the relation between lockf() and fcntl() is unspecified."). |
|
|
| msg114673 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2010-08-22 11:02 |
| Is this still an issue on later versions of Python and/or FreeBSD? |
|
|
| msg115013 - (view) |
Author: David Watson (baikie) |
Date: 2010-08-26 18:03 |
| > Is this still an issue on later versions of Python and/or FreeBSD? Yes, there is still an issue. There is no longer a deadlock on FreeBSD because the module been changed to use only lockf() and dot-locking (on all platforms), but the issue is now about how users can enable other locking mechanisms that they need, such as flock(), without causing a deadlock on platforms where they refer to the same lock as lockf(). They can't just override the classes' .lock() and .unlock() methods, because some parts of the code perform locking operations directly without calling those methods. |
|
|
| msg407749 - (view) |
Author: Irit Katriel (iritkatriel) * |
Date: 2021-12-05 21:58 |
| The original bug has been fixed, so I am closing this. Please create a new issue (probably of type enhancement) for the .lock()/.unlock() override support if this is still an issue. The use case/test can be discussed on that issue. |
|
|