From: billh <billh@e2bd861d-eb25-0410-b326-f6ed22b6b98c>
Date: Wed, 19 Jun 2002 13:47:11 +0000 (+0000)
Subject: Changes to event notification, to fix bugs associated with the use of oneways.
X-Git-Tag: AT_SPI2_CORE_0_1_3~887
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=14c750668c8e5c881ac452fecbae505c04b95f25;p=platform%2Fupstream%2Fat-spi2-core.git

Changes to event notification, to fix bugs associated with the use of oneways.


git-svn-id: http://svn.gnome.org/svn/at-spi/trunk@319 e2bd861d-eb25-0410-b326-f6ed22b6b98c
---

diff --git a/ChangeLog b/ChangeLog
index 0141215..5fe618a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,91 @@
+2002-06-18  Bill Haneman  <bill.haneman@sun.com>
+
+	* configure.in:
+	Revved version to 1.1.0 
+	(reserving the 1.0.X branch for gnome-2-0-0 branch, this
+	version is going to HEAD which will be used for gnome-2-0-1 and later.)
+	
+	* idl/Accessibility_Registry.idl:
+	(EventListener::notifyEvent):
+	Removed 'oneway' directive after extensive consulation with
+	ORBit2 team and others.  This means also that unref() of the event
+	source can and should be done synchronously after emission, rather
+	than remotely in the client, after servicing the notify call on
+	the listener side.
+
+	NOTE: This change speeds up listener performance considerably, but
+	introduces new latency on the application side.  We may want to
+	add an event queue to the atk-bridge.
+
+	* atk-bridge/bridge.c:
+	(spi_atk_bridge_focus_tracker):
+	Do a local unref() on the event source after emission.
+	
+	* registryd/registry.c:
+	(desktop_remove_application):
+	Do an unref() on the event source after emission.
+	(desktop_add_application):
+	Do an unref() on the event source after emission.
+	(notify_listeners_cb):
+	When relaying an event, don't automatically add the event source
+	to the local object cache, just CORBA_dup it instead.  Likewise,
+	if this method reenters, release the ref rather than calling
+	unref() as well.
+	(impl_registry_notify_event):
+	No longer call remote unref() on the event source after dispatch.
+
+	* libspi/eventlistener.c:
+	(impl_accessible_event_notify_event):
+	Removed remote unref ()
+	
+	* cspi/bonobo/cspi-bonobo-listener.c:
+	(cspi_event):
+	We now call cspi_object_new() instead of cspi_object_add() on
+	receipt of an event; thus we only have an implicit object ref
+	while the cspi_event method is being executed.  If we need to keep
+	a reference to the object, the listener must call ref() on the
+	object.  Thus also we don't need to call cspi_object_unref() after
+	invoking the listener callbacks in this method.
+	
+	* cspi/spi_main.c:
+	(cspi_object_new):
+	New internal API for creating a new cspi object, without adding it
+	to the object cache.
+	(cspi_object_add):
+	Now uses cspi_object_new() to create the cspi object instance.
+	(cspi_object_ref):
+	Now checks to see if the object is in the internal object cache,
+	and adds it if necessary before incrementing its refcount (note
+	that new objects are added with a refcount of 1).
+	(report_leaked_ref):
+	New method which gives some information on leaked object instances
+	if leak detection is turned on.
+
+	* test/event-listener-test.c:
+	(timing_test_event):
+	New method, used for testing event delivery timing for special
+	events of type "object:test".  It reports elapsed time and
+	events/sec every 500 events.
+	(main):
+	Added a new listener, 'test listener', to check timing of event
+	receipt as noted above.
+	(report_event):
+	Added some timing output here also, reports elapsed time every 100
+	events.
+
+	* test/stress-test.c:
+	Emit events of type "object:test", for use with "event-listener-test".
+
+	* test/test-simple.c:
+	(global_listener_cb):
+	Call Accessible_ref() on the event source before calling
+	validate_accessible, since the validation process does pointer
+	comparisons on the event source, meaning that the event source
+	needs to be added to the local object cache first.  Any use of
+	such pointer comparisons between Accessible objects requires that
+	the caller hold an explicit reference to those objects.
+	We also must therefore call Accessible_unref() when leaving this method.
+
 2002-06-13  Bill Haneman  <bill.haneman@sun.com>
 
 	* registryd/deviceeventcontroller.c:
