ayatana-commits team mailing list archive

[Ted Gould <ted@xxxxxxxx>]

Ted Gould has proposed merging lp:~ted/dbusmenu/reference-with-update into lp:dbusmenu.

Requested reviews:
  DBus Menu Team (dbusmenu-team)
Related bugs:
  #709754 indicator-applet crashed with SIGSEGV in g_simple_async_result_complete()
  https://bugs.launchpad.net/bugs/709754

For more details, see:
https://code.launchpad.net/~ted/dbusmenu/reference-with-update/+merge/48022

A set of bugs that were caused by objects getting unreferenced without entirely cleaning up.  These were a result of working with lool on #ubuntu-desktop who had a stressful situation on his laptop and could recreate easily.  Thanks lool!
-- 
https://code.launchpad.net/~ted/dbusmenu/reference-with-update/+merge/48022
Your team ayatana-commits is subscribed to branch lp:dbusmenu.

=== modified file 'libdbusmenu-glib/client.c'
--- libdbusmenu-glib/client.c	2011-01-27 19:54:18 +0000
+++ libdbusmenu-glib/client.c	2011-01-31 17:04:14 +0000
@@ -129,6 +129,12 @@
 	gchar * type;
 };
 
+typedef struct _properties_callback_t properties_callback_t;
+struct _properties_callback_t {
+	DbusmenuClient * client;
+	GArray * listeners;
+};
+
 
 #define DBUSMENU_CLIENT_GET_PRIVATE(o) (DBUSMENU_CLIENT(o)->priv)
 #define DBUSMENU_INTERFACE  "com.canonical.dbusmenu"
@@ -512,7 +518,8 @@
 static void 
 get_properties_callback (GObject *obj, GAsyncResult * res, gpointer user_data)
 {
-	GArray * listeners = (GArray *)user_data;
+	properties_callback_t * cbdata = (properties_callback_t *)user_data;
+	GArray * listeners = cbdata->listeners;
 	int i;
 	GError * error = NULL;
 	GVariant * params = NULL;
@@ -526,9 +533,8 @@
 			properties_listener_t * listener = &g_array_index(listeners, properties_listener_t, i);
 			listener->callback(NULL, error, listener->user_data);
 		}
-		g_array_free(listeners, TRUE);
 		g_error_free(error);
-		return;
+		goto out;
 	}
 
 	/* Callback all the folks we can find */
@@ -575,8 +581,11 @@
 		g_error_free(localerror);
 	}
 
+out:
 	/* Clean up */
 	g_array_free(listeners, TRUE);
+	g_object_unref(cbdata->client);
+	g_free(user_data);
 
 	return;
 }
@@ -586,6 +595,7 @@
 static gboolean
 get_properties_idle (gpointer user_data)
 {
+	properties_callback_t * cbdata = NULL;
 	DbusmenuClientPrivate * priv = DBUSMENU_CLIENT_GET_PRIVATE(user_data);
 	g_return_val_if_fail(priv->menuproxy != NULL, TRUE);
 
@@ -616,6 +626,11 @@
 	g_variant_builder_add_value(&builder, variant_props);
 	GVariant * variant_params = g_variant_builder_end(&builder);
 
+	cbdata = g_new(properties_callback_t, 1);
+	cbdata->listeners = priv->delayed_property_listeners;
+	cbdata->client = DBUSMENU_CLIENT(user_data);
+	g_object_ref(G_OBJECT(user_data));
+
 	g_dbus_proxy_call(priv->menuproxy,
 	                  "GetGroupProperties",
 	                  variant_params,
@@ -623,7 +638,7 @@
 	                  -1,   /* timeout */
 	                  NULL, /* cancellable */
 	                  get_properties_callback,
-	                  priv->delayed_property_listeners);
+	                  cbdata);
 
 	/* Free properties */
 	gchar ** dataregion = (gchar **)g_array_free(priv->delayed_property_list, FALSE);
@@ -1553,11 +1568,6 @@
 	DbusmenuClient * client = DBUSMENU_CLIENT(data);
 	DbusmenuClientPrivate * priv = DBUSMENU_CLIENT_GET_PRIVATE(client);
 
-	if (priv->layoutcall != NULL) {
-		g_object_unref(priv->layoutcall);
-		priv->layoutcall = NULL;
-	}
-
 	GError * error = NULL;
 	GVariant * params = NULL;
 
@@ -1566,7 +1576,7 @@
 	if (error != NULL) {
 		g_warning("Getting layout failed: %s", error->message);
 		g_error_free(error);
-		return;
+		goto out;
 	}
 
 	guint rev;
@@ -1580,7 +1590,7 @@
 
 	if (parseable == 0) {
 		g_warning("Unable to parse layout!");
-		return;
+		goto out;
 	}
 
 	priv->my_revision = rev;
