Patches item #1045656, was opened at 2004-10-12 19:50 Message generated for change (Settings changed) made by tkikuchi You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=300103&aid=1045656&group_id=103 Category: mail delivery Group: Mailman 2.1 Status: Open Resolution: None Priority: 3 Submitted By: Brian Greenberg (grnbrg)
Assigned to: Tokio Kikuchi (tkikuchi) Summary: Race in LockFile.py resulting in extraneous log entries.
Initial Comment: There is a race condition (that does not affect correct operation of Mailman) in .../Mailman/LockFile.py. This race results in entries to the "locks" log file (usually in pairs) that are unnecessary and confusing. If there is a process holding a lock for a file, and a process waiting for that lock to be freed, the following sequence can occur: 1) The waiting process executes os.link(). Since the running process still has the lock, this fails with EEXIST. 2) The running process releases the lock, and removes the lock file. 3) The waiting process proceeds with it's checks, and checks self.__linkcount() which returns -1 due to an ENOENT error. This throws an error into the "locks" log file. 4) The waiting process continues processing, checks it's timeout, and then checks to see if the lock lifetime has expired. The lifetime check is based on a call to self.__releasetime(), which returns -1, due to an ENOENT error. This results in the lockfile being declared stale, and self.__break() is called. This then throws a second error into the "locks" log file. The attached patch checks for these ENOENT errors, immediately restarting the loop when they are detected. It does not affect operation is the lock file exists and is held by another process. Brian Greenberg. ---------------------------------------------------------------------- Comment By: Brian Greenberg (grnbrg) Date: 2004-10-15 14:22 Message: Logged In: YES user_id=902583 This would fix this bug, as long as CLOCK_SLOP is large enough. But I don't think it's the Right Thing, as you are returning a result that says that the lockfile has a lifetime, when it in fact simply no longer exists. For a function whose purpose is to report the mtime of the master lockfile, -1 is the correct response if the file doesn't exist. The code that calls that function should check for and interpret that condition. ---------------------------------------------------------------------- Comment By: SHIGENO Kazutaka (shigeno) Date: 2004-10-15 00:11 Message: Logged In: YES user_id=1138453 How about changing as follows about self.__releasetime()? @@ -448,7 +448,7 @@ return os.stat(self.__lockfile)[ST_MTIME] except OSError, e: if e.errno <> errno.ENOENT: raise - return -1 + return time.time() def __linkcount(self): try: ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=300103&aid=1045656&group_id=103