[Zodb-checkins] SVN: ZODB/trunk/ Merge rev 29350 from ZODB 3.3 branch.

[Tim Peters]

Log message for revision 29351:
  Merge rev 29350 from ZODB 3.3 branch.
  
  Port from ZODB 3.2.
  
  Change FileStorage .restore() and .store() to update max oid in use.
  
  This is the last of the checkins to fix critical bugs involving rare cases
  where a FileStorage could end up reusing old oids for new objects.
  

Changed:
  U   ZODB/trunk/NEWS.txt
  U   ZODB/trunk/src/ZODB/BaseStorage.py
  U   ZODB/trunk/src/ZODB/FileStorage/FileStorage.py
  U   ZODB/trunk/src/ZODB/tests/testFileStorage.py

-=-
Modified: ZODB/trunk/NEWS.txt
===================================================================
--- ZODB/trunk/NEWS.txt	2005-02-28 21:42:51 UTC (rev 29350)
+++ ZODB/trunk/NEWS.txt	2005-02-28 21:45:31 UTC (rev 29351)
@@ -85,6 +85,18 @@
 FileStorage
 -----------
 
+- The ``.store()`` and ``.restore()`` methods didn't update the storage's
+  belief about the largest oid in use when passed an oid larger than the
+  largest oid the storage already knew about.  Because ``.restore()`` in
+  particular is used  by ``copyTransactionsFrom()``, and by the first stage
+  of ZRS recovery, a large database could be created that believed the only
+  oid in use was oid 0 (the special oid reserved for the root object).  In
+  rare cases, it could go on from there assigning duplicate oids to new
+  objects, starting over from oid 1 again.  This has been repaired.  A
+  new ``set_max_oid()`` method was added to the ``BaseStorage`` class so
+  that derived storages can update the largest oid in use in a threadsafe
+  way.
+
 - A FileStorage's index file tried to maintain the index's largest oid as a
   separate piece of data, incrementally updated over the storage's lifetime.
   This scheme was more complicated than necessary, so was also more brittle

Modified: ZODB/trunk/src/ZODB/BaseStorage.py
===================================================================
--- ZODB/trunk/src/ZODB/BaseStorage.py	2005-02-28 21:42:51 UTC (rev 29350)
+++ ZODB/trunk/src/ZODB/BaseStorage.py	2005-02-28 21:45:31 UTC (rev 29351)
@@ -157,6 +157,17 @@
             if d < 255: return last[:-1]+chr(d+1)+'\0'*(8-len(last))
             else:       return self.new_oid(last[:-1])
 
+    # Update the maximum oid in use, under protection of a lock.  The
+    # maximum-in-use attribute is changed only if possible_new_max_oid is
+    # larger than its current value.
+    def set_max_oid(self, possible_new_max_oid):
+        self._lock_acquire()
+        try:
+            if possible_new_max_oid > self._oid:
+                self._oid = possible_new_max_oid
+        finally:
+            self._lock_release()
+
     def registerDB(self, db, limit):
         pass # we don't care
 
@@ -371,10 +382,7 @@
         # using store().  However, if we use store, then
         # copyTransactionsFrom() may fail with VersionLockError or
         # ConflictError.
-        if hasattr(self, 'restore'):
-            restoring = 1
-        else:
-            restoring = 0
+        restoring = hasattr(self, 'restore')
         fiter = other.iterator()
         for transaction in fiter:
             tid=transaction.tid

Modified: ZODB/trunk/src/ZODB/FileStorage/FileStorage.py
===================================================================
--- ZODB/trunk/src/ZODB/FileStorage/FileStorage.py	2005-02-28 21:42:51 UTC (rev 29350)
+++ ZODB/trunk/src/ZODB/FileStorage/FileStorage.py	2005-02-28 21:45:31 UTC (rev 29351)
@@ -632,6 +632,8 @@
 
         self._lock_acquire()
         try:
+            if oid > self._oid:
+                self.set_max_oid(oid)
             old = self._index_get(oid, 0)
             cached_tid = None
             pnv = None
@@ -758,6 +760,8 @@
 
         self._lock_acquire()
         try:
+            if oid > self._oid:
+                self.set_max_oid(oid)
             prev_pos = 0
             if prev_txn is not None:
                 prev_txn_pos = self._txn_find(prev_txn, 0)

Modified: ZODB/trunk/src/ZODB/tests/testFileStorage.py
===================================================================
--- ZODB/trunk/src/ZODB/tests/testFileStorage.py	2005-02-28 21:42:51 UTC (rev 29350)
+++ ZODB/trunk/src/ZODB/tests/testFileStorage.py	2005-02-28 21:45:31 UTC (rev 29351)
@@ -205,6 +205,39 @@
 
         self.failUnless(self._storage._records_before_save > 20)
 
+    def checkStoreBumpsOid(self):
+        # If .store() is handed an oid bigger than the storage knows
+        # about already, it's crucial that the storage bump its notion
+        # of the largest oid in use.
+        t = transaction.Transaction()
+        self._storage.tpc_begin(t)
+        giant_oid = '\xee' * 8
+        # Store an object.
+        # oid, serial, data, version, transaction
+        r1 = self._storage.store(giant_oid, '\0'*8, 'data', '', t)
+        # Finish the transaction.
+        r2 = self._storage.tpc_vote(t)
+        self._storage.tpc_finish(t)
+        # Before ZODB 3.2.6, this failed, with ._oid == z64.
+        self.assertEqual(self._storage._oid, giant_oid)
+
+    def checkRestoreBumpsOid(self):
+        # As above, if .restore() is handed an oid bigger than the storage
+        # knows about already, it's crucial that the storage bump its notion
+        # of the largest oid in use.  Because copyTransactionsFrom(), and
+        # ZRS recovery, use the .restore() method, this is plain critical.
+        t = transaction.Transaction()
+        self._storage.tpc_begin(t)
+        giant_oid = '\xee' * 8
+        # Store an object.
+        # oid, serial, data, version, prev_txn, transaction
+        r1 = self._storage.restore(giant_oid, '\0'*8, 'data', '', None, t)
+        # Finish the transaction.
+        r2 = self._storage.tpc_vote(t)
+        self._storage.tpc_finish(t)
+        # Before ZODB 3.2.6, this failed, with ._oid == z64.
+        self.assertEqual(self._storage._oid, giant_oid)
+
     def checkCorruptionInPack(self):
         # This sets up a corrupt .fs file, with a redundant transaction
         # length mismatch.  The implementation of pack in many releases of