GNOME Bugzilla – Bug 668299
Crash when meta_display_close() is called with attached modal dialogs
Last modified: 2012-01-19 21:13:28 UTC
If you have an attached modal dialog open, restarting gnome-shell with Alt-F2 r will cause a crash; for backtraces see: https://bugzilla.redhat.com/show_bug.cgi?id=760918 The problem is the code in meta_window_unmanage(): if (meta_prefs_get_attach_modal_dialogs ()) { GList *attached_children = NULL, *iter; /* Detach any attached dialogs by unmapping and letting them * be remapped after @window is destroyed. */ meta_window_foreach_transient (window, detach_foreach_func, &attached_children); for (iter = attached_children; iter; iter = iter->next) meta_window_unmanage (iter->data, timestamp); g_list_free (attached_children); } This means that when we unmanage one window, we can unmanage other windows, and if we don't guard against that when we're iterating over all windows and unmanaging them, we'll crash.
Created attachment 205656 [details] [review] Fix crash when meta_display_close() is called with attached modal dialogs When meta_display_unmanage_window_for_screen() is called, it gets a list of windows and iterates over them and unmanages them, but unmanaging a window with attached modal dialogs also unmanages those attached modal dialogs (in the normal case, temporarily), so we need to guard against such cases by ref'ing the windows in the list and checking if they have already been unmanaged. https://bugzilla.gnome.org/show_bug.cgi?id=668299 https://bugzilla.redhat.com/show_bug.cgi?id=760918
Review of attachment 205656 [details] [review]: ::: src/core/display.c @@ +5026,3 @@ META_LIST_INCLUDE_OVERRIDE_REDIRECT); winlist = g_slist_sort (winlist, meta_display_stack_cmp); + g_slist_foreach (winlist, (GFunc)g_object_unref, NULL); uh, what? Was this meant to be "g_object_ref"? I don't see where we're reffing them otherwise.
Review of attachment 205656 [details] [review]: You mention ref'ing the windows, but you're not adding a new _ref() call, only moving an _unref(). I think the commit is right, but maybe phrase the commit message like "need to guard against such cases by holding the reference longer"?
Created attachment 205659 [details] [review] Fix crash when meta_display_close() is called with attached modal dialogs Fixed version
Review of attachment 205659 [details] [review]: Seems sane enough to me.
Pushed