Commit 2751ff22 authored by Jim Fulton's avatar Jim Fulton

Because of the, um, automatic way that objects are added to databases

by virtue of being reachable from objects in databases, there is
a danger of ambiguity when an object is reachable from multiple
databases.  This ambiguity can lead to very subtle bugs with the
symtom that objects are in unexpected databases.

Introduced a check to refuse to commit when the ambiguity exists.

(This required changing the connection '_creating' attribute to a
dictionary.)
parent a8a91c72
...@@ -107,6 +107,7 @@ class Connection(ExportImport, object): ...@@ -107,6 +107,7 @@ class Connection(ExportImport, object):
self._reset_counter = global_reset_counter self._reset_counter = global_reset_counter
self._load_count = 0 # Number of objects unghosted self._load_count = 0 # Number of objects unghosted
self._store_count = 0 # Number of objects stored self._store_count = 0 # Number of objects stored
self._creating = {}
# List of oids of modified objects (to be invalidated on an abort). # List of oids of modified objects (to be invalidated on an abort).
self._modified = [] self._modified = []
...@@ -395,6 +396,7 @@ class Connection(ExportImport, object): ...@@ -395,6 +396,7 @@ class Connection(ExportImport, object):
self._flush_invalidations() self._flush_invalidations()
self._needs_to_join = True self._needs_to_join = True
self._registered_objects = [] self._registered_objects = []
self._creating.clear()
# Process pending invalidations. # Process pending invalidations.
def _flush_invalidations(self): def _flush_invalidations(self):
...@@ -445,7 +447,7 @@ class Connection(ExportImport, object): ...@@ -445,7 +447,7 @@ class Connection(ExportImport, object):
# _creating is a list of oids of new objects, which is used to # _creating is a list of oids of new objects, which is used to
# remove them from the cache if a transaction aborts. # remove them from the cache if a transaction aborts.
self._creating = [] self._creating.clear()
self._normal_storage.tpc_begin(transaction) self._normal_storage.tpc_begin(transaction)
def commit(self, transaction): def commit(self, transaction):
...@@ -468,8 +470,9 @@ class Connection(ExportImport, object): ...@@ -468,8 +470,9 @@ class Connection(ExportImport, object):
"""Commit changes to an object""" """Commit changes to an object"""
if self._import: if self._import:
# TODO: This code seems important for Zope, but needs docs # We are importing an export file. We alsways do this
# to explain why. # while making a savepoint so we can copy export data
# directly to out storage, typically a TmpStore.
self._importDuringCommit(transaction, *self._import) self._importDuringCommit(transaction, *self._import)
self._import = None self._import = None
...@@ -516,7 +519,7 @@ class Connection(ExportImport, object): ...@@ -516,7 +519,7 @@ class Connection(ExportImport, object):
if serial == z64: if serial == z64:
# obj is a new object # obj is a new object
self._creating.append(oid) self._creating[oid] = 1
# Because obj was added, it is now in _creating, so it can # Because obj was added, it is now in _creating, so it can
# be removed from _added. # be removed from _added.
self._added.pop(oid, None) self._added.pop(oid, None)
...@@ -622,7 +625,7 @@ class Connection(ExportImport, object): ...@@ -622,7 +625,7 @@ 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 = [] self._creating = {}
for oid in creating: for oid in creating:
o = self._cache.get(oid) o = self._cache.get(oid)
...@@ -1033,10 +1036,10 @@ class Connection(ExportImport, object): ...@@ -1033,10 +1036,10 @@ class Connection(ExportImport, object):
self._normal_storage) self._normal_storage)
self._storage = self._savepoint_storage self._storage = self._savepoint_storage
self._creating = [] self._creating.clear()
self._commit(None) self._commit(None)
self._storage.creating.extend(self._creating) self._storage.creating.update(self._creating)
del self._creating[:] 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()
...@@ -1061,7 +1064,7 @@ class Connection(ExportImport, object): ...@@ -1061,7 +1064,7 @@ class Connection(ExportImport, object):
# Copy invalidating and creating info from temporary storage: # Copy invalidating and creating info from temporary storage:
self._modified.extend(oids) self._modified.extend(oids)
self._creating.extend(src.creating) self._creating.update(src.creating)
for oid in oids: for oid in oids:
data, serial = src.load(oid, src) data, serial = src.load(oid, src)
...@@ -1129,7 +1132,7 @@ class TmpStore: ...@@ -1129,7 +1132,7 @@ class TmpStore:
self.position = 0L self.position = 0L
# index: map oid to pos of last committed version # index: map oid to pos of last committed version
self.index = {} self.index = {}
self.creating = [] self.creating = {}
def __len__(self): def __len__(self):
return len(self.index) return len(self.index)
......
...@@ -63,6 +63,47 @@ It isn't valid to create references outside a multi database: ...@@ -63,6 +63,47 @@ It isn't valid to create references outside a multi database:
InvalidObjectReference: InvalidObjectReference:
Attempt to store an object from a foreign database connection Attempt to store an object from a foreign database connection
>>> tm.abort()
Databases for new objects
-------------------------
Objects are normally added to a database by making them reachable from
an object already in the database. This is unambiguous when there is
only one database. With modultiple databases, it is not so clear what
happens. Consider:
>>> p4 = MyClass()
>>> p1.p4 = p4
>>> p2.p4 = p4
In this example, the new object is reachable from both p1 in database
1 and p2 in database 2. If we commit, which database will p4 end up
in? This sort of ambiguity can lead to subtle bugs. For that reason,
an error is generated if we commit changes when new objects are
reachable from multiple databases:
>>> tm.commit() # doctest: +NORMALIZE_WHITESPACE
Traceback (most recent call last):
...
InvalidObjectReference: A new object is reachable from multiple
databases. Won't try to guess which one was correct!
>>> tm.abort()
To resolve this ambiguity, we need to commit, or create a savepoint
before an object becomes reachable from multiple databases. Here
we'll use a savepoint to make sure that p4 lands in database 1:
>>> p4 = MyClass()
>>> p1.p4 = p4
>>> s = tm.savepoint()
>>> p2.p4 = p4
The advantage of using a savepoint is that we aren't making a
commitment. Changes made in the savepoint will be rolled back if the
transaction is aborted.
NOTE NOTE
---- ----
......
...@@ -343,6 +343,16 @@ class ObjectWriter: ...@@ -343,6 +343,16 @@ class ObjectWriter:
"a separate onnection to the same database or " "a separate onnection to the same database or "
"multidatabase" "multidatabase"
) )
# OK, we have an object from another database.
# Lets make sure the object ws not *just* loaded.
# TODO: shouldn't depend on underware (_creating)
if oid in obj._p_jar._creating:
raise InvalidObjectReference(
"A new object is reachable from multiple databases. "
"Won't try to guess which one was correct!"
)
klass = type(obj) klass = type(obj)
...@@ -358,6 +368,7 @@ class ObjectWriter: ...@@ -358,6 +368,7 @@ class ObjectWriter:
if database_name: if database_name:
return ['n', (database_name, oid)] return ['n', (database_name, oid)]
return oid return oid
# Note that we never get here for persistent classes. # Note that we never get here for persistent classes.
...@@ -365,6 +376,7 @@ class ObjectWriter: ...@@ -365,6 +376,7 @@ class ObjectWriter:
if database_name: if database_name:
return ['m', (database_name, oid, klass)] return ['m', (database_name, oid, klass)]
return oid, klass return oid, klass
def serialize(self, obj): def serialize(self, obj):
......
...@@ -78,14 +78,19 @@ different connections to the same database. ...@@ -78,14 +78,19 @@ different connections to the same database.
""" """
def tearDownDbs(test):
test.globs['db1'].close()
test.globs['db2'].close()
def test_suite(): def test_suite():
return unittest.TestSuite(( return unittest.TestSuite((
doctest.DocFileSuite('../cross-database-references.txt', doctest.DocFileSuite('../cross-database-references.txt',
globs=dict(MyClass=MyClass), globs=dict(MyClass=MyClass),
tearDown=tearDownDbs,
), ),
doctest.DocFileSuite('../cross-database-references.txt', doctest.DocFileSuite('../cross-database-references.txt',
globs=dict(MyClass=MyClass_w_getnewargs), globs=dict(MyClass=MyClass_w_getnewargs),
tearDown=tearDownDbs,
), ),
doctest.DocTestSuite(), doctest.DocTestSuite(),
)) ))
......
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