[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