diff --git a/atk-bridge/bridge.c b/atk-bridge/bridge.c
index 7cb489d..2d1bb1e 100644
--- a/atk-bridge/bridge.c
+++ b/atk-bridge/bridge.c
@@ -329,7 +329,8 @@ spi_atk_bridge_focus_tracker (AtkObject *object)
   e.detail2 = 0;
 
   Accessibility_Registry_notifyEvent (registry, &e, &ev);
-
+  Accessibility_Accessible_unref (e.source, &ev);
+  
   CORBA_exception_free (&ev);
 }
 
@@ -382,6 +383,7 @@ spi_atk_emit_eventv (GObject      *gobject,
 #endif
 
       Accessibility_Registry_notifyEvent (registry, &e, &ev);
+      Accessibility_Accessible_unref (e.source, &ev);
 
       CORBA_exception_free (&ev);
 
@@ -389,6 +391,7 @@ spi_atk_emit_eventv (GObject      *gobject,
     }
 
   va_end (args);
+
 }
 
 static gboolean
diff --git a/configure.in b/configure.in
index 4a71fdc..41e6990 100644
--- a/configure.in
+++ b/configure.in
@@ -1,8 +1,8 @@
 AC_INIT(idl/Accessibility.idl)
 
 AT_SPI_MAJOR_VERSION=1
-AT_SPI_MINOR_VERSION=0
-AT_SPI_MICRO_VERSION=1
+AT_SPI_MINOR_VERSION=1
+AT_SPI_MICRO_VERSION=0
 AT_SPI_INTERFACE_AGE=0
 AT_SPI_BINARY_AGE=0
 AT_SPI_VERSION="$AT_SPI_MAJOR_VERSION.$AT_SPI_MINOR_VERSION.$AT_SPI_MICRO_VERSION"
diff --git a/cspi/bonobo/cspi-bonobo-listener.c b/cspi/bonobo/cspi-bonobo-listener.c
index 0c200f8..3a4545c 100644
--- a/cspi/bonobo/cspi-bonobo-listener.c
+++ b/cspi/bonobo/cspi-bonobo-listener.c
@@ -91,10 +91,8 @@ cspi_event (SpiEventListener    *listener,
   GList *l;
   CSpiEventListener *clistener = (CSpiEventListener *) listener;
   AccessibleEvent    aevent;
-  Accessible        *source;
-
-  source = cspi_object_add_ref (event->source, TRUE);
-
+  Accessible        *source = cspi_object_new (event->source);
+  
   aevent.type    = event->type;
   aevent.source  = source;
   aevent.detail1 = event->detail1;
@@ -107,8 +105,6 @@ cspi_event (SpiEventListener    *listener,
 
       eh->cb.event (&aevent, eh->user_data);
     }
-
-  cspi_object_unref (source);
 }
 
 static void
diff --git a/cspi/spi_main.c b/cspi/spi_main.c
index 346a7d1..6cf1924 100644
--- a/cspi/spi_main.c
+++ b/cspi/spi_main.c
@@ -157,6 +157,30 @@ cspi_exception (void)
 }
 
 Accessible *
