[Zope3-checkins] CVS: Zope3/src/zodb/storage - bdbminimal.py:1.21

[Tim Peters]

Update of /cvs-repository/Zope3/src/zodb/storage
In directory cvs.zope.org:/tmp/cvs-serv19712/src/zodb/storage

Modified Files:
	bdbminimal.py 
Log Message:
Fix rare failure in testPackWhileWriting by holding the commit lock for
the duration of packing.  This was reported by a user on a Debian system,
and was relatively easy to provoke on Win2K.  I was never able to
provoke it on Win98, and Barry wasn't able to provoke it on Linux.

What happens:  A new object Q is stored.  Packing then begins.  The
mark and sweep phases complete, and don't find that Q is reachable from
the root.  Then a new revision of an old object P is stored which points
to Q.  But storing doesn't chase references, so the collect phase of
packing still thinks Q is trash, and erroneously deletes it.

XXX The mark code here still looks flawed:  on object may be associated
XXX with up to two serials, but _mark only chases one of them.  Leaving
XXX that for Barry to fix.


=== Zope3/src/zodb/storage/bdbminimal.py 1.20 => 1.21 ===
--- Zope3/src/zodb/storage/bdbminimal.py:1.20	Thu Jun 26 14:01:01 2003
+++ Zope3/src/zodb/storage/bdbminimal.py	Mon Jun 30 12:35:39 2003
@@ -380,10 +380,19 @@
         self.log('pack started')
         # A simple wrapper around the bulk of packing, but which acquires a
         # lock that prevents multiple packs from running at the same time.
+        # It's redundant in this storage because we hold the commit lock
+        # for the duration, but it doesn't hurt.
         self._packlock.acquire()
         # Before setting the packing flag to true, acquire the storage lock
         # and clear out the packmark table, in case there's any cruft left
         # over from the previous pack.
+        # Caution:  this used to release the commit lock immediately after
+        # clear_packmark (below) was called, so there was a small chance
+        # for transactions to commit between the packing phases.  This
+        # suffered rare races, where packing could (erroneously) delete
+        # a newly-added object.  Since interleaving packing with commits
+        # is thought to be unimportant for minimal storages, the easiest
+        # (by far) fix is to hold the commit lock throughout packing.
         self._commit_lock_acquire()
         try:
             # We have to do this within a Berkeley transaction
@@ -391,9 +400,6 @@
                 self._packmark.truncate(txn=txn)
             self._withtxn(clear_packmark)
             self._packing = True
-        finally:
-            self._commit_lock_release()
-        try:
             # We don't wrap this in _withtxn() because we're going to do the
             # operation across several Berkeley transactions, which allows
             # other work to happen (stores and reads) while packing is being
@@ -405,6 +411,7 @@
         finally:
             self._packing = False
             self._packlock.release()
+            self._commit_lock_release()
         self.log('pack finished')
 
     def _dopack(self):