Commit 18cb46c4 authored by Tim Peters's avatar Tim Peters

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.
parent dd715297
......@@ -85,6 +85,18 @@ depending on platform, and should suffer far fewer (if any) intermittent
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
......
......@@ -157,6 +157,17 @@ class BaseStorage(UndoLogCompatible):
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 @@ class BaseStorage(UndoLogCompatible):
# 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
......
......@@ -632,6 +632,8 @@ class FileStorage(BaseStorage.BaseStorage,
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 @@ class FileStorage(BaseStorage.BaseStorage,
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)
......
......@@ -205,6 +205,39 @@ class FileStorageTests(
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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment