Re: oaf plugin patch

[Mark McLoughlin <mark skynet ie>]

Hi,
	Replying to myself... bad sign ;)

	Okay, so myself and Elliot are at odds how to solve this
problem. I thought I'd summarise our IRC discussion and hopefully
get some input.

Problem:
--------
	Need to find some way that we can unload a plugin safely while
executing a CORBA method on an object in the plugin.

Elliot's Solution:
------------------
	Add a ORBit specific API to ORBit:

void ORBit_servant_set_deathwatch(PortableServer_ServantBase *servant,
                                  int *use_count,
                                  GFunc death_func,
                                  gpointer user_data)

	This will maintain reference count which, when it falls to
zero will call death_func when a CORBA method has completed. death_func
will unload the plugin.

	bad:
	* requires an ORBit specific API. non-CORBA compliance. Do we
	want to set the precedence, just to solve this issue, that
	ORBit specific hacks are okay?
	* complicates the internal structures used by the ORB.
	* is a complete hack.

	good:
	* the plugin has total freedom with regard to which POAs
	  it uses etc.

My Solution:
------------
	Use a CORBA specified feature. That is, that is
PortableServer_POA_destroy is called on a POA, and that POA is
currently being used in a method invocation, then the BAD_INV_ORDER
exception is returned.

	A plugin's activate function currently takes a POA as its
first argument. To implement this solution, that POA will take on a
new importance - the plugin will have to agree to use this POA as it's
'root' POA i.e. all objects created by the plugin will have to exist
within that POA, or that POA's children.

	bad:
	* the plugin is restricted to using this POA. Could be a
	problem with some bonobo calls? Not sure...
	* could be on the limits of CORBA spec interpretation...

	good:
	* no ORBit specific hack.
	* API will not have to be maintained.
	* less of a hack.

	Anyway, comments are more than welcome - in fact, demanded ;)

Cheers,
Mark.

diff -urpN oaf.old/liboaf/oaf-activate.c oaf/liboaf/oaf-activate.c
--- oaf.old/liboaf/oaf-activate.c	Thu Jul 26 09:11:19 2001
+++ oaf/liboaf/oaf-activate.c	Thu Jul 26 09:13:27 2001
@@ -26,13 +26,9 @@
 #include "liboaf/liboaf-private.h"
 #include "liboaf/oaf-activate.h"
 #include "liboaf/oaf-activate-private.h"
-
-extern CORBA_Object oaf_server_activate_shlib (OAF_ActivationResult * sh,
-					       CORBA_Environment * ev);
-
+#include "liboaf/oaf-plugin.h"
 
 static gboolean test_components_enabled = FALSE;
-
 
 /**
  * oaf_set_test_components_enabled:
diff -urpN oaf.old/liboaf/oaf-plugin.c oaf/liboaf/oaf-plugin.c
--- oaf.old/liboaf/oaf-plugin.c	Thu Jul 26 09:11:19 2001
+++ oaf/liboaf/oaf-plugin.c	Thu Jul 26 09:13:27 2001
@@ -26,16 +26,15 @@
 #include "liboaf-private.h"
 
 #include <gmodule.h>
-/* ORBit-specific hack */
-#include <orbit/poa/poa.h>
 
 #include "oaf-plugin.h"
 
 typedef struct
 {
-	GModule *loaded;
-	int refcount;
-	char filename[1];
+	GModule           *loaded;
+	int                refcount;
+	char               filename[1];
+	PortableServer_POA poa;
 }
 ActivePluginInfo;
 
@@ -68,8 +67,6 @@ oaf_server_activate_shlib (OAF_Activatio
 	ActivePluginInfo *local_plugin_info = NULL;
 	const OAFPluginObject *pobj;
 	int i;
-	PortableServer_POA poa;
-	CORBA_ORB orb;
 	char *filename;
 	const char *iid;
 
@@ -88,6 +85,8 @@ oaf_server_activate_shlib (OAF_Activatio
 		/* We have to load the thing from scratch */
 		GModule *gmod;
 		gboolean success;
+		PortableServer_POA rootpoa;
+		CORBA_ORB orb;
 
 		gmod = g_module_open (filename, G_MODULE_BIND_LAZY);
 		if (!gmod) {
@@ -127,6 +126,14 @@ oaf_server_activate_shlib (OAF_Activatio
 		local_plugin_info->loaded = gmod;
 		strcpy (local_plugin_info->filename, filename);
 
+		orb = oaf_orb_get ();
+		rootpoa = (PortableServer_POA)
+				CORBA_ORB_resolve_initial_references (orb, "RootPOA", ev);
+
+		local_plugin_info->poa = PortableServer_POA_create_POA (rootpoa, filename, 
+								        NULL, NULL, ev );
+		CORBA_Object_release ((CORBA_Object)rootpoa, ev);
+
 		if (!living_by_filename)
 			living_by_filename =
 				g_hash_table_new (g_str_hash, g_str_equal);
@@ -157,10 +164,6 @@ oaf_server_activate_shlib (OAF_Activatio
 
 	retval = CORBA_OBJECT_NIL;
 
-	orb = oaf_orb_get ();
-	poa = (PortableServer_POA)
-		CORBA_ORB_resolve_initial_references (orb, "RootPOA", ev);
-
 	/* Index into the string list one element from the end to get the iid of the shlib */
 	iid = sh->res._u.res_shlib._buffer[sh->res._u.res_shlib._length - 2];
 	for (pobj = plugin->plugin_object_list; pobj->iid; pobj++) {
@@ -172,7 +175,8 @@ oaf_server_activate_shlib (OAF_Activatio
 
 	if (pobj->iid) {
 		/* Activate the shlib */
-		retval = pobj->activate (poa, pobj->iid, local_plugin_info, ev);
+		retval = pobj->activate (local_plugin_info->poa, pobj->iid, 
+					 local_plugin_info, ev);
 
 		if (ev->_major != CORBA_NO_EXCEPTION)
 			retval = CORBA_OBJECT_NIL;
@@ -205,7 +209,6 @@ oaf_server_activate_shlib (OAF_Activatio
 
 /**
  * oaf_plugin_use:
- * @servant: The servant that was created
  * @impl_ptr: The impl_ptr that was passed to the original activation routine
  *
  * You should call this routine to activate a shared library-based 
@@ -213,31 +216,41 @@ oaf_server_activate_shlib (OAF_Activatio
  * correctly an %OAFPlugin structure named "OAF_Plugin_info".
  */
 void
-oaf_plugin_use (PortableServer_Servant servant, gpointer impl_ptr)
+oaf_plugin_use (gpointer impl_ptr)
 {
 	ActivePluginInfo *local_plugin_info = impl_ptr;
 
 	local_plugin_info->refcount++;
-
-#if 0
-	ORBit_servant_set_deathwatch (servant, &(local_plugin_info->refcount),
-				      gnome_plugin_unload, local_plugin_info);
-#endif
 }
 
 static gboolean
 oaf_plugin_real_unuse (gpointer impl_ptr)
 {
-	ActivePluginInfo *api;
+	ActivePluginInfo   *api;
 
 	g_return_val_if_fail (impl_ptr, FALSE);
 
 	api = impl_ptr;
 
-	api->refcount--;
+	if (--api->refcount <= 0) {
+		PortableServer_POA  poa = api->poa;
+		CORBA_Environment   env;
+
+		CORBA_exception_init (&env);
+		PortableServer_POA_destroy (poa, CORBA_TRUE, CORBA_TRUE, &env);
+
+		if (env._major == CORBA_SYSTEM_EXCEPTION && 
+		    strcmp (CORBA_exception_id(&env), ex_CORBA_BAD_INV_ORDER) == 0) {
+
+			CORBA_exception_free (&env);
+			return TRUE;
+		}
+
+		CORBA_Object_release ((CORBA_Object)poa, &env);
 
-	if (api->refcount <= 0) {
 		gnome_plugin_unload (&(api->refcount), api);
+
+		CORBA_exception_free (&env);
         }        
 
         return FALSE;
diff -urpN oaf.old/liboaf/oaf-plugin.h oaf/liboaf/oaf-plugin.h
--- oaf.old/liboaf/oaf-plugin.h	Thu Jul 26 09:11:19 2001
+++ oaf/liboaf/oaf-plugin.h	Thu Jul 26 09:13:27 2001
@@ -26,24 +26,27 @@
 #ifndef OAF_PLUGIN_H
 #define OAF_PLUGIN_H
 
-
 typedef struct {
 	const char *iid;
 
-	/* This routine should call oaf_plugin_use(servant, impl_ptr), 
+	/* This routine should call oaf_plugin_use(impl_ptr), 
          * as should all routines which activate CORBA objects
 	 * implemented by this shared library. This needs to be done 
          * before making any CORBA calls on the object, or
 	 * passing that object around. First thing after servant creation 
          * always works. :) 
+	 *
+	 * @poa should be used by the plugin as its 'RootPOA'. This should
+	 * be done to ensure the plugin can be unloaded safely.
+	 *
+	 * @impl_ptr should be stored by the implementation to be passed to 
+	 * oaf_plugin_unuse() in the implementation's destruction routine.
          */
 
-        CORBA_Object (*activate) (PortableServer_POA poa,
-                                  const char *iid, 
-                                  gpointer impl_ptr,	/* This pointer should be stored by the implementation
-                                                         * to be passed to oaf_plugin_unuse() in the 
-                                                         * implementation's destruction routine. */
-				  CORBA_Environment * ev);
+        CORBA_Object (*activate) (PortableServer_POA  poa,
+                                  const char         *iid, 
+                                  gpointer            impl_ptr,
+				  CORBA_Environment  *ev);
 } OAFPluginObject;
 
 typedef struct {
@@ -51,16 +54,10 @@ typedef struct {
 	const char *description;
 } OAFPlugin;
 
-void  oaf_plugin_use    (PortableServer_Servant servant, 
-			 gpointer impl_ptr);
+void  oaf_plugin_use    (gpointer impl_ptr);
 void  oaf_plugin_unuse  (gpointer impl_ptr);
 
-
 CORBA_Object oaf_server_activate_shlib (OAF_ActivationResult * sh, 
                                         CORBA_Environment * ev);
 
-
 #endif /* OAF_PLUGIN_H */
-
-
-
diff -urpN oaf.old/test/Makefile.am oaf/test/Makefile.am
--- oaf.old/test/Makefile.am	Thu Jul 26 09:11:23 2001
+++ oaf/test/Makefile.am	Thu Jul 26 09:13:27 2001
@@ -6,6 +6,8 @@ noinst_PROGRAMS=		\
 	oaf-test-async		\
 	$(NULL)
 
+noinst_LTLIBRARIES=liboaf-test-plugin.la
+
 bin_PROGRAMS=			\
 	oaf-run-query		\
 	$(NULL)
@@ -19,6 +21,9 @@ oaf_test_client_SOURCES=	\
 	empty-common.c 		\
 	empty.h 		\
 	empty-stubs.c		\
+	plugin-common.c		\
+	plugin.h 		\
+	plugin-stubs.c		\
 	$(NULL)
 
 oaf_run_query_SOURCES=		\
@@ -35,33 +40,51 @@ oaf_empty_server_SOURCES=	\
 	empty-skels.c		\
 	$(NULL)
 
+# Horrible, digusting and atrocious hack to 
+# get libtool to build a noinst shared lib.
+liboaf_test_plugin_la_LDFLAGS = -rpath $(shell (cd $(top_builddir);pwd))/test/.libs
+liboaf_test_plugin_la_SOURCES = oaf-test-plugin.c	  \
+				plugin.h		\
+				plugin-common.c		\
+				plugin-skels.c		\
+				$(NULL)
+
 BUILT_SOURCES=			\
 	empty.h 		\
 	empty-common.c 		\
 	empty-stubs.c 		\
 	empty-skels.c		\
+	plugin.h		\
+	plugin-common.c		\
+	plugin-stubs.c		\
+	plugin-skels.c		\
 	$(NULL)
 
 empty.h empty-common.c empty-stubs.c empty-skels.c: empty_built
+plugin.h plugin-common.c plugin-stubs.c plugin-skels.c: plugin_built
 
 empty_built: empty.idl $(ORBIT_IDL)
 	$(ORBIT_IDL) $(srcdir)/empty.idl
 	touch empty_built
 
-CLEANFILES=empty_built
+plugin_built: plugin.idl $(ORBIT_IDL)
+	$(ORBIT_IDL) $(srcdir)/plugin.idl
+	touch plugin_built
+
+CLEANFILES=empty_built plugin_built
 
 INCLUDES=-I$(top_srcdir) -I$(top_builddir) -I$(top_builddir)/liboaf -DOAFINFODIR=\"$(pkgdatadir)\" \
 	$(UTILS_CFLAGS)
 LDADD=../liboaf/liboaf-2.la $(UTILS_LIBS)
 
-oaffiles=empty.oafinfo
+oaffiles=empty.oafinfo plugin.oafinfo
 
 emptydatadir=$(datadir)/oaf
 emptydata_DATA=$(oaffiles)
 
 bin_SCRIPTS=oaf-slay
 
-EXTRA_DIST=empty.idl $(oaffiles) broken.oafinfo oaf-slay
+EXTRA_DIST=empty.idl plugin.idl $(oaffiles) broken.oafinfo oaf-slay
 
 check:
-	OAF_INFO_PATH="$(top_srcdir)/test:$$OAF_INFO_PATH" PATH="$(top_builddir)/test:$$PATH" $(top_builddir)/test/oaf-test-client
+	OAF_INFO_PATH="$(top_srcdir)/test:$$OAF_INFO_PATH" PATH="$(top_builddir)/test:$$PATH" LD_LIBRARY_PATH="$(top_builddir)/test/.libs:$$LD_LIBRARY_PATH" $(top_builddir)/test/oaf-test-client
diff -urpN oaf.old/test/oaf-test-client.c oaf/test/oaf-test-client.c
--- oaf.old/test/oaf-test-client.c	Thu Jul 26 09:11:24 2001
+++ oaf/test/oaf-test-client.c	Thu Jul 26 09:13:27 2001
@@ -4,6 +4,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include "empty.h"
+#include "plugin.h"
 
 #define TOTAL_TEST_SCORE 13
 
@@ -73,6 +74,21 @@ test_object (CORBA_Object obj, CORBA_Env
 }
 
 static int
+test_plugin (CORBA_Object obj, CORBA_Environment *ev, const char *type)
+{
+        Plugin_doPluginTest (obj, ev);
+
+        if (ev->_major != CORBA_NO_EXCEPTION) {
+                g_warning ("Call failed: %s\n",
+                           oaf_exception_id (ev));
+                return 0;
+        } else {
+                fprintf (stderr, "Test %s succeeded\n", type);
+                return 1;
+        }
+}
+
+static int
 test_empty (CORBA_Object obj, CORBA_Environment *ev, const char *type)
 {
         Empty_doNothing (obj, ev);
@@ -117,6 +133,12 @@ main (int argc, char *argv[])
                 passed += test_empty (obj, &ev, "from aid");
         }
 
+        fprintf (stderr, "Plugin test");
+	obj = oaf_activate ("repo_ids.has('IDL:Plugin:1.0')", NULL, 0, NULL,
+                            &ev);
+        if (test_object (obj, &ev, "by query")) {
+                passed += test_plugin (obj, &ev, "by query");
+        }
 
         fprintf (stderr, "Broken link test ");
         obj = oaf_activate_from_id ("OAFIID:Bogus:20000526", 0, NULL, &ev);
diff -urpN oaf.old/test/oaf-test-plugin.c oaf/test/oaf-test-plugin.c
--- oaf.old/test/oaf-test-plugin.c	Thu Jan  1 01:00:00 1970
+++ oaf/test/oaf-test-plugin.c	Thu Jul 26 09:14:25 2001
@@ -0,0 +1,58 @@
+#include <liboaf/liboaf.h>
+
+#include "plugin.h"
+
+static gpointer oaf_plugin_impl_ptr = NULL;
+
+static void plugin_test_impl (PortableServer_Servant servant, 
+			      CORBA_Environment *ev)
+{
+  oaf_plugin_unuse (oaf_plugin_impl_ptr);
+}
+
+static PortableServer_ServantBase__epv base_epv = {
+   ._private    = NULL,
+   .finalize    = NULL,
+   .default_POA = NULL
+   };
+
+static POA_Plugin__epv plugin_epv = {
+   ._private     = NULL,
+   .doPluginTest = plugin_test_impl
+   };
+
+static POA_Plugin__vepv plugin_vepv = {
+   ._base_epv = &base_epv,
+   .Plugin_epv  = &plugin_epv
+   };
+
+static POA_Plugin plugin_servant = {
+   ._private = NULL,
+   .vepv     = &plugin_vepv
+   };
+
+static CORBA_Object 
+activate_plugin (PortableServer_POA poa,
+		 const char        *iid,
+		 gpointer           impl_ptr,
+		 CORBA_Environment *ev)
+{
+	PortableServer_ObjectId *objid;
+	CORBA_Object             objref;
+
+	POA_Plugin__init (&plugin_servant, ev);
+
+	oaf_plugin_use (impl_ptr);
+
+	oaf_plugin_impl_ptr = impl_ptr;
+
+	objid = PortableServer_POA_activate_object (poa, &plugin_servant, ev);
+	objref = PortableServer_POA_id_to_reference (poa, objid, ev);
+
+	CORBA_free (objid);
+
+	return objref;
+}
+
+static OAFPluginObject plugin_list[] = { {"OAFIID:Plugin:20010713", activate_plugin}, {NULL} };
+const OAFPlugin OAF_Plugin_info = { plugin_list, "OAF test plugin" };
diff -urpN oaf.old/test/plugin.idl oaf/test/plugin.idl
--- oaf.old/test/plugin.idl	Thu Jan  1 01:00:00 1970
+++ oaf/test/plugin.idl	Thu Jul 26 09:13:51 2001
@@ -0,0 +1,5 @@
+/* -*- Mode: C; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 8 -*- */
+
+interface Plugin {
+	void doPluginTest ();
+};
diff -urpN oaf.old/test/plugin.oafinfo oaf/test/plugin.oafinfo
--- oaf.old/test/plugin.oafinfo	Thu Jan  1 01:00:00 1970
+++ oaf/test/plugin.oafinfo	Thu Jul 26 09:14:03 2001
@@ -0,0 +1,9 @@
+<oaf_info>
+
+<oaf_server iid="OAFIID:Plugin:20010713" type="shlib" location="./.libs/liboaf-test-plugin.so">
+<oaf_attribute name="repo_ids" type="stringv">
+        <item value="IDL:Plugin:1.0"/>
+</oaf_attribute>
+</oaf_server>
+
+</oaf_info>