GNOME Bugzilla – Bug 649114
Crash when trying to kill unresponsive client with no title
Last modified: 2011-05-03 15:01:30 UTC
Created attachment 186992 [details] Test case When gnome-shell tries to kill an unresponsive client it ends up crashing it self instead. To reproduce: 1. Compile and launch test case code attached 2. Try to close the window using the x button (or alt+f4) 3. Wait a few seconds. Expected result: The client should be killed. Actual result: gnome-shell crashes and reloads. If you repeatedly try to close the window gnome-shell will give you a crash screen and require you to log out. Note: I experience the same result even when i set the WM_CLIENT_MACHINE and _NET_WM_PID properties.
From delete.c: window_title = g_locale_from_utf8 (window->title, -1, NULL, NULL, NULL); /* Translators: %s is a window title */ tmp = g_strdup_printf (_("<tt>%s</tt> is not responding."), window_title); window_content = g_strdup_printf ( "<big><b>%s</b></big>\n\n<i>%s</i>", tmp, _("You may choose to wait a short while for it to " "continue or force the application to quit entirely.")); g_free (window_title); dialog_pid = meta_show_dialog ("--question", window_content, NULL, window->screen->screen_name, _("_Wait"), _("_Force Quit"), window->xwindow, NULL, NULL); So, the bug is that g_locale_from_utf8() crashes when window->title is NULL; we really should be using an alternate string. Like "Untitled window is not responding" - if we have to do a new string, we might as wellhg avoid saying "<tt>Untitled</tt> is not responding"." But there's also a bug that we call g_locale_from_utf8() at all on the title, but don't for all the other arguments. If meta_show_dialog() wants to convert things to the locale encoding to pass to Zenity via the command line, then it should do so internally for all arguments. And yes, it should, since GOption inside Zenity will do g_locale_to_utf8(). (Really, the not-responding dialog should be turned into a GNOME Shell dialog, which would also allow us to primarily use the application name and not say <tt>Bug 649114 - Crash when trying to kill unresponsive client - Mozilla Firefox</tt> is not responding )
Created attachment 187069 [details] [review] meta_show_dialog: fix encoding of command line arguments Here's a warm-up patch to fix the noted issue with encoding; I have grave doubts about GNOME working correctly in a non-UTF-8 locale, and wouldn't have bothered to fix this except having blatantly wrong code in there bothered me. === Command line arguments are supposed to be in the locale encoding, not UTF-8, and Zenity decodes command line string command line arguments with this assumption using GOption. There was a half-hearted attempt to deal with this in delete.c, but it wasn't correct since it immediately mixed the window title, converted to the locale encoding with a UTF-8 message.
Created attachment 187073 [details] [review] Fix escaping for application-not-responding dialog We need to escape markup in the title, or a title "<i>Italic</i>" will be displayed as italic.
Created attachment 187074 [details] [review] meta_show_dialog: fix encoding of command line arguments Fix a g_free() that shouldn't have been there.
Created attachment 187075 [details] [review] Fix crash with non-responding application with no title If a window had no title property set, then the application-not-responding dialog would cause Mutter to crash because window->title was NULL; handle that case and use the string "Application is not responding."
Comment on attachment 187073 [details] [review] Fix escaping for application-not-responding dialog >+ tmp = g_markup_printf_escaped (_("<tt>%s</tt> is not responding."), huh. I don't think I knew about that function...
Comment on attachment 187074 [details] [review] meta_show_dialog: fix encoding of command line arguments i guess since this is an error anyway it's not worth trying to optimize out not converting the known-to-be-ASCII strings
(In reply to comment #6) > (From update of attachment 187073 [details] [review]) > >+ tmp = g_markup_printf_escaped (_("<tt>%s</tt> is not responding."), > > huh. I don't think I knew about that function... It's my favorite function in GLib, at least of the ones I've written: A) It's really useful B) It appears at first glance impossible to write without a private printf implementation, but a clever implementation technique avoids that need at the cost of a bit of runtime inefficiency.
Attachment 187073 [details] pushed as 60dd31e - Fix escaping for application-not-responding dialog Attachment 187074 [details] pushed as 40f5111 - meta_show_dialog: fix encoding of command line arguments Attachment 187075 [details] pushed as 89dbef9 - Fix crash with non-responding application with no title