Changes to event notification, to fix bugs associated with the use of oneways.
authorbillh <billh@e2bd861d-eb25-0410-b326-f6ed22b6b98c>
Wed, 19 Jun 2002 13:47:11 +0000 (13:47 +0000)
committerbillh <billh@e2bd861d-eb25-0410-b326-f6ed22b6b98c>
Wed, 19 Jun 2002 13:47:11 +0000 (13:47 +0000)
git-svn-id: http://svn.gnome.org/svn/at-spi/trunk@319 e2bd861d-eb25-0410-b326-f6ed22b6b98c

ChangeLog
atk-bridge/bridge.c
configure.in
cspi/bonobo/cspi-bonobo-listener.c
cspi/spi_main.c
idl/Accessibility_Event.idl
libspi/eventlistener.c
registryd/registry.c
test/event-listener-test.c
test/stress-test.c
test/test-simple.c

index 0141215..5fe618a 100644 (file)
--- 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:
index 7cb489d..2d1bb1e 100644 (file)
@@ -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
index 4a71fdc..41e6990 100644 (file)
@@ -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"
index 0c200f8..3a4545c 100644 (file)
@@ -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
index 346a7d1..6cf1924 100644 (file)
@@ -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
     {
index 06ac8ee..ba0465c 100644 (file)
@@ -37,7 +37,7 @@ module Accessibility
   };
 
   interface EventListener : Bonobo::Unknown {
-    oneway void notifyEvent (in Event e);
+    void notifyEvent (in Event e);
   };
 };
 
index 8dde552..a3f5d20 100644 (file)
@@ -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
index 17682c2..abff002 100644 (file)
@@ -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);
-    }
 }
 
 
index 7a776ac..2cf4286 100644 (file)
@@ -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);
index fdc0f8e..dc9f40f 100644 (file)
@@ -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);
        }
index 279e53c..47699bb 100644 (file)
@@ -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