[Mailman-Developers] problem with view other subscriptions..
Harald Meland
Harald.Meland@usit.uio.no
18 May 2000 22:58:20 +0200
--=-=-=
[Matt Davis]
> On 18 May 2000, Harald Meland wrote:
> > Try again, but only after you have put
> >
> > LIST_LOCK_DEBUGGING = 1
> >
> > in your Mailman/mm_cfg.py, as current CVS Mailman has debug lock
> > logging turned off by default.
>
> Did that and got this..
>
> [mailman@dogpound logs]$ cat locks
> May 18 14:46:29 2000 (9011) davis.lock laying claim
> May 18 14:46:29 2000 (9011) davis.lock unexpected linkcount <> 2: 1
> May 18 14:46:29 2000 (9011) davis.lock lifetime has expired, breaking
> May 18 14:46:30 2000 (9011) davis.lock got the lock
> May 18 14:46:30 2000 (9011) davis.lock laying claim
> May 18 14:46:30 2000 (9011) davis.lock already locked
> [mailman@dogpound logs]$
The above strangeness was probably caused by a buglet in the current
CVS LockFile.py; I believe it gets the order of unlink() calls wrong.
As I see it, it is more important for the lock file to never have a
link count that is neither 0 or 2 than it is to make sure there are no
tempfile turds. This implies that the real lock file should be
unlink()ed before the tempfile, and not the other way around. Here's
a (untested) patch (which also touches on some other issues I noticed
while I was at it :):
--=-=-=
Content-Type: text/x-patch
Content-Disposition: inline
Index: LockFile.py
===================================================================
RCS file: /cvsroot/mailman/mailman/Mailman/LockFile.py,v
retrieving revision 1.17
diff -u -r1.17 LockFile.py
--- LockFile.py 2000/05/08 15:35:09 1.17
+++ LockFile.py 2000/05/18 20:22:27
@@ -259,7 +259,10 @@
elif self.__read() == self.__tmpfname:
# It was us that already had the link.
self.__writelog('already locked')
- os.unlink(self.__tmpfname)
+ # HM: I don't understand why the tempfile should be
+ # unlinked in this situation. I think that all it will do
+ # is confuse things, so I've commented it out.
+ ## os.unlink(self.__tmpfname)
raise AlreadyLockedError
# otherwise, someone else has the lock
pass
@@ -287,20 +290,11 @@
If we don't already own the lock (either because of unbalanced unlock
calls, or because the lock was stolen out from under us), raise a
- NotLockedError, unless optional `unconditional' is true.
+ NotLockedError, unless optional `unconditionally' is true.
"""
- islocked = 0
- try:
- islocked = self.locked()
- except OSError, e:
- if e.errno <> errno.ENOENT: raise
+ islocked = self.locked()
if not islocked and not unconditionally:
raise NotLockedError
- # Remove our tempfile
- try:
- os.unlink(self.__tmpfname)
- except OSError, e:
- if e.errno <> errno.ENOENT: raise
# If we owned the lock, remove the global file, relinquishing it.
if islocked:
try:
@@ -308,6 +302,11 @@
except OSError, e:
if e.errno <> errno.ENOENT: raise
self.__writelog('unlocked')
+ # Remove our tempfile
+ try:
+ os.unlink(self.__tmpfname)
+ except OSError, e:
+ if e.errno <> errno.ENOENT: raise
def locked(self):
"""Returns 1 if we own the lock, 0 if we do not.
@@ -394,21 +393,27 @@
self.__touch(self.__lockfile)
except OSError, e:
if e.errno <> errno.EPERM: raise
- # Try to remove the old winner's temp file, since we're assuming the
- # winner process has hung or died. Don't worry too much if we can't
- # unlink their temp file -- this doesn't break the locking algorithm,
- # but will leave temp file turds laying around, a minor inconvenience.
- try:
- winner = self.__read()
- if winner:
- os.unlink(winner)
- except OSError, e:
- if e.errno <> errno.ENOENT: raise
+ # Try finding the name of the old winner's temp file. We're assuming
+ # the winner process has hung or died, so we should try cleaning up
+ # their mess.
+ winner = self.__read()
# Now remove the global lockfile, which actually breaks the lock.
+ # This should be done before the temp file is unlinked, or other lock
+ # claimants will be confused over a lock file whose link count is
+ # neither 0 or 2.
try:
os.unlink(self.__lockfile)
except OSError, e:
if e.errno <> errno.ENOENT: raise
+ # Finally, if we found the old winner's temp filename, try to unlink
+ # it. Don't worry too much if this doesn't succeed -- it won't break
+ # the locking algorithm, but will leave temp file turds laying around,
+ # a minor inconvenience.
+ if winner:
+ try:
+ os.unlink(winner)
+ except OSError, e:
+ if e.errno <> errno.ENOENT: raise
def __sleep(self):
interval = random.random() * 2.0 + 0.01
--=-=-=
Content-Disposition: inline
I noticed another apparent inconsistency in LockFile.py, too: The
comment at the start of __break() seems to imply that calling
__touch() will totally remove the race condition, while I believe all
it does is make the race condition a little less likely.
However, I don't think that neither the order of unlink calls nor any
race condition is responsible for the problem you're having -- in
fact, I believe everyone running current CVS Mailman will have the
exact same problem. Here's another (untested) patch which tries to
address the problem (these patches are not supposed to depend on the
above patch, BTW):
--=-=-=
Content-Type: text/x-patch
Content-Disposition: inline
Index: Mailman/Utils.py
===================================================================
RCS file: /cvsroot/mailman/mailman/Mailman/Utils.py,v
retrieving revision 1.87
diff -u -r1.87 Utils.py
--- Mailman/Utils.py 2000/04/09 21:03:31 1.87
+++ Mailman/Utils.py 2000/05/18 20:52:33
@@ -409,16 +409,20 @@
-def map_maillists(func, names=None, unlock=None, verbose=0):
+def map_maillists(func, names=None, lock=1, verbose=0, from_mlist=None):
"""Apply function (of one argument) to all list objs in turn.
Returns a list of the results.
Optional arg 'names' specifies which lists, default all.
- Optional arg unlock says to unlock immediately after instantiation.
+ Optional arg lock says whether or not to lock the list on instantiation.
Optional arg verbose says to print list name as it's about to be
instantiated, CR when instantiation is complete, and result of
- application as it shows."""
+ application as it shows.
+ Optional arg from_mlist is an already instantiated list object, which
+ should neither be re-instantiated nor unlocked.
+
+ """
from Mailman import MailList
if names == None:
names = list_names()
@@ -426,15 +430,16 @@
for i in names:
if verbose:
print i,
- l = MailList.MailList(i)
+ if mlist and mlist.internal_name() == i:
+ l = mlist
+ else:
+ MailList.MailList(i, lock=lock)
if verbose:
print
- if unlock and l.Locked():
- l.Unlock()
got.append(apply(func, (l,)))
if verbose:
print got[-1]
- if not unlock:
+ if not (mlist and mlist.internal_name() == i):
l.Unlock()
del l
return got
--=-=-=
Content-Type: text/x-patch
Content-Disposition: inline
Index: Mailman/Cgi/handle_opts.py
===================================================================
RCS file: /cvsroot/mailman/mailman/Mailman/Cgi/handle_opts.py,v
retrieving revision 1.22
diff -u -r1.22 handle_opts.py
--- Mailman/Cgi/handle_opts.py 2000/04/04 23:42:18 1.22
+++ Mailman/Cgi/handle_opts.py 2000/05/18 20:52:53
@@ -164,7 +164,8 @@
link = Link(url, l.real_name)
return l._internal_name, link
- all_links = filter(None, Utils.map_maillists(optionslinks))
+ all_links = filter(None, Utils.map_maillists(optionslinks,
+ from_mlist=mlist))
all_links.sort()
items = OrderedList()
for name, link in all_links:
--=-=-=
Content-Disposition: inline
HTH, HAND,
--
Harald
--=-=-=--