+cspi_object_new (CORBA_Object corba_object)
+{
+  Accessible *ref;
+
+  if (corba_object == CORBA_OBJECT_NIL)
+    {
+      ref = NULL;
+    }
+  else if (!cspi_check_ev ("pre method check: add"))
+    {
+      ref = NULL;
+    }
+  else
+    {
+      ref = malloc (sizeof (Accessible));
+//    ref->objref = CORBA_Object_duplicate (corba_object, cspi_ev ());
+      ref->objref = corba_object;
+      ref->ref_count = 1;
+    }
+
+  return ref;
+}
+
+Accessible *
 cspi_object_add (CORBA_Object corba_object)
 {
 	return cspi_object_add_ref (corba_object, FALSE);
@@ -189,21 +213,18 @@ cspi_object_add_ref (CORBA_Object corba_object, gboolean do_ref)
 	}
       else
         {
-          ref = malloc (sizeof (Accessible));
+	  ref = cspi_object_new (corba_object);
 
 #ifdef DEBUG_OBJECTS
-          g_print ("allocating %p => %p\n", ref, corba_object);
+          g_print ("allocated %p => %p\n", ref, corba_object);
 #endif
 	  if (do_ref) {
 #ifdef JAVA_BRIDGE_BUG_IS_FIXED
 		  g_assert (CORBA_Object_is_a (corba_object,
 						"IDL:Bonobo/Unknown:1.0", cspi_ev ()));
 #endif
-		  ref->objref = bonobo_object_dup_ref (corba_object, cspi_ev ());
-	  } else 
-		  ref->objref = corba_object;
-          ref->ref_count = 1;
-
+		  Bonobo_Unknown_ref (corba_object, cspi_ev ());
+	  }
           g_hash_table_insert (cspi_get_live_refs (), ref->objref, ref);
 	}
     }
@@ -216,7 +237,12 @@ cspi_object_ref (Accessible *accessible)
 {
   g_return_if_fail (accessible != NULL);
 
-  accessible->ref_count++;
+  if (g_hash_table_lookup (cspi_get_live_refs (), accessible->objref) == NULL) {
+	  accessible->objref = bonobo_object_dup_ref (accessible->objref, cspi_ev ());
+	  g_hash_table_insert (cspi_get_live_refs (), accessible->objref, accessible);	  
+  } else {
+	  accessible->ref_count++;
+  }
 }
 
 void
@@ -342,6 +368,21 @@ SPI_nextEvent (SPIBoolean waitForEvent)
   return NULL;
 }
 
+static void
+report_leaked_ref (gpointer key, gpointer val, gpointer user_data)
+{
+	Accessible *a = (Accessible *) val;
+	char *name, *role;
+	name = Accessible_getName (a);
+	if (cspi_exception ()) name = NULL;
+	role = Accessible_getRoleName (a);
+	if (cspi_exception ()) role = NULL;
+	fprintf (stderr, "leaked object %s, role %s\n", (name) ? name : "<?>",
+		 (role) ? role : "<?>");
+	if (name) SPI_freeString (name);
+}
+
+
 /**
  * SPI_exit:
  *
@@ -365,6 +406,10 @@ SPI_exit (void)
   if (live_refs)
     {
       leaked = g_hash_table_size (live_refs);
+#define PRINT_LEAKS
+#ifdef PRINT_LEAKS
+      g_hash_table_foreach (live_refs, report_leaked_ref, NULL);
+#endif
     }
   else
     {
diff --git a/idl/Accessibility_Event.idl b/idl/Accessibility_Event.idl
index 06ac8ee..ba0465c 100644
--- a/idl/Accessibility_Event.idl
+++ b/idl/Accessibility_Event.idl
@@ -37,7 +37,7 @@ module Accessibility
   };
 
   interface EventListener : Bonobo::Unknown {
-    oneway void notifyEvent (in Event e);
+    void notifyEvent (in Event e);
   };
 };
 
diff --git a/libspi/eventlistener.c b/libspi/eventlistener.c
index 8dde552..a3f5d20 100644
--- a/libspi/eventlistener.c
+++ b/libspi/eventlistener.c
@@ -50,10 +50,6 @@ impl_accessible_event_notify_event (PortableServer_Servant     servant,
 
   g_signal_emit (G_OBJECT (listener), signals [EVENT], 0, e);
 
-  if (e->source != CORBA_OBJECT_NIL)
-    {
-      Accessibility_Accessible_unref (e->source, ev);
-    }
 }
 
 static void
diff --git a/registryd/registry.c b/registryd/registry.c
index 17682c2..abff002 100644
--- a/registryd/registry.c
+++ b/registryd/registry.c
@@ -98,6 +98,7 @@ desktop_add_application (SpiDesktop *desktop,
   CORBA_exception_init (&ev);
   Accessibility_Registry_notifyEvent (BONOBO_OBJREF (registry),
 				      &e, &ev);
+  Accessibility_Desktop_unref (e.source, &ev);
   CORBA_exception_free (&ev);
 }
 
@@ -118,6 +119,7 @@ desktop_remove_application (SpiDesktop *desktop,
   CORBA_exception_init (&ev);
   Accessibility_Registry_notifyEvent (BONOBO_OBJREF (registry),
 				      &e, &ev);
+  Accessibility_Desktop_unref (e.source, &ev);
   CORBA_exception_free (&ev);
 }
 
@@ -568,26 +570,27 @@ notify_listeners_cb (GList * const *list, gpointer user_data)
       CORBA_free (s);
 #endif
       
-      ctx->e_out.source = bonobo_object_dup_ref (ctx->source, ctx->ev);
+      ctx->e_out.source = CORBA_Object_duplicate (ctx->source, ctx->ev);
+
       if (BONOBO_EX (ctx->ev))
         {
           return SPI_RE_ENTRANT_CONTINUE;;
 	}
-
+      
       if ((*list) && (*list)->data == ls)
         {
           Accessibility_EventListener_notifyEvent (
             (Accessibility_EventListener) ls->listener, &ctx->e_out, ctx->ev);
-        if (ctx->ev->_major != CORBA_NO_EXCEPTION)
-          {
-            g_warning ("Accessibility app error: exception during "
-		       "event notification: %s\n",
-		       CORBA_exception_id (ctx->ev));
-	  }
+          if (ctx->ev->_major != CORBA_NO_EXCEPTION)
+            {
+              g_warning ("Accessibility app error: exception during "
+		        "event notification: %s\n",
+		        CORBA_exception_id (ctx->ev));
+	    }
 	}
       else /* dup re-entered */
         {
-          bonobo_object_release_unref (ctx->e_out.source, ctx->ev);
+          CORBA_Object_release (ctx->e_out.source, ctx->ev);
 	}
     }  
 
