GnomePropertyBag

[Dave Camp <campd oit edu>]

There is a fairly serious bug in GnomePropertyBag that prevents 
more than one property bag from working properly in a single 
server. The core of the problem is that one POA is used for all 
propertybags, but the ObjectIds used by this POA are not unique 
between bags.  

gnome_property_bag_create_poa() is called for each PropertyBag 
that is created.  This function reuses a static POA object for 
each bag.  Also, for each bag it creates a new ServantManager.  
This ServantManager contains a pointer to the PropertyBag it is 
associated with.  It then selects the new ServantManager into the 
POA with PortableServer_POA_set_servant_manager().

This causes a problem.  When the new ServantManager is selected 
into the POA, the old one is discared, and all future requests 
handled by the POA will go through this servant manager.  So the 
POA only recognizes the last created servant.

There are two solutions to this problem.  One is to give each 
bag its own POA.  This would be easy to implement, but perhaps 
not the best solution.

The other solution is to have one POA which is used by all
propertybags.  There are two requirements for this approach:
  
* The ObjectIds created by gnome_property_bag_create_objref()
  must be unique between all PropertyBags.
* gnome_property_servant_locator_preinvoke must have a way to
  find which PropertyBag the ObjectId refers to (it currently
  uses a pointer stored in the ServantManager).

Below is a patch which implements the second solution.  I would
appreciate feedback.

Thanks,
-dave

Index: gnome-property-bag.c
===================================================================
RCS file: /cvs/gnome/bonobo/bonobo/gnome-property-bag.c,v
retrieving revision 1.10
diff -u -r1.10 gnome-property-bag.c
--- gnome-property-bag.c	1999/12/13 20:45:22	1.10
+++ gnome-property-bag.c	2000/01/11 21:45:27
@@ -53,7 +53,6 @@
 
 typedef struct {
 	POA_PortableServer_ServantLocator servant_locator;
-	GnomePropertyBag *property_bag;
 } GnomePropertyBagServantManager;
 
 /*
@@ -69,24 +68,31 @@
 					  PortableServer_ServantLocator_Cookie *cookie,
 					  CORBA_Environment *ev)
 {
-	GnomePropertyBagServantManager *sm;
 	PortableServer_Servant servant;
-	GnomePropertyBag *pb;
+	GnomePropertyBag *pb = NULL;
+	char *oid_str;
 	char *property_name;
 
 	/*
-	 * Get the PropertyBag out of the servant manager.
-	 */
-	sm = (GnomePropertyBagServantManager *) servant_manager;
-	pb = sm->property_bag;
-
-	/*
 	 * Grab the Property name and the Property Bag.
 	 */
-	property_name = PortableServer_ObjectId_to_string (oid, ev);
+	oid_str = PortableServer_ObjectId_to_string (oid, ev);
+	
 	if (ev->_major != CORBA_NO_EXCEPTION) {
 		g_warning ("GnomePropertyBag: Could not get property name from Object ID");
 		return NULL;
+	}	
+
+	/* 
+	 * Get the associated Propert Bag from the Object ID.
+	 */
+	sscanf (oid_str, "%p", &pb);
+	property_name = strchr (oid_str, ' ');	
+	if (property_name) {
+		property_name++;
+	} else {
+		g_warning ("GnomePropertyBag: Malformed Object ID");
+		return NULL;
 	}
 
 	/*
@@ -314,31 +320,31 @@
 		}
 
 		property_poa = pb->priv->poa;
-	}
-
-	/*
-	 * Create our ServantManager.
-	 */
-	sm = g_new0 (GnomePropertyBagServantManager, 1);
-	sm->property_bag = pb;
-
-	((POA_PortableServer_ServantLocator *) sm)->vepv = gnome_property_bag_get_servant_locator_vepv ();
 
-	POA_PortableServer_ServantLocator__init (((PortableServer_ServantLocator *) sm), &ev);
-	if (ev._major != CORBA_NO_EXCEPTION) {
-		g_warning ("GnomePropertyBag: Could not initialize ServantLocator");
-		CORBA_exception_free (&ev);
-		g_free (sm);
-		return FALSE;
+		/*
+		 * Create our ServantManager.
+		 */
+		sm = g_new0 (GnomePropertyBagServantManager, 1);
 		
-	}
-
-	PortableServer_POA_set_servant_manager (pb->priv->poa, (PortableServer_ServantManager) sm, &ev);
-	if (ev._major != CORBA_NO_EXCEPTION) {
-		g_warning ("GnomePropertyBag: Could not set POA servant manager");
-		CORBA_exception_free (&ev);
-		g_free (sm);
-		return FALSE;
+		((POA_PortableServer_ServantLocator *) sm)->vepv = 
+			gnome_property_bag_get_servant_locator_vepv ();
+		
+		POA_PortableServer_ServantLocator__init (((PortableServer_ServantLocator *) sm), &ev);
+		if (ev._major != CORBA_NO_EXCEPTION) {
+			g_warning ("GnomePropertyBag: Could not initialize ServantLocator");
+			CORBA_exception_free (&ev);
+			g_free (sm);
+			return FALSE;
+			
+		}
+		
+		PortableServer_POA_set_servant_manager (pb->priv->poa, (PortableServer_ServantManager) sm, &ev);
+		if (ev._major != CORBA_NO_EXCEPTION) {
+			g_warning ("GnomePropertyBag: Could not set POA servant manager");
+			CORBA_exception_free (&ev);
+			g_free (sm);
+			return FALSE;
+		}
 	}
 
 	return TRUE;
@@ -390,9 +396,12 @@
 				  GNOME_Property *obj, CORBA_Environment *ev)
 {
 	PortableServer_ObjectId *oid;
-
-	oid = PortableServer_string_to_ObjectId (name, ev);
-
+	char *oid_str;
+	
+	oid_str = g_strdup_printf ("%p %s", pb, name);
+	oid = PortableServer_string_to_ObjectId (oid_str, ev);
+	g_free (oid_str);
+	
 	*obj = (GNOME_Property) PortableServer_POA_create_reference_with_id (
 		pb->priv->poa, oid, "IDL:Bonobo/Property:1.0", ev);
 
@@ -692,16 +701,6 @@
 gnome_property_bag_destroy (GtkObject *object)
 {
 	GnomePropertyBag *pb = GNOME_PROPERTY_BAG (object);
-	CORBA_Environment ev;
-
-
-	/* Destroy the POA. */
-	CORBA_exception_init (&ev);
-	PortableServer_POA_destroy (pb->priv->poa, TRUE, TRUE, &ev);
-	if (ev._major != CORBA_NO_EXCEPTION) {
-		g_warning ("gnome_property_bag_destroy: Could not destroy POA.\n");
-	}
-	CORBA_exception_free (&ev);
 
 	/* Destroy all properties. */
 	g_hash_table_foreach_remove (pb->priv->props,