GNOME Bugzilla – Bug 786385
Shortcuts inhibit dialog crashes the system
Last modified: 2017-08-19 09:27:57 UTC
Every application prompting with the new shortcut inhibit dialogs crashes the system as soon one of the buttons are pressed. This is the log output: Aug 15 20:39:14 buzz-fedlet systemd-coredump[31050]: Process 1962 (gnome-shell) of user 1000 dumped core. Stack trace of thread 1962: #0 0x00007f640a50cafc g_hash_table_contains (libglib-2.0.so.0) #1 0x00007f6408abadb7 inhibit_shortcuts_dialog_response_apply (libmutter-1.so.0) #2 0x00007f6408abafdf inhibit_shortcuts_dialog_response_cb (libmutter-1.so.0) #3 0x00007f640a7f68cd g_closure_invoke (libgobject-2.0.so.0) #4 0x00007f640a80a19e signal_emit_unlocked_R (libgobject-2.0.so.0) #5 0x00007f640a8120e3 g_signal_emitv (libgobject-2.0.so.0) #6 0x00007f6409507381 n/a (libgjs.so.0) #7 0x00007f64015b93d8 _ZN2js6InvokeEP9JSContextN2JS8CallArgsENS_14MaybeConstructE (libmozjs-38.so) #8 0x00007f64015af2cd _ZL9InterpretP9JSContextRN2js8RunStateE (libmozjs-38.so) #9 0x00007f64015b9054 _ZN2js9RunScriptEP9JSContextRNS_8RunStateE (libmozjs-38.so) #10 0x00007f64015b9344 _ZN2js6InvokeEP9JSContextN2JS8CallArgsENS_14MaybeConstructE (libmozjs-38.so) #11 0x00007f64015b9f73 _ZN2js6InvokeEP9JSContextRKN2JS5ValueES5_jPS4_NS2_13MutableHandleIS3_EE (libmozjs-38.so) #12 0x00007f640181215b _ZN2js3jit14InvokeFunctionEP9JSContextN2JS6HandleIP8JSObjectEEjPNS3_5ValueES9_ (libmozjs-38.so) #13 0x00007f640b2a7134 n/a (ld-linux-x86-64.so.2)
Created attachment 357912 [details] [review] wayland/inhibit-shortcuts-dialog: Only reuse last reply if there was one We might have hidden the dialog, without a response. To avoid using the not answered response, make sure we have actually got one before reusing.
Created attachment 357913 [details] [review] wayland/inhibit-shortcuts-dialog: Make data life cycle a bit clearer The 'data' object is attached to the MetaWaylandSurface as a GObject qdata. It is created once, and stays allocated until the surface is destroyed. To make things clearer, connect to the "destroy" signal just after creating, and from a on_surface_destroyed() callback call the .._free() function.
Created attachment 357914 [details] [review] wayland/inhibit-shortcuts-dialog: Make the dialog ownership clearer Make it clear that the data object is the owner of the dialog; it creates it, and eventually destroys it.
Created attachment 357915 [details] [review] wayland/inhibit-shortcuts-dialog: Just hide the dialog when hiding The meta_wayland_surface_hide_inhibit_shortcuts_dialog() function disconnected the "destroy" handler, but we'd still be listening on response events. Change this to just hide the dialog, leaving the data intact with the proper life time signal in place.
Created attachment 357916 [details] [review] wayland/inhibit-shortcuts-dialog: Destroy the dialog after response We'll never actually show it again, but just use the last response, so we can just destroy it now already.
Review of attachment 357912 [details] [review]: OK
Review of attachment 357913 [details] [review]: The commit message is a bit confusing - the most relevant change (IMHO) is that you fix the wrong destroy handler, so the free function is called on the actual data rather than the surface ...
Review of attachment 357914 [details] [review]: LGTM
Review of attachment 357915 [details] [review]: OK
Review of attachment 357916 [details] [review]: OK
Fwiw, the gtk+ bits were handled in https://bugzilla.gnome.org/show_bug.cgi?id=786480
(In reply to Florian Müllner from comment #7) > Review of attachment 357913 [details] [review] [review]: > > The commit message is a bit confusing - the most relevant change (IMHO) is > that you fix the wrong destroy handler, so the free function is called on > the actual data rather than the surface ... Actually, seems I made a mistake when splitting up the changes into commits. It already freed the data correctly, this patch should just have moved around and renamed things to make it easier to understand.
Pushed after having corrected the 'wayland/inhibit-shortcuts-dialog: Make data life cycle a bit clearer' patch. Attachment 357912 [details] pushed as a7915ff - wayland/inhibit-shortcuts-dialog: Only reuse last reply if there was one Attachment 357913 [details] pushed as dceb0f1 - wayland/inhibit-shortcuts-dialog: Make data life cycle a bit clearer Attachment 357914 [details] pushed as c1439e1 - wayland/inhibit-shortcuts-dialog: Make the dialog ownership clearer Attachment 357915 [details] pushed as 2f45e88 - wayland/inhibit-shortcuts-dialog: Just hide the dialog when hiding Attachment 357916 [details] pushed as 66996de - wayland/inhibit-shortcuts-dialog: Destroy the dialog after response