@@ -619,10 +622,6 @@ impl_registry_notify_event (PortableServer_Servant     servant,
       spi_re_entrant_list_foreach (list, notify_listeners_cb, &ctx);
     }
 
-  if (e->source != CORBA_OBJECT_NIL)
-    {
-      Accessibility_Accessible_unref (e->source, ev);
-    }
 }
 
 
diff --git a/test/event-listener-test.c b/test/event-listener-test.c
index 7a776ac..2cf4286 100644
--- a/test/event-listener-test.c
+++ b/test/event-listener-test.c
@@ -27,10 +27,13 @@ static void traverse_accessible_tree (Accessible *accessible);
 
 static void report_event  (const AccessibleEvent *event, void *user_data);
 static void report_detail_event  (const AccessibleEvent *event, void *user_data);
+static void timing_test_event (const AccessibleEvent *event, void *user_data);
 
 static AccessibleEventListener *generic_listener;
 static AccessibleEventListener *specific_listener;
+static AccessibleEventListener *test_listener;
 static gint n_elements_traversed = 0;
+static GTimer *timer;
 
 int
 main (int argc, char **argv)
@@ -39,7 +42,6 @@ main (int argc, char **argv)
   int n_desktops;
   int n_apps;
   char *s;
-  GTimer *timer;
   gdouble elapsed_time;
   Accessible *desktop;
   Accessible *application;
@@ -51,6 +53,8 @@ main (int argc, char **argv)
 	  report_event, NULL); 
   specific_listener = SPI_createAccessibleEventListener (
 	  report_detail_event, NULL); 
+  test_listener = SPI_createAccessibleEventListener (
+	  timing_test_event, NULL);
 
   SPI_registerGlobalEventListener (generic_listener,
 				   "focus:");
@@ -111,6 +115,8 @@ main (int argc, char **argv)
 	                           "window:shade");
   SPI_registerGlobalEventListener (generic_listener,
 	                           "window:unshade");
