ayatana-commits team mailing list archive

[Michael Terry <michael.terry@xxxxxxxxxxxxx>]

Michael Terry has proposed merging lp:~mterry/indicator-appmenu/free-entries-first into lp:indicator-appmenu.

Requested reviews:
  Indicator Applet Developers (indicator-applet-developers)

For more details, see:
https://code.launchpad.net/~mterry/indicator-appmenu/free-entries-first/+merge/49120

So, when a WindowMenus object is free'd, it first unref's (which frees) its dbusmenu client during dispose.  Then, during finalize, it frees all the entries it has.

The problem is that the dbusmenu client was holding a GtkAccel object that all GtkAccelLabels in the entries were pointing at (and apparently such GtkAccelLabels aren't smart enough to notice when it dies).

So you can get crashes like in bug 685151.  I was only seeing criticals and warnings, but for some (like Seb), it manifested as crashes in the panel.

The easiest fixes I saw were to never free the GtkAccel object (which is obviously bad) or free the entries before freeing the dbusmenu client.

So I moved the entry-freeing into the dispose method and dropped the now-empty finalize method.  Is that suitable?  Is there a reason entries should be free'd during finalize?
-- 
https://code.launchpad.net/~mterry/indicator-appmenu/free-entries-first/+merge/49120
Your team ayatana-commits is subscribed to branch lp:indicator-appmenu.

=== modified file 'src/window-menus.c'
--- src/window-menus.c	2011-02-09 14:02:51 +0000
+++ src/window-menus.c	2011-02-09 19:38:28 +0000
@@ -76,7 +76,6 @@
 static void window_menus_class_init (WindowMenusClass *klass);
 static void window_menus_init       (WindowMenus *self);
 static void window_menus_dispose    (GObject *object);
-static void window_menus_finalize   (GObject *object);
 static void root_changed            (DbusmenuClient * client, DbusmenuMenuitem * new_root, gpointer user_data);
 static void menu_entry_added        (DbusmenuMenuitem * root, DbusmenuMenuitem * newentry, guint position, gpointer user_data);
 static void menu_entry_removed      (DbusmenuMenuitem * root, DbusmenuMenuitem * oldentry, gpointer user_data);
@@ -96,7 +95,6 @@
 	g_type_class_add_private (klass, sizeof (WindowMenusPrivate));
 
 	object_class->dispose = window_menus_dispose;
-	object_class->finalize = window_menus_finalize;
 
 	/* Signals */
 	signals[ENTRY_ADDED] =  g_signal_new(WINDOW_MENUS_SIGNAL_ENTRY_ADDED,
@@ -148,46 +146,6 @@
 	return;
 }
 
-/* Destroy objects */
-static void
-window_menus_dispose (GObject *object)
-{
-	WindowMenusPrivate * priv = WINDOW_MENUS_GET_PRIVATE(object);
-
-	if (priv->root != NULL) {
-		g_object_unref(G_OBJECT(priv->root));
-		priv->root = NULL;
-	}
-
-	if (priv->client != NULL) {
-		g_object_unref(G_OBJECT(priv->client));
-		priv->client = NULL;
-	}
-	
-	if (priv->props != NULL) {
-		g_object_unref(G_OBJECT(priv->props));
-		priv->props = NULL;
-	}
-
-	if (priv->props_cancel != NULL) {
-		g_cancellable_cancel(priv->props_cancel);
-		g_object_unref(priv->props_cancel);
-		priv->props_cancel = NULL;
-	}
-
-	if (priv->retry_timer != 0) {
-		g_source_remove(priv->retry_timer);
-		priv->retry_timer = 0;
-		g_variant_unref(priv->retry_data);
-		priv->retry_data = NULL;
-		g_free(priv->retry_name);
-		priv->retry_name = NULL;
-	}
-
-	G_OBJECT_CLASS (window_menus_parent_class)->dispose (object);
-	return;
-}
-
 static void
 entry_free(IndicatorObjectEntry * entry)
 {
@@ -238,14 +196,12 @@
 	}
 }
 
-/* Free memory */
+/* Destroy objects */
 static void
-window_menus_finalize (GObject *object)
+window_menus_dispose (GObject *object)
 {
 	WindowMenusPrivate * priv = WINDOW_MENUS_GET_PRIVATE(object);
 
-	g_debug("Window Menus Object finalizing for: %d", priv->windowid);
-
 	free_entries(object, FALSE);
 
 	if (priv->entries != NULL) {
@@ -253,7 +209,37 @@
 		priv->entries = NULL;
 	}
 
-	G_OBJECT_CLASS (window_menus_parent_class)->finalize (object);
+	if (priv->root != NULL) {
+		g_object_unref(G_OBJECT(priv->root));
+		priv->root = NULL;
+	}
+
+	if (priv->client != NULL) {
+		g_object_unref(G_OBJECT(priv->client));
+		priv->client = NULL;
+	}
+	
+	if (priv->props != NULL) {
+		g_object_unref(G_OBJECT(priv->props));
+		priv->props = NULL;
+	}
+
+	if (priv->props_cancel != NULL) {
+		g_cancellable_cancel(priv->props_cancel);
+		g_object_unref(priv->props_cancel);
+		priv->props_cancel = NULL;
+	}
+
+	if (priv->retry_timer != 0) {
+		g_source_remove(priv->retry_timer);
+		priv->retry_timer = 0;
+		g_variant_unref(priv->retry_data);
+		priv->retry_data = NULL;
+		g_free(priv->retry_name);
+		priv->retry_name = NULL;
+	}
+
+	G_OBJECT_CLASS (window_menus_parent_class)->dispose (object);
 	return;
 }