Commit b43aab18 authored by Jim Fulton's avatar Jim Fulton

Bugs fixed:

  When objects were added in savepoints and either the savepoint was
  rolled back (https://bugs.launchpad.net/zodb/+bug/143560) or the
  transaction was aborted
  (https://mail.zope.org/pipermail/zodb-dev/2010-June/013488.html)
  The objects' _p_oid and _p_jar variables weren't cleared, leading to
  surprizing errors.
parent 4a8783a0
...@@ -2,6 +2,20 @@ ...@@ -2,6 +2,20 @@
Change History Change History
================ ================
3.10.0b7 (2010-09-??)
=====================
Bugs fixed
----------
- When objects were added in savepoints and either the savepoint was
rolled back (https://bugs.launchpad.net/zodb/+bug/143560) or the
transaction was aborted
(https://mail.zope.org/pipermail/zodb-dev/2010-June/013488.html)
The objects' _p_oid and _p_jar variables weren't cleared, leading to
surprizing errors.
3.10.0b6 (2010-09-08) 3.10.0b6 (2010-09-08)
===================== =====================
......
...@@ -442,6 +442,8 @@ class Connection(ExportImport, object): ...@@ -442,6 +442,8 @@ class Connection(ExportImport, object):
assert oid is not None assert oid is not None
if oid in self._added: if oid in self._added:
del self._added[oid] del self._added[oid]
if self._cache.get(oid) is not None:
del self._cache[oid]
del obj._p_jar del obj._p_jar
del obj._p_oid del obj._p_oid
self._db.save_oid(oid) self._db.save_oid(oid)
...@@ -742,7 +744,6 @@ class Connection(ExportImport, object): ...@@ -742,7 +744,6 @@ class Connection(ExportImport, object):
"""Disown any objects newly saved in an uncommitted transaction.""" """Disown any objects newly saved in an uncommitted transaction."""
if creating is None: if creating is None:
creating = self._creating creating = self._creating
self._creating = {}
for oid in creating: for oid in creating:
self._db.save_oid(oid) self._db.save_oid(oid)
...@@ -752,6 +753,8 @@ class Connection(ExportImport, object): ...@@ -752,6 +753,8 @@ class Connection(ExportImport, object):
del o._p_jar del o._p_jar
del o._p_oid del o._p_oid
creating.clear()
def tpc_vote(self, transaction): def tpc_vote(self, transaction):
"""Verify that a data manager can commit the transaction.""" """Verify that a data manager can commit the transaction."""
try: try:
...@@ -1104,7 +1107,10 @@ class Connection(ExportImport, object): ...@@ -1104,7 +1107,10 @@ class Connection(ExportImport, object):
self._creating.clear() self._creating.clear()
self._registered_objects = [] self._registered_objects = []
state = self._storage.position, self._storage.index.copy() state = (self._storage.position,
self._storage.index.copy(),
self._storage.creating.copy(),
)
result = Savepoint(self, state) result = Savepoint(self, state)
# While the interface doesn't guarantee this, savepoints are # While the interface doesn't guarantee this, savepoints are
# sometimes used just to "break up" very long transactions, and as # sometimes used just to "break up" very long transactions, and as
...@@ -1117,6 +1123,7 @@ class Connection(ExportImport, object): ...@@ -1117,6 +1123,7 @@ class Connection(ExportImport, object):
self._abort() self._abort()
self._registered_objects = [] self._registered_objects = []
src = self._storage src = self._storage
self._invalidate_creating(src.creating)
index = src.index index = src.index
src.reset(*state) src.reset(*state)
self._cache.invalidate(index) self._cache.invalidate(index)
...@@ -1163,6 +1170,7 @@ class Connection(ExportImport, object): ...@@ -1163,6 +1170,7 @@ class Connection(ExportImport, object):
def _abort_savepoint(self): def _abort_savepoint(self):
"""Discard all savepoint data.""" """Discard all savepoint data."""
src = self._savepoint_storage src = self._savepoint_storage
self._invalidate_creating(src.creating)
self._storage = self._normal_storage self._storage = self._normal_storage
self._savepoint_storage = None self._savepoint_storage = None
...@@ -1182,8 +1190,11 @@ class Connection(ExportImport, object): ...@@ -1182,8 +1190,11 @@ class Connection(ExportImport, object):
# by another thread, so the risk of a reread is pretty low. # by another thread, so the risk of a reread is pretty low.
# It's really not worth the effort to pursue this. # It's really not worth the effort to pursue this.
# Note that we do this *after* reseting the storage so that, if
# data are read, we read it from the reset storage!
self._cache.invalidate(src.index) self._cache.invalidate(src.index)
self._invalidate_creating(src.creating)
src.close() src.close()
# Savepoint support # Savepoint support
...@@ -1311,7 +1322,7 @@ class TmpStore: ...@@ -1311,7 +1322,7 @@ class TmpStore:
def temporaryDirectory(self): def temporaryDirectory(self):
return self._storage.temporaryDirectory() return self._storage.temporaryDirectory()
def reset(self, position, index): def reset(self, position, index, creating):
self._file.truncate(position) self._file.truncate(position)
self.position = position self.position = position
# Caution: We're typically called as part of a savepoint rollback. # Caution: We're typically called as part of a savepoint rollback.
...@@ -1324,6 +1335,7 @@ class TmpStore: ...@@ -1324,6 +1335,7 @@ class TmpStore:
# a copy of the index here. An alternative would be to ensure that # a copy of the index here. An alternative would be to ensure that
# all callers pass copies. As is, our callers do not make copies. # all callers pass copies. As is, our callers do not make copies.
self.index = index.copy() self.index = index.copy()
self.creating = creating
class RootConvenience(object): class RootConvenience(object):
......
...@@ -759,8 +759,72 @@ Read checks to work accross savepoints. ...@@ -759,8 +759,72 @@ Read checks to work accross savepoints.
""" """
# check interaction w savepoint class C_invalidations_of_new_objects_work_after_savepoint(Persistent):
# check call in read-only trans followed by write trans def __init__(self):
self.settings = 1
def _p_invalidate(self):
print 'INVALIDATE', self.settings
Persistent._p_invalidate(self)
print self.settings # POSKeyError here
def abort_of_savepoint_creating_new_objects_w_exotic_invalidate_doesnt_break():
r"""
Before, the following would fail with a POSKeyError, which was
somewhat surprizing, in a very edgy sort of way. :)
Really, when an object add is aborted, the object should be "removed" from
the db and its invalidatuon method shouldm't even be called:
>>> conn = ZODB.connection(None)
>>> conn.root.x = x = C_invalidations_of_new_objects_work_after_savepoint()
>>> _ = transaction.savepoint()
>>> x._p_oid
'\x00\x00\x00\x00\x00\x00\x00\x01'
>>> x._p_jar is conn
True
>>> transaction.abort()
After the abort, the oid and jar are None:
>>> x._p_oid
>>> x._p_jar
"""
class Clp9460655(Persistent):
def __init__(self, word, id):
super(Clp9460655, self).__init__()
self.id = id
self._word = word
def lp9460655():
r"""
>>> conn = ZODB.connection(None)
>>> root = conn.root()
>>> Word = Clp9460655
>>> from BTrees.OOBTree import OOBTree
>>> data = root['data'] = OOBTree()
>>> commonWords = []
>>> count = "0"
>>> for x in ('hello', 'world', 'how', 'are', 'you'):
... commonWords.append(Word(x, count))
... count = str(int(count) + 1)
>>> sv = transaction.savepoint()
>>> for word in commonWords:
... sv2 = transaction.savepoint()
... data[word.id] = word
>>> sv.rollback()
>>> print commonWords[1].id # raises POSKeyError
1
"""
......
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