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.

7 files changed, 154 insertions, 21 deletions

diff --git a/Makefile.am b/Makefile.am
index 37d80085..c67aab60 100644
--- 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 7971bd3c..6732d1c9 100644
--- 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 90391908..65cabdc4 100644
--- 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 482c5fa7..8e853615 100644
--- 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 52d012ac..fd0efddf 100644
--- 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
index 00000000..ab47b948
--- /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
index 00000000..28c3a8ec
--- /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 ();
+}