Fix some ref count issues

Add refs in some places where they were previously not present, and be more
thorough about removing refs when disposing. This should fix some leaks,
though it is likely that many still remain.
Also add a test program that can be used to test for some leaks.

diff --git a/Makefile.am b/Makefile.am
index 37d8008..c67aab6 100644 (file)
--- a/Makefile.am
+++ b/Makefile.am
@@ -5,7 +5,7 @@ INTROSPECTION_GIRS =
 INTROSPECTION_SCANNER_ARGS = --add-include-path=$(srcdir)
 INTROSPECTION_COMPILER_ARGS = --includedir=$(srcdir)
 
-SUBDIRS=po dbind xml atspi bus registryd doc
+SUBDIRS=po dbind xml atspi bus registryd doc test
 
 ACLOCAL_AMFLAGS=-I m4 ${ACLOCAL_FLAGS}
 

diff --git a/atspi/atspi-accessible.c b/atspi/atspi-accessible.c
index 7971bd3..6732d1c 100644 (file)
--- a/atspi/atspi-accessible.c
+++ b/atspi/atspi-accessible.c
@@ -104,7 +104,7 @@ atspi_accessible_init (AtspiAccessible *accessible)
 {
 #ifdef DEBUG_REF_COUNTS
   accessible_count++;
-  printf("at-spi: init: %d objects\n", accessible_count);
+  g_print("at-spi: init: %d objects\n", accessible_count);
 #endif
 }
 
@@ -114,6 +114,8 @@ atspi_accessible_dispose (GObject *object)
   AtspiAccessible *accessible = ATSPI_ACCESSIBLE (object);
   AtspiEvent e;
   AtspiAccessible *parent;
+  GList *children;
+  GList *l;
 
   /* TODO: Only fire if object not already marked defunct */
   memset (&e, 0, sizeof (e));
@@ -149,6 +151,20 @@ atspi_accessible_dispose (GObject *object)
     accessible->accessible_parent = NULL;
   }
 
+  children = accessible->children;
+  accessible->children = NULL;
+  for (l = children; l; l = l->next)
+  {
+    AtspiAccessible *child = l->data;
+    if (child && child->accessible_parent == accessible)
+    {
+      g_object_unref (accessible);
+      child->accessible_parent = NULL;
+    }
+    g_object_unref (child);
+  }
+  g_list_free (children);
+
   G_OBJECT_CLASS (atspi_accessible_parent_class) ->dispose (object);
 }
 

diff --git a/atspi/atspi-application.c b/atspi/atspi-application.c
index 9039190..65cabdc 100644 (file)
--- a/atspi/atspi-application.c
+++ b/atspi/atspi-application.c
@@ -43,6 +43,8 @@ atspi_application_dispose (GObject *object)
 
   if (application->bus)
   {
+    if (application->bus != _atspi_bus ())
+      dbus_connection_close (application->bus);
     dbus_connection_unref (application->bus);
     application->bus = NULL;
   }
@@ -54,6 +56,12 @@ atspi_application_dispose (GObject *object)
     application->hash = NULL;
   }
 
+  if (application->root)
+  {
+    g_object_unref (application->root);
+    application->root = NULL;
+  }
+
   G_OBJECT_CLASS (atspi_application_parent_class)->dispose (object);
 }
 

diff --git a/atspi/atspi-misc.c b/atspi/atspi-misc.c
index 482c5fa..8e85361 100644 (file)
--- a/atspi/atspi-misc.c
+++ b/atspi/atspi-misc.c
@@ -124,10 +124,13 @@ _atspi_bus ()
 
 #define APP_IS_REGISTRY(app) (!strcmp (app->bus_name, atspi_bus_registry))
 
+static AtspiAccessible *desktop;
+
 static void
 cleanup ()
 {
   GHashTable *refs;
+  GList *l;
 
   refs = live_refs;
   live_refs = NULL;
@@ -142,6 +145,20 @@ cleanup ()
       dbus_connection_unref (bus);
       bus = NULL;
     }
+
+  if (!desktop)
+    return;
+  for (l = desktop->children; l;)
+  {
+    GList *next = l->next;
+    AtspiAccessible *child = l->data;
+    g_object_run_dispose (G_OBJECT (child->parent.app));
+    g_object_run_dispose (G_OBJECT (child));
+    l = next;
+  }
+  g_object_run_dispose (G_OBJECT (desktop->parent.app));
+  g_object_unref (desktop);
+  desktop = NULL;
 }
 
 static gboolean atspi_inited = FALSE;
@@ -247,6 +264,7 @@ ref_accessible (const char *app_name, const char *path)
     {
       app->root = _atspi_accessible_new (app, atspi_path_root);
       app->root->accessible_parent = atspi_get_desktop (0);
+      app->root->accessible_parent->children = g_list_append (app->root->accessible_parent->children, g_object_ref (app->root));
     }
     return g_object_ref (app->root);
   }
@@ -259,8 +277,7 @@ ref_accessible (const char *app_name, const char *path)
   a = _atspi_accessible_new (app, path);
   if (!a)
     return NULL;
-  g_hash_table_insert (app->hash, g_strdup (a->parent.path), a);
-  g_object_ref (a);    /* for the hash */
+  g_hash_table_insert (app->hash, g_strdup (a->parent.path), g_object_ref (a));
   return a;
 }
 
@@ -354,6 +371,18 @@ handle_name_owner_changed (DBusConnection *bus, DBusMessage *message, void *user
     else if (!new[0])
       registry_lost = TRUE;
   }