+  SPI_registerGlobalEventListener (test_listener,
+				   "object:test");
 #ifdef NOT_YET_IMPLEMENTED
   /* event below possibly should just be property change? */
   SPI_registerGlobalEventListener (generic_listener,
@@ -133,7 +139,7 @@ main (int argc, char **argv)
   g_print ("[%f elements/sec, %f SPI calls/sec]\n", 
 	n_elements_traversed/g_timer_elapsed(timer, NULL),
 	(n_elements_traversed*8+1)/g_timer_elapsed(timer, NULL));
-  
+  g_timer_reset (timer);
   SPI_event_main ();
 
   putenv ("AT_BRIDGE_SHUTDOWN=1");
@@ -179,9 +185,20 @@ traverse_accessible_tree (Accessible *accessible)
 void
 report_event (const AccessibleEvent *event, void *user_data)
 {
+  static long count = 0;
   char *s = Accessible_getName (event->source);
   fprintf (stderr, "%s %s\n", event->type, s);
   if (s) SPI_freeString (s);
+  if (count == 0) {
+	  g_timer_reset (timer);
+	  g_timer_start (timer);
+  }
+  ++count;
+  if ((count % 100) == 0) {
+	  g_print ("%d events received, %f events/sec\n",
+		   count,
+		   count/g_timer_elapsed(timer, NULL));
+  }
 }
 
 void
@@ -191,6 +208,19 @@ report_detail_event (const AccessibleEvent *event, void *user_data)
 }
 
 void
+timing_test_event (const AccessibleEvent *event, void *user_data)
+{
+	static long count = 0;
+	if (count == 0) g_timer_start (timer);
+	++count;
+	if ((count % 500) == 0) {
+		g_print ("%d events received, %f events/sec\n",
+			 count,
+			 count/g_timer_elapsed(timer, NULL));
+	}
+}
+
+void
 test_exit ()
 {
   SPI_deregisterGlobalEventListenerAll (generic_listener);
diff --git a/test/stress-test.c b/test/stress-test.c
index fdc0f8e..dc9f40f 100644
--- a/test/stress-test.c
+++ b/test/stress-test.c
@@ -73,7 +73,7 @@ main (int argc, char **argv)
 
 	source = spi_accessible_new (atko);
 	
-	e.type = "focus:";
+	e.type = "object:test";
 	e.source = BONOBO_OBJREF (source);
 	e.detail1 = 0;
 	e.detail2 = 0;
@@ -81,7 +81,7 @@ main (int argc, char **argv)
 	timer = g_timer_new ();
 	g_timer_start (timer);
 
-	for (i = 0; i < 10000; ++i) {
+	for (i = 0; i < 500; ++i) {
 		Accessibility_Accessible_ref (e.source, &ev);
 		Accessibility_Registry_notifyEvent (registry, &e, &ev);
 	}
diff --git a/test/test-simple.c b/test/test-simple.c
index 279e53c..47699bb 100644
--- a/test/test-simple.c
+++ b/test/test-simple.c
@@ -650,6 +650,13 @@ global_listener_cb (const AccessibleEvent *event,
 
 	fprintf (stderr, "Fielded focus event ...\n");
 
+	/*
+	 * must ref before doing "direct pointer" identity comparisons,
+	 * e.g. "accessible == a".
+	 * Alternatively, one can use cspi_object_equal (a, b)
+	 */
+	Accessible_ref (event->source); 
+		
 	if (!do_poke) {
 		desktop = SPI_getDesktop (0);
 		application = Accessible_getChildAtIndex (desktop, 0);
@@ -661,13 +668,17 @@ global_listener_cb (const AccessibleEvent *event,
 		AccessibleApplication_unref (application);
 		
 		print_tree = FALSE;
+
 		validate_accessible (event->source, TRUE, TRUE);
 
+		fprintf (stderr, "quitting mainloop.\n");
 		gtk_main_quit ();
 	}
 
 	print_tree = TRUE;
 	validate_accessible (event->source, TRUE, TRUE);
+	
+	Accessible_unref (event->source);
 }
 
 static SPIBoolean