Fixed critical shutdown race in _Database._commit.

Related to SF patch 723231 (which pointed out the problem, but didn't
fix it, just shut up the warning msg -- which was pointing out a dead-
serious bug!).

Bugfix candidate.

diff --git a/Lib/dumbdbm.py b/Lib/dumbdbm.py
index 8e2ec907fb85605c215e8750f37f5f322bb4be7a..4e426f1c53249a5c1c7de91a5d5e31a6c4e39237 100644 (file)
--- a/Lib/dumbdbm.py
+++ b/Lib/dumbdbm.py
@@ -33,6 +33,17 @@ error = IOError                         # For anydbm
 
 class _Database(UserDict.DictMixin):
 
+    # The on-disk directory and data files can remain in mutually
+    # inconsistent states for an arbitrarily long time (see comments
+    # at the end of __setitem__).  This is only repaired when _commit()
+    # gets called.  One place _commit() gets called is from __del__(),
+    # and if that occurs at program shutdown time, module globals may
+    # already have gotten rebound to None.  Since it's crucial that
+    # _commit() finish sucessfully, we can't ignore shutdown races
+    # here, and _commit() must not reference any globals.
+    _os = _os       # for _commit()
+    _open = _open   # for _commit()
+
     def __init__(self, filebasename, mode):
         self._mode = mode
 
@@ -78,17 +89,20 @@ class _Database(UserDict.DictMixin):
     # file (if any) is renamed with a .bak extension first.  If a .bak
     # file currently exists, it's deleted.
     def _commit(self):
+        # CAUTION:  It's vital that _commit() succeed, and _commit() can
+        # be called from __del__().  Therefore we must never reference a
+        # global in this routine.
         try:
-            _os.unlink(self._bakfile)
-        except _os.error:
+            self._os.unlink(self._bakfile)
+        except self._os.error:
             pass
 
         try:
-            _os.rename(self._dirfile, self._bakfile)
-        except _os.error:
+            self._os.rename(self._dirfile, self._bakfile)
+        except self._os.error:
             pass
 
-        f = _open(self._dirfile, 'w', self._mode)
+        f = self._open(self._dirfile, 'w', self._mode)
         for key, pos_and_siz_pair in self._index.iteritems():
             f.write("%r, %r\n" % (key, pos_and_siz_pair))
         f.close()

diff --git a/Misc/NEWS b/Misc/NEWS
index b793a73e3b7e472bd1aa442a2996f1fe7fa4e214..1098ad31731239415a3808740fe69990d829a9bd 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -38,6 +38,14 @@ Extension modules
 Library
 -------
 
+- It's vital that a dumbdbm database be closed properly, else the
+  on-disk data and directory files can be left in mutually inconsistent
+  states.  dumbdbm.py's _Database.__del__() method attempted to close
+  the database properly, but a shutdown race in _Database._commit()
+  could prevent this form working, so that a program trusting __del__()
+  to get the on-disk files in synch could be badly surprised.  The race
+  has been repaired.
+
 - The classes in threading.py are now new-style classes.  That they
   weren't before was an oversight.