[Zodb-checkins] CVS: Zope3/src/zodb/storage - bdbfull.py:1.26

[Tim Peters]

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

Modified Files:
	bdbfull.py 
Log Message:
BDBFullStorage():  Plug a pack hole by not aiding oids to packmark
during commit, but by holding the commit lock while computing the pack
time.  See new comments for details.  It was possible to garbage-
collect a reachable object before this.

hestPackNowWhileWriting():  A new variant of testPackWhileWriting(),
which packs to "now" instead of to some time in the past.  The full
storage hole was very much easier to stumble into this way.


=== Zope3/src/zodb/storage/bdbfull.py 1.25 => 1.26 ===
--- Zope3/src/zodb/storage/bdbfull.py:1.25	Fri Jun  6 11:24:20 2003
+++ Zope3/src/zodb/storage/bdbfull.py	Wed Jul  2 16:00:27 2003
@@ -25,7 +25,7 @@
 from zodb.interfaces import *
 from zodb.storage.interfaces import *
 from zodb.utils import p64, u64
-from zodb.timestamp import TimeStamp
+from zodb.timestamp import TimeStamp, timeStampFromTime
 from zodb.conflict import ResolvedSerial
 from zodb.interfaces import ITransactionAttrs
 from zodb.storage.interfaces import StorageSystemError
@@ -449,14 +449,6 @@
         self._pvids.truncate(txn)
         self._prevrevids.truncate(txn)
         self._pending.truncate(txn)
-        # If we're in the middle of a pack, we need to add to the packmark
-        # table any objects that were modified in this transaction.
-        # Otherwise, there's a race condition where mark might have happened,
-        # then the object is added, then sweep runs, deleting the object
-        # created in the interrim.
-        if self._packing:
-            for oid in self._oids.keys():
-                self._packmark.put(oid, tid, txn=txn)
         self._oids.truncate(txn)
 
     def _dobegin(self, txn, tid):
@@ -1358,9 +1350,23 @@
         # BAW: should a pack time in the future be a ValueError?  We'd have to
         # worry about clock skew, so for now, we just set the pack time to the
         # minimum of t and now.
+        #
+        # If a transaction is currently in progress, wait for it to finish.
+        # If we don't hold the commit lock while computing pack time, then,
+        # for example:  a transaction is currently in progress, and we
+        # pack to "now".  After marking the root-reachable objects but
+        # before sweeping, a store comes in adding a new object, but not
+        # yet a store that makes the new object reachable from the root.
+        # Then the new object is in the collection of all objects, but
+        # doesn't appear to be reachable from the root, and has a time
+        # before the pack time, so it's collected as garbage.  Holding
+        # the commit lock while computing the pack time ensures that the
+        # next transaction begins after the pack time, implying that
+        # nothing it may add will be treated as garbage.
+        self._commit_lock_acquire()
         packtime = min(t, time.time())
-        t0 = TimeStamp(*(time.gmtime(packtime)[:5] + (packtime % 60,)))
-        packtid = t0.raw()
+        packtid = timeStampFromTime(packtime).raw()
+        self._commit_lock_release()
         # Collect all revisions of all objects earlier than the pack time.
         self._lock_acquire()
         try: