From 460e058f4eb30b90e87cdda6dcbaba3f4115c43b Mon Sep 17 00:00:00 2001 From: Tim Peters <tim.one@comcast.net> Date: Fri, 18 Jun 2004 20:04:14 +0000 Subject: [PATCH] New test checkRestoreWithMultipleObjectsInUndoRedo from ZODB 3.2.2. 3.3 didn't have the bug, and this confirms it. --- trunk/src/ZODB/tests/RecoveryStorage.py | 103 ++++++++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/trunk/src/ZODB/tests/RecoveryStorage.py b/trunk/src/ZODB/tests/RecoveryStorage.py index 5fb33bdf..2a056ec9 100644 --- a/trunk/src/ZODB/tests/RecoveryStorage.py +++ b/trunk/src/ZODB/tests/RecoveryStorage.py @@ -183,3 +183,106 @@ class RecoveryStorage(IteratorDeepCompare): data, serial = self._dst.load(root._p_oid, '') raises(KeyError, self._dst.load, obj1._p_oid, '') raises(KeyError, self._dst.load, obj2._p_oid, '') + + def checkRestoreWithMultipleObjectsInUndoRedo(self): + from ZODB.FileStorage import FileStorage + + # Undo creates backpointers in (at least) FileStorage. ZODB 3.2.1 + # FileStorage._data_find() had an off-by-8 error, neglecting to + # account for the size of the backpointer when searching a + # transaction with multiple data records. The results were + # unpredictable. For example, it could raise a Python exception + # due to passing a negative offset to file.seek(), or could + # claim that a transaction didn't have data for an oid despite + # that it actually did. + # + # The former failure mode was seen in real life, in a ZRS secondary + # doing recovery. On my box today, the second failure mode is + # what happens in this test (with an unpatched _data_find, of + # course). Note that the error can only "bite" if more than one + # data record is in a transaction, and the oid we're looking for + # follows at least one data record with a backpointer. + # + # Unfortunately, _data_find() is a low-level implementation detail, + # and this test does some horrid white-box abuse to test it. + + is_filestorage = isinstance(self._storage, FileStorage) + + db = DB(self._storage) + c = db.open() + r = c.root() + + # Create some objects. + r["obj1"] = MinPO(1) + r["obj2"] = MinPO(1) + transaction.commit() + + # Add x attributes to them. + r["obj1"].x = 'x1' + r["obj2"].x = 'x2' + transaction.commit() + + r = db.open().root() + self.assertEquals(r["obj1"].x, 'x1') + self.assertEquals(r["obj2"].x, 'x2') + + # Dirty tricks. + if is_filestorage: + obj1_oid = r["obj1"]._p_oid + obj2_oid = r["obj2"]._p_oid + # This will be the offset of the next transaction, which + # will contain two backpointers. + pos = self._storage.getSize() + + # Undo the attribute creation. + info = self._storage.undoInfo() + tid = info[0]['id'] + t = Transaction() + self._storage.tpc_begin(t) + oids = self._storage.undo(tid, t) + self._storage.tpc_vote(t) + self._storage.tpc_finish(t) + + r = db.open().root() + self.assertRaises(AttributeError, getattr, r["obj1"], 'x') + self.assertRaises(AttributeError, getattr, r["obj2"], 'x') + + if is_filestorage: + # _data_find should find data records for both objects in that + # transaction. Without the patch, the second assert failed + # (it claimed it couldn't find a data record for obj2) on my + # box, but other failure modes were possible. + self.assert_(self._storage._data_find(pos, obj1_oid, '') > 0) + self.assert_(self._storage._data_find(pos, obj2_oid, '') > 0) + + # The offset of the next ("redo") transaction. + pos = self._storage.getSize() + + # Undo the undo (restore the attributes). + info = self._storage.undoInfo() + tid = info[0]['id'] + t = Transaction() + self._storage.tpc_begin(t) + oids = self._storage.undo(tid, t) + self._storage.tpc_vote(t) + self._storage.tpc_finish(t) + + r = db.open().root() + self.assertEquals(r["obj1"].x, 'x1') + self.assertEquals(r["obj2"].x, 'x2') + + if is_filestorage: + # Again _data_find should find both objects in this txn, and + # again the second assert failed on my box. + self.assert_(self._storage._data_find(pos, obj1_oid, '') > 0) + self.assert_(self._storage._data_find(pos, obj2_oid, '') > 0) + + # Indirectly provoke .restore(). .restore in turn indirectly + # provokes _data_find too, but not usefully for the purposes of + # the specific bug this test aims at: copyTransactionsFrom() uses + # storage iterators that chase backpointers themselves, and + # return the data they point at instead. The result is that + # _data_find didn't actually see anything dangerous in this + # part of the test. + self._dst.copyTransactionsFrom(self._storage) + self.compare(self._storage, self._dst) -- GitLab