+  else
+  {
+    AtspiAccessible *desktop = atspi_get_desktop (0);
+    GList *l;
+    for (l = desktop->children; l; l = l->next)
+    {
+      AtspiAccessible *child = l->data;
+      if (!strcmp (child->parent.app->bus_name, old))
+        g_object_run_dispose (G_OBJECT (child->parent.app));
+    }
+    g_object_unref (desktop);
+  }
   return DBUS_HANDLER_RESULT_HANDLED;
 }
 
@@ -361,16 +390,10 @@ static gboolean
 add_app_to_desktop (AtspiAccessible *a, const char *bus_name)
 {
   AtspiAccessible *obj = ref_accessible (bus_name, atspi_path_root);
-  if (obj)
-  {
-    a->children = g_list_append (a->children, obj);
-    return TRUE;
-  }
-  else
-  {
-    g_warning ("AT-SPI: Error calling getRoot for %s", bus_name);
-  }
-  return FALSE;
+  /* The app will be added to the desktop as a side-effect of calling
+   * ref_accessible */
+  g_object_unref (obj);
+  return (obj != NULL);
 }
 
 static void
@@ -419,8 +442,6 @@ remove_app_from_desktop (AtspiAccessible *a, const char *bus_name)
   return TRUE;
 }
 
-static AtspiAccessible *desktop;
-
 void
 get_reference_from_iter (DBusMessageIter *iter, const char **app_name, const char **path)
 {
@@ -560,8 +581,9 @@ ref_accessible_desktop (AtspiApplication *app)
   {
     return NULL;
   }
-  g_hash_table_insert (app->hash, desktop->parent.path, desktop);
-  g_object_ref (desktop);      /* for the hash */
+  g_hash_table_insert (app->hash, g_strdup (desktop->parent.path),
+                       g_object_ref (desktop));
+  app->root = g_object_ref (desktop);
   desktop->name = g_strdup ("main");
   message = dbus_message_new_method_call (atspi_bus_registry,
        atspi_path_root,
@@ -597,17 +619,20 @@ ref_accessible_desktop (AtspiApplication *app)
   if (bus_name_dup)
     g_hash_table_insert (app_hash, bus_name_dup, app);
 
-  return desktop;
+  return g_object_ref (desktop);
 }
 
 AtspiAccessible *
 _atspi_ref_accessible (const char *app, const char *path)
 {
   AtspiApplication *a = get_application (app);
-  if (!a) return NULL;
+  if (!a)
+    return NULL;
   if ( APP_IS_REGISTRY(a))
   {
-    return a->root = ref_accessible_desktop (a);
+    if (!a->root)
+      g_object_unref (ref_accessible_desktop (a));     /* sets a->root */
+    return g_object_ref (a->root);
   }
   return ref_accessible (app, path);
 }

diff --git a/configure.ac b/configure.ac
index 52d012a..fd0efdd 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -203,6 +203,7 @@ dbind/dbind-config.h
        bus/Makefile
 doc/Makefile
 doc/libatspi/Makefile
+test/Makefile
 atspi-2.pc
 atspi-2-uninstalled.pc
 ])

diff --git a/test/Makefile.am b/test/Makefile.am
new file mode 100644 (file)
index 0000000..ab47b94
--- /dev/null
+++ b/test/Makefile.am
@@ -0,0 +1,5 @@
+LDADD = $(top_builddir)/atspi/libatspi.la
+noinst_PROGRAMS = memory
+memory_SOURCES = memory.c
+memory_CFLAGS = $(GLIB_CFLAGS) $(DBUS_CFLAGS)
+memory_LDFLAGS = 

diff --git a/test/memory.c b/test/memory.c
new file mode 100644 (file)
index 0000000..28c3a8e
--- /dev/null
+++ b/test/memory.c
@@ -0,0 +1,78 @@
+#include "atspi/atspi.h"
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+
+pid_t child_pid;
+AtspiEventListener *listener;
+
+void
+basic (AtspiAccessible *obj)
+{
+  gchar *str;
+  gint count;
+  gint i;
+  AtspiAccessible *accessible;
+  GError *error = NULL;
+
+  str = atspi_accessible_get_name (obj, &error);
+  if (str)
+    g_free (str);
+  accessible = atspi_accessible_get_parent (obj, NULL);
+  if (accessible)
+    g_object_unref (accessible);
+  count = atspi_accessible_get_child_count (obj, &error);
+  for (i = 0; i < count; i++)
+  {
+    accessible = atspi_accessible_get_child_at_index (obj, i, &error);
+    if (accessible)
+      g_object_unref (accessible);
+  }
+}
+
+static gboolean
+end (void *data)
+{
+  atspi_event_quit ();
+  atspi_exit ();
+  exit (0);
+}
+
+static gboolean
+kill_child (void *data)
+{
+  kill (child_pid, SIGTERM);
+  return FALSE;
+}
+
+void
+on_event (AtspiEvent *event, void *data)
+{
+  if (atspi_accessible_get_role (event->source, NULL) == ATSPI_ROLE_DESKTOP_FRAME)
+  {
+    if (strstr (event->type, "add"))
+    {
+      AtspiAccessible *desktop = atspi_get_desktop (0);
+      basic (desktop);
+      g_object_unref (desktop);
+      g_timeout_add (3000, kill_child, NULL);
+    }
+    else
+    {
+      g_idle_add (end, NULL);
+    }
+  }
+  g_boxed_free (ATSPI_TYPE_EVENT, event);
+}
+
+main()
+{
+  atspi_init ();
+
+  listener = atspi_event_listener_new (on_event, NULL, NULL);
+  atspi_event_listener_register (listener, "object:children-changed", NULL);
+  child_pid = fork ();
+  if (!child_pid)
+    execlp ("gedit", "gedit", NULL);
+  atspi_event_main ();
+}