SVN: ZODB/trunk/src/ cross-database weakrefs weren't handled correctly.

Jim Fulton <[email protected]>
Newsgroups gmane.comp.python.zope.zodb.cvs
Message-ID <20100426205159.D91FE941BF__26356.5779816874$1272315143$gmane$org@cvs.zope.org>
Log message for revision 111454:
  cross-database weakrefs weren't handled correctly.
  
  https://bugs.launchpad.net/zodb/+bug/435547
  

Changed:
  U   ZODB/trunk/src/CHANGES.txt
  U   ZODB/trunk/src/ZODB/ConflictResolution.py
  U   ZODB/trunk/src/ZODB/ConflictResolution.txt
  U   ZODB/trunk/src/ZODB/serialize.py
  U   ZODB/trunk/src/persistent/wref.py

-=-
Modified: ZODB/trunk/src/CHANGES.txt
===================================================================
--- ZODB/trunk/src/CHANGES.txt	2010-04-26 20:34:58 UTC (rev 111453)
+++ ZODB/trunk/src/CHANGES.txt	2010-04-26 20:51:59 UTC (rev 111454)
@@ -34,15 +34,19 @@
 
   https://bugs.launchpad.net/zodb/+bug/544305
 
+- When using using a ClientStorage in a Storage server, there was a
+  threading bug that caused clients to get disconnected.
+
 - Fixed a bug that caused savepoint rollback to not properly
   set object state when objects implemented _p_invalidate methods
   that reloaded ther state (unghostifiable objects).
 
   https://bugs.launchpad.net/zodb/+bug/428039
 
-- When using using a ClientStorage in a Storage server, there was a
-  threading bug that caused clients to get disconnected.
+- cross-database wekrefs weren't handled correctly.
 
+  https://bugs.launchpad.net/zodb/+bug/435547
+
 3.10.0a1 (2010-02-08)
 =====================
 

Modified: ZODB/trunk/src/ZODB/ConflictResolution.py
===================================================================
--- ZODB/trunk/src/ZODB/ConflictResolution.py	2010-04-26 20:34:58 UTC (rev 111453)
+++ ZODB/trunk/src/ZODB/ConflictResolution.py	2010-04-26 20:51:59 UTC (rev 111454)
@@ -116,13 +116,17 @@
             # 'm' = multi_persistent: (database_name, oid, klass)
             # 'n' = multi_oid: (database_name, oid)
             # 'w' = persistent weakref: (oid)
+            #    or persistent weakref: (oid, database_name)
             # else it is a weakref: reference_type
             if reference_type == 'm':
                 self.database_name, self.oid, self.klass = data[1]
             elif reference_type == 'n':
                 self.database_name, self.oid = data[1]
             elif reference_type == 'w':
-                self.oid, = data[1]
+                try:
+                    self.oid, = data[1]
+                except ValueError:
+                    self.oid, self.database_name = data[1]
                 self.weak = True
             else:
                 assert len(data) == 1, 'unknown reference format'

Modified: ZODB/trunk/src/ZODB/ConflictResolution.txt
===================================================================
--- ZODB/trunk/src/ZODB/ConflictResolution.txt	2010-04-26 20:34:58 UTC (rev 111453)
+++ ZODB/trunk/src/ZODB/ConflictResolution.txt	2010-04-26 20:51:59 UTC (rev 111454)
@@ -474,6 +474,16 @@
     >>> ref3.weak
     True
 
+    >>> ref3a = PersistentReference(['w', ('my_oid', 'other_db')])
+    >>> ref3a.oid
+    'my_oid'
+    >>> print ref3a.klass
+    None
+    >>> ref3a.database_name
+    'other_db'
+    >>> ref3a.weak
+    True
+
     >>> ref4 = PersistentReference(['m', ('other_db', 'my_oid', 'my_class')])
     >>> ref4.oid
     'my_oid'
@@ -508,7 +518,7 @@
 
     >>> ref1 == ref1 and ref2 == ref2 and ref4 == ref4 and ref5 == ref5
     True
-    >>> ref3 == ref3 and ref6 == ref6 # weak references
+    >>> ref3 == ref3 and ref3a == ref3a and ref6 == ref6 # weak references
     True
 
     Non-weak references with the same oid and database_name are equal.

Modified: ZODB/trunk/src/ZODB/serialize.py
===================================================================
--- ZODB/trunk/src/ZODB/serialize.py	2010-04-26 20:34:58 UTC (rev 111453)
+++ ZODB/trunk/src/ZODB/serialize.py	2010-04-26 20:51:59 UTC (rev 111454)
@@ -100,7 +100,8 @@
     The following reference types are defined:
 
     'w'
-        Persistent weak reference.  The arguments consist of an oid.
+        Persistent weak reference.  The arguments consist of an oid
+        and optionally a database name.
 
     The following are planned for the future:
 
