After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 668299 - Crash when meta_display_close() is called with attached modal dialogs
Crash when meta_display_close() is called with attached modal dialogs
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2012-01-19 19:19 UTC by Owen Taylor
Modified: 2012-01-19 21:13 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix crash when meta_display_close() is called with attached modal dialogs (1.79 KB, patch)
2012-01-19 19:22 UTC, Owen Taylor
accepted-commit_now Details | Review
Fix crash when meta_display_close() is called with attached modal dialogs (1.79 KB, patch)
2012-01-19 20:14 UTC, Owen Taylor
accepted-commit_now Details | Review

Description Owen Taylor 2012-01-19 19:19:01 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.
Comment 1 Owen Taylor 2012-01-19 19:22:23 UTC
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
Comment 2 Jasper St. Pierre (not reading bugmail) 2012-01-19 20:06:09 UTC
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.
Comment 3 Colin Walters 2012-01-19 20:07:26 UTC
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"?
Comment 4 Owen Taylor 2012-01-19 20:14:59 UTC
Created attachment 205659 [details] [review]
Fix crash when meta_display_close() is called with attached modal dialogs

Fixed version
Comment 5 Jasper St. Pierre (not reading bugmail) 2012-01-19 20:36:19 UTC
Review of attachment 205659 [details] [review]:

Seems sane enough to me.
Comment 6 Owen Taylor 2012-01-19 21:13:28 UTC
Pushed