[Mailman-Developers] patch candidate for MailList.Save()

[Barry A. Warsaw]

>>>>> "HM" == Harald Meland <Harald.Meland@usit.uio.no> writes:

    HM> You missed

    HM>  2.5) config.db is removed

    HM> which constitutes a race condition -- I think the below patch
    HM> fixes that (but as you say, this really is crucial code to get
    HM> right, so I'm posting it here for others to comment before
    HM> committing):

Arg!  You're right of course.  Here's a slightly reworked version of
your patch.  Also, not committed yet, but seems to work for me.

-Barry

-------------------- snip snip --------------------
Index: MailList.py
===================================================================
RCS file: /projects/cvsroot/mailman/Mailman/MailList.py,v
retrieving revision 1.130
diff -c -r1.130 MailList.py
*** MailList.py	1999/07/18 17:26:55	1.130
--- MailList.py	1999/07/19 22:22:56
***************
*** 26,31 ****
--- 26,32 ----
  			 'for the site?')
  
  import sys, os, marshal, string, posixfile, time
+ import errno
  import re
  import Utils
  import Errors
***************
*** 759,764 ****
--- 760,767 ----
          fname = os.path.join(self._full_path, 'config.db')
          fname_tmp = fname + '.tmp.' + `os.getpid()`
          fname_last = fname + ".last"
+         # Make config.db unreadable by `other', as it contains all the
+         # list members' passwords (in clear text).
          omask = os.umask(007)
          try:
              try:
***************
*** 771,780 ****
                              'Failed config.db file write, retaining old state'
                              '\n %s' % `status.args`)
                  Utils.reraise()
!             # now move config.db -> config.db.last
!             # then move config.db.tmp.xxx -> config.db
!             aside_new(fname, fname_last)
!             aside_new(fname_tmp, fname)
          finally:
              os.umask(omask)
          self.CheckHTMLArchiveDir()
--- 774,789 ----
                              'Failed config.db file write, retaining old state'
                              '\n %s' % `status.args`)
                  Utils.reraise()
!             # Now do config.db.tmp.xxx -> config.db -> config.db.last
!             # rotation -- safely.
!             try:
!                 # might not exist yet
!                 os.unlink(fname_last)
!             except os.error, (code, msg):
!                 if code <> errno.ENOENT:
!                     Utils.reraise()
!             os.link(fname, fname_last)
!             os.rename(fname_tmp, fname)
          finally:
              os.umask(omask)
          self.CheckHTMLArchiveDir()
***************
*** 1376,1397 ****
  	return ("<%s.%s %s%s at %s>"
  		% (self.__module__, self.__class__.__name__,
  		   `self._internal_name`, status, hex(id(self))[2:]))
- 
- def aside_new(old_name, new_name, reopen=0):
-     """Utility to move aside a file, optionally returning a fresh open version.
- 
-     We ensure maintanance of proper umask in the process."""
-     # Make config.db unreadable by `other', as it contains all the list
-     # members' passwords (in clear text).
-     ou = os.umask(007)
-     try:
-         if os.path.exists(new_name):
-             os.unlink(new_name)
-         if os.path.exists(old_name):
-             os.link(old_name, new_name)
-             os.unlink(old_name)
-         if reopen:
-             file = open(old_name, 'w')
-             return file
-     finally:
-         os.umask(ou)
--- 1385,1387 ----