@@ -298,8 +299,8 @@
 
                 oid = obj.oid
                 if oid is None:
-                    obj = obj() # get the referenced object
-                    oid = obj._p_oid
+                    target = obj() # get the referenced object
+                    oid = target._p_oid
                     if oid is None:
                         # Here we are causing the object to be saved in
                         # the database. One could argue that we shouldn't
@@ -308,10 +309,16 @@
                         # assume that the object will be added eventually.
 
                         oid = self._jar.new_oid()
-                        obj._p_jar = self._jar
-                        obj._p_oid = oid
-                        self._stack.append(obj)
-                return ['w', (oid, )]
+                        target._p_jar = self._jar
+                        target._p_oid = oid
+                        self._stack.append(target)
+                    obj.oid = oid
+                    obj.dm = target._p_jar
+                    obj.database_name = obj.dm.db().database_name
+                if obj.dm is self._jar:
+                    return ['w', (oid, )]
+                else:
+                    return ['w', (oid, obj.database_name)]
 
 
         # Since we have an oid, we have either a persistent instance
@@ -523,10 +530,20 @@
     loaders['m'] = load_multi_persistent
 
 
-    def load_persistent_weakref(self, oid):
+    def load_persistent_weakref(self, oid, database_name=None):
         obj = WeakRef.__new__(WeakRef)
         obj.oid = oid
-        obj.dm = self._conn
+        if database_name is None:
+            obj.dm = self._conn
+        else:
+            obj.database_name = database_name
+            try:
+                obj.dm = self._conn.get_connection(database_name)
+            except KeyError:
+                # XXX Not sure what to do here.  It seems wrong to
+                # fail since this is a weak reference.  For now we'll
+                # just pretend that the target object has gone.
+                pass
         return obj
 
     loaders['w'] = load_persistent_weakref
@@ -632,7 +649,7 @@
     return oids
 
 oid_klass_loaders = {
-    'w': lambda oid: None,
+    'w': lambda oid, database_name=None: None,
     }
 
 def get_refs(a_pickle):

Modified: ZODB/trunk/src/persistent/wref.py
===================================================================
--- ZODB/trunk/src/persistent/wref.py	2010-04-26 20:34:58 UTC (rev 111453)
+++ ZODB/trunk/src/persistent/wref.py	2010-04-26 20:51:59 UTC (rev 111454)
@@ -99,7 +99,62 @@
     Always explicitly close databases: :)
 
     >>> db.close()
+    >>> del ob, ref, db, conn1, conn2, conn3
 
+    When multiple databases are in use, a weakref in one database may
+    point to an object in a different database.  Let's create two new
+    databases to demonstrate this.
+
+    >>> dbA = ZODB.tests.util.DB(
+    ...     database_name = 'dbA',
+    ...     )
+    >>> dbB = ZODB.tests.util.DB(
+    ...     database_name = 'dbB',
+    ...     databases = dbA.databases,
+    ...     )
+    >>> connA1 = dbA.open()
+    >>> connB1 = connA1.get_connection('dbB')
+
+    Now create and add a new object and a weak reference, and add them
+    to different databases.
+
+    >>> ob = ZODB.tests.MinPO.MinPO()
+    >>> ref = WeakRef(ob)
+    >>> connA1.root()['ob'] = ob
+    >>> connA1.add(ob)
+    >>> connB1.root()['ref'] = ref
+    >>> transaction.commit()
+
+    After a succesful commit, the reference should know the oid,
+    database name and connection of the object.
+
+    >>> ref.oid == ob._p_oid
+    True
+    >>> ref.database_name == 'dbA'
+    True
+    >>> ref.dm is ob._p_jar is connA1
+    True
+
+    If we open new connections, we should be able to use the reference.
+
+    >>> connA2 = dbA.open()
+    >>> connB2 = connA2.get_connection('dbB')
+    >>> ref2 = connB2.root()['ref']
+    >>> ob2 = connA2.root()['ob']
+    >>> ref2() is ob2
+    True
+    >>> ref2.oid == ob2._p_oid
+    True
+    >>> ref2.database_name == 'dbA'
+    True
+    >>> ref2.dm is ob2._p_jar is connA2
+    True
+
+    Always explicitly close databases: :)
+
+    >>> dbA.close()
+    >>> dbB.close()
+
     """
 
     # We set _p_oid to a marker so that the serialization system can
@@ -110,6 +165,8 @@
         self._v_ob = ob
         self.oid = ob._p_oid
         self.dm = ob._p_jar
+        if self.dm is not None:
+            self.database_name = self.dm.db().database_name
 
     def __call__(self):
         try:
@@ -117,7 +174,7 @@
         except AttributeError:
             try:
                 self._v_ob = self.dm[self.oid]
-            except KeyError:
+            except (KeyError, AttributeError):
                 return None
             return self._v_ob
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.