Merge lp:~agateau/libindicate/free-fixes into lp:libindicate/0.6

[Aurélien Gâteau]

[Aurélien Gâteau (agateau)]

Necessary to get more libindicate-qt tests to pass.

[Ted Gould (ted)]

  review needs-information

The remove filter makes complete sense to me.

The return on the destruction of the proxy struct doesn't. It seems
like if the proxy is already freed, we'd still need to clean up the
members of the proxy_t object itself. It seems like that'd just
introduce a memory leak as if the proxy dies we'd never destroy the
struct.

review: Needs Information

[Aurélien Gâteau (agateau)]

> review needs-information
>
> The remove filter makes complete sense to me.
>
> The return on the destruction of the proxy struct doesn't. It seems
> like if the proxy is already freed, we'd still need to clean up the
> members of the proxy_t object itself. It seems like that'd just
> introduce a memory leak as if the proxy dies we'd never destroy the
> struct.

I assumed this line from proxy_destroyed():

 proxyt->proxy = NULL; /* Clear this so we don't get a double free on this guy */

was setting proxy to NULL as a marker to avoid a crash in proxy_struct_destroy(), but now that I think more about it, I am probably wrong. My first attempt was to modify proxy_destroyed() so that it would remove the proxy_t instance from the listener proxies list. It failed, but it made more sense I think.

[lp:~agateau/libindicate/free-fixes]

363. By Aurélien Gâteau on 2010-02-10

More dbus filter fixes

364. By Aurélien Gâteau on 2010-02-10

Better fix for crash in double free on proxyt

[Ted Gould (ted)]

review: Approve

=== modified file 'libindicate/listener.c'
--- libindicate/listener.c	2010-02-09 03:10:00 +0000
+++ libindicate/listener.c	2010-02-10 14:46:13 +0000
@@ -110,6 +110,7 @@
 static void proxy_indicator_modified (DBusGProxy * proxy, guint id, const gchar * property, proxy_t * proxyt);
 static void proxy_server_count_changed (DBusGProxy * proxy, guint count, proxy_t * proxyt);
 static void proxy_get_indicator_list (DBusGProxy * proxy, GArray * indicators, GError * error, gpointer data);
+static void proxy_destroyed (GObject * proxy, gpointer user_data);
 
 /* DBus interface */
 gboolean _indicate_listener_server_get_indicator_servers (IndicateListener * listener, GList * servers);
@@ -246,6 +247,8 @@
 	g_list_foreach(priv->proxies, (GFunc)proxy_struct_destroy, NULL);
 	g_list_free(priv->proxies);
 
+	dbus_connection_remove_filter(dbus_g_connection_get_connection(priv->session_bus), dbus_filter_show_server, listener);
+
 	G_OBJECT_CLASS (indicate_listener_parent_class)->finalize (obj);
 	return;
 }
@@ -349,6 +352,7 @@
 	}
 
 	if (DBUS_IS_G_PROXY(proxy_data->proxy)) {
+		g_signal_handlers_disconnect_by_func(proxy_data->proxy, proxy_destroyed, proxy_data);
 		g_object_unref(G_OBJECT(proxy_data->proxy));
 	}
 
@@ -402,6 +406,9 @@
 {
 	proxy_t * proxyt = (proxy_t *)user_data;
 	proxyt->proxy = NULL; /* Clear this so we don't get a double free on this guy */
+	IndicateListener * listener = proxyt->listener;
+	IndicateListenerPrivate * priv = INDICATE_LISTENER_GET_PRIVATE(listener);
+	priv->proxies = g_list_remove_all(priv->proxies, proxyt);
 	proxy_struct_destroy(proxyt);
 	return;
 }

=== modified file 'libindicate/server.c'
--- libindicate/server.c	2010-02-05 22:53:21 +0000
+++ libindicate/server.c	2010-02-10 14:46:13 +0000
@@ -453,6 +453,8 @@
 		g_free(priv->type);
 	}
 
+	dbus_connection_remove_filter(dbus_g_connection_get_connection(priv->connection), dbus_filter_new_listener, server);
+
 	G_OBJECT_CLASS (indicate_server_parent_class)->finalize (obj);
 
 	return;