[Zodb-checkins] SVN: ZODB/branches/3.3/ Port from ZODB 3.2.

[Tim Peters]

Log message for revision 29350:
  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/branches/3.3/NEWS.txt
  U   ZODB/branches/3.3/src/ZODB/BaseStorage.py
  U   ZODB/branches/3.3/src/ZODB/FileStorage/FileStorage.py
  U   ZODB/branches/3.3/src/ZODB/tests/testFileStorage.py

-=-
Modified: ZODB/branches/3.3/NEWS.txt
===================================================================
--- ZODB/branches/3.3/NEWS.txt	2005-02-28 19:58:58 UTC (rev 29349)
+++ ZODB/branches/3.3/NEWS.txt	2005-02-28 21:42:51 UTC (rev 29350)
@@ -32,6 +32,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/branches/3.3/src/ZODB/BaseStorage.py
===================================================================
--- ZODB/branches/3.3/src/ZODB/BaseStorage.py	2005-02-28 19:58:58 UTC (rev 29349)
+++ ZODB/branches/3.3/src/ZODB/BaseStorage.py	2005-02-28 21:42:51 UTC (rev 29350)
@@ -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/branches/3.3/src/ZODB/FileStorage/FileStorage.py
===================================================================
--- ZODB/branches/3.3/src/ZODB/FileStorage/FileStorage.py	2005-02-28 19:58:58 UTC (rev 29349)
+++ ZODB/branches/3.3/src/ZODB/FileStorage/FileStorage.py	2005-02-28 21:42:51 UTC (rev 29350)
@@ -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/branches/3.3/src/ZODB/tests/testFileStorage.py
===================================================================
--- ZODB/branches/3.3/src/ZODB/tests/testFileStorage.py	2005-02-28 19:58:58 UTC (rev 29349)
+++ ZODB/branches/3.3/src/ZODB/tests/testFileStorage.py	2005-02-28 21:42:51 UTC (rev 29350)
@@ -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