@@ -1596,6 +1606,13 @@
 		update_layout(client);
 	}
 
+out:
+	if (priv->layoutcall != NULL) {
+		g_object_unref(priv->layoutcall);
+		priv->layoutcall = NULL;
+	}
+
+	g_object_unref(G_OBJECT(client));
 	return;
 }
 
@@ -1622,6 +1639,7 @@
 
 	priv->layoutcall = g_cancellable_new();
 
+	g_object_ref(G_OBJECT(client));
 	g_dbus_proxy_call(priv->menuproxy,
 	                  "GetLayout",
 	                  g_variant_new("(i)", 0), /* root */

=== modified file 'libdbusmenu-gtk/menu.c'
--- libdbusmenu-gtk/menu.c	2010-11-11 14:15:20 +0000
+++ libdbusmenu-gtk/menu.c	2011-01-31 17:04:14 +0000
@@ -46,6 +46,7 @@
 /* Private */
 struct _DbusmenuGtkMenuPrivate {
 	DbusmenuGtkClient * client;
+	DbusmenuMenuitem * root;
 
 	gchar * dbus_object;
 	gchar * dbus_name;
@@ -63,6 +64,8 @@
 /* Internal */
 static void build_client (DbusmenuGtkMenu * self);
 static void child_realized (DbusmenuMenuitem * child, gpointer userdata);
+static void remove_child_signals (gpointer data, gpointer user_data);
+static void root_changed (DbusmenuGtkClient * client, DbusmenuMenuitem * newroot, DbusmenuGtkMenu * menu);
 
 /* GObject Stuff */
 G_DEFINE_TYPE (DbusmenuGtkMenu, dbusmenu_gtkmenu, GTK_TYPE_MENU);
@@ -127,6 +130,12 @@
 {
 	DbusmenuGtkMenuPrivate * priv = DBUSMENU_GTKMENU_GET_PRIVATE(object);
 
+	/* Remove signals from the root */
+	if (priv->root != NULL) {
+		/* This will clear the root */
+		root_changed(priv->client, NULL, DBUSMENU_GTKMENU(object));
+	}
+
 	if (priv->client != NULL) {
 		g_object_unref(G_OBJECT(priv->client));
 		priv->client = NULL;
@@ -271,6 +280,10 @@
 	#ifdef MASSIVEDEBUGGING
 	g_debug("Root child deleted");
 	#endif
+
+	/* Remove signal for realized */
+	remove_child_signals(child, menu);
+
 	DbusmenuGtkMenuPrivate * priv = DBUSMENU_GTKMENU_GET_PRIVATE(menu);
 	GtkWidget * item = GTK_WIDGET(dbusmenu_gtkclient_menuitem_get(priv->client, child));
 	if (item != NULL) {
@@ -308,15 +321,41 @@
 	return;
 }
 
+/* Remove any signals we attached to children -- just realized right now */
+static void
+remove_child_signals (gpointer data, gpointer user_data)
+{
+	g_signal_handlers_disconnect_by_func(G_OBJECT(data), child_realized, user_data);
+	return;
+}
+
 /* When the root menuitem changes we need to resetup things so that
    we're back in the game. */
 static void
 root_changed (DbusmenuGtkClient * client, DbusmenuMenuitem * newroot, DbusmenuGtkMenu * menu) {
+	DbusmenuGtkMenuPrivate * priv = DBUSMENU_GTKMENU_GET_PRIVATE(menu);
+
+	/* Clear out our interest in the old root */
+	if (priv->root != NULL) {
+		GList * children = dbusmenu_menuitem_get_children(priv->root);
+		g_list_foreach(children, remove_child_signals, menu);
+
+		g_signal_handlers_disconnect_by_func(G_OBJECT(priv->root), root_child_added, menu);
+		g_signal_handlers_disconnect_by_func(G_OBJECT(priv->root), root_child_moved, menu);
+		g_signal_handlers_disconnect_by_func(G_OBJECT(priv->root), root_child_delete, menu);
+
+		g_object_unref(priv->root);
+		priv->root = NULL;
+	}
+
 	if (newroot == NULL) {
 		gtk_widget_hide(GTK_WIDGET(menu));
 		return;
 	}
 
+	priv->root = newroot;
+	g_object_ref(priv->root);
+
 	g_signal_connect(G_OBJECT(newroot), DBUSMENU_MENUITEM_SIGNAL_CHILD_ADDED,   G_CALLBACK(root_child_added),  menu);
 	g_signal_connect(G_OBJECT(newroot), DBUSMENU_MENUITEM_SIGNAL_CHILD_MOVED,   G_CALLBACK(root_child_moved),  menu);
 	g_signal_connect(G_OBJECT(newroot), DBUSMENU_MENUITEM_SIGNAL_CHILD_REMOVED, G_CALLBACK(root_child_delete), menu);