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

Harald Meland Harald.Meland@usit.uio.no
19 Jul 1999 17:36:49 +0200


[Barry A. Warsaw]

> The idea is that the marshal is first written to a temp file called
> config.db.tmp.<mypid>.  If this succeeds completely, then
> 
>     1) config.db.last is unlinked
>     2) a hard link config.db.last -> config.db is created
>     3) a hard link config.db -> config.db.tmp.<mypid> is created
>     4) config.db.tmp.<mypid> is unlinked
> 
> Comments?

You missed

 2.5) config.db is removed

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

Index: MailList.py
===================================================================
RCS file: /export/public/cvsroot/mailman/Mailman/MailList.py,v
retrieving revision 1.130
diff -u -r1.130 MailList.py
--- MailList.py	1999/07/18 17:26:55	1.130
+++ MailList.py	1999/07/19 15:09:08
@@ -759,6 +759,8 @@
         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,10 +773,14 @@
                             '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)
+            # Now do config.db.tmp.xxx -> config.db -> config.db.last
+            # rotation -- safely.
+            if os.path.exists(fname_last):
+                os.unlink(fname_last)
+            os.link(fname, fname_last)
+            os.rename(fname_tmp, fname)
+##            aside_new(fname, fname_last)
+##            aside_new(fname_tmp, fname)
         finally:
             os.umask(omask)
         self.CheckHTMLArchiveDir()

After this change, the function aside_new() in MailList.py can be
removed (or moved to Utils.py), as it isn't being used anywhere else.
-- 
Harald