GNOME Bugzilla – Bug 784016
Crash in gnome-terminal due to calling a GdkDisplayClass vfunc on a display not of that class
Last modified: 2017-08-15 19:36:50 UTC
I found another place that has the same issue as bug 776472. https://bugzilla.redhat.com/show_bug.cgi?id=1463182 is a crash in gnome-terminal with this trace:
+ Trace 237585
Thread 1 (Thread 0x7f57d26c0a80 (LWP 23794))
Notice that it's calling the X11 vfunc on a GdkWaylandDisplay. The code is at https://git.gnome.org/browse/gtk+/tree/gdk/gdkdisplay.c?h=gtk-3-22#n2308 : 2301 gdk_error_trap_push (void) [...] 2308 manager = gdk_display_manager_get (); 2309 class = GDK_DISPLAY_GET_CLASS (gdk_display_manager_get_default_display (manager)); 2310 2311 if (class->push_error_trap == NULL) 2312 return; 2313 2314 trap = g_slice_new (GdkGlobalErrorTrap); 2315 trap->displays = gdk_display_manager_list_displays (manager); 2316 2317 g_slist_foreach (trap->displays, (GFunc) g_object_ref, NULL); 2318 for (l = trap->displays; l != NULL; l = l->next) 2319 class->push_error_trap (l->data); This assumes that all displays returned from gdk_display_manager_list_displays() are of the same class as its default display, which is not necessarily true. Lines 2309 to 2312 need to be deleted, and 2319 changed to use the class of the GdkDisplay l->data. The same bug occurs in the gdk_error_trap_pop_internal() function below this one. I have *not* investigated any of the other occurrences of gdk_display_manager_{list_displays,get_default_display}() in gdk/gtk+ to see if there are more instance of this bug.
push() also needs to only call push_error_trap() on display classes that have that method; it looks like it is NULLable. Should we then only ref() and unref() displays that get pushed/popped?
Created attachment 357037 [details] [review] gdkdisplay: Remove a pointless assignment
Comment on attachment 357037 [details] [review] gdkdisplay: Remove a pointless assignment Attachment 357037 [details] pushed as b3ab230 - gdkdisplay: Remove a pointless assignment
Created attachment 357038 [details] [review] gdkdisplay: Call the correct (push|pop)_error_trap It is wrong to assume that all Displays are of the same class as the default. -- Testing/confirmation of sense would be appreciated.
Created attachment 357039 [details] [review] gdkdisplay: Call the correct (push|pop)_error_trap It is wrong to assume that all Displays are of the same class as the default. -- g_slice_new0() is needed now so that the list is initialised to NULL.
Hi Christian, would you be able to test/review my latest patch? It'd be good to get this in. (Then we can eventually dive in and look for other such cases...)
I don't have time to test this, but the patch looks in line with what I outlined in comment 0, so it should be fine.
Created attachment 357643 [details] [review] gdkdisplay: Call the correct (push|pop)_error_trap The GSList needs to be assigned from the return value of each g_slist_prepend().