From c24358cfbc1c6712d7a14ae8543c2346a16aea2d Mon Sep 17 00:00:00 2001 From: Carlos Garnacho Date: Wed, 7 Dec 2022 13:16:36 +0100 Subject: [PATCH] gdk/x11: Clear all data in GdkSurfaceX11 finalization Currently, the GdkSurfaceX11 implementation relies that the upper layers hid the surface before destruction, and that no GdkSurfaceClass.compute_resize happened between them. If these circumstances happened, there would be a compute_size timeout left dangling after the surface got destroyed, poking at incorrect data later on. Something that looks like this was reported in the recent mutter-x11-frames "SSD frames server": mutter-x11-frames:423016): GLib-GObject-WARNING **: 19:41:16.869: invalid unclassed pointer in cast to 'GtkWindow' Thread 1 "mutter-x11-fram" received signal SIGTRAP, Trace/breakpoint trap. g_logv (log_domain=0x7ffff7f7c4f8 "GLib-GObject", log_level=G_LOG_LEVEL_WARNING, format=, args=) at ../../../glib/gmessages.c:1433 1433 ../../../glib/gmessages.c: No such file or directory. (gdb) bt #0 g_logv (log_domain=0x7ffff7f7c4f8 "GLib-GObject", log_level=G_LOG_LEVEL_WARNING, format=, args=) at ../../../glib/gmessages.c:1433 #1 0x00007ffff73470ff in g_log (log_domain=log_domain@entry=0x7ffff7f7c4f8 "GLib-GObject", log_level=log_level@entry=G_LOG_LEVEL_WARNING, format=format@entry=0x7ffff7f84da8 "invalid unclassed pointer in cast to '%s'") at ../../../glib/gmessages.c:1471 #2 0x00007ffff7f72892 in g_type_check_instance_cast (type_instance=type_instance@entry=0x5555558e04b0, iface_type=) at ../../../gobject/gtype.c:4144 #3 0x00007ffff791e77d in toplevel_compute_size (toplevel=, size=0x7fffffffe170, widget=0x5555558e04b0) at ../../../gtk/gtkwindow.c:4227 #4 0x00007ffff7f4f3b0 in g_closure_invoke (closure=0x555555898cc0, return_value=return_value@entry=0x0, n_param_values=2, param_values=param_values@entry=0x7fffffffdeb0, invocation_hint=invocation_hint@entry=0x7fffffffde30) at ../../../gobject/gclosure.c:832 #5 0x00007ffff7f62076 in signal_emit_unlocked_R (node=node@entry=0x55555588feb0, detail=detail@entry=0, instance=instance@entry=0x55555560e990, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7fffffffdeb0) at ../../../gobject/gsignal.c:3796 #6 0x00007ffff7f68bf5 in g_signal_emit_valist (instance=, signal_id=, detail=, var_args=var_args@entry=0x7fffffffe050) at ../../../gobject/gsignal.c:3549 #7 0x00007ffff7f68dbf in (instance=, signal_id=, detail=detail@entry=0) at ../../../gobject/gsignal.c:3606 #8 0x00007ffff7a8de96 in gdk_toplevel_notify_compute_size (toplevel=, size=size@entry=0x7fffffffe170) at ../../../gdk/gdktoplevel.c:112 #9 0x00007ffff7a4b15a in compute_toplevel_size (surface=surface@entry=0x55555560e990 [GdkX11Toplevel], update_geometry=update_geometry@entry=1, width=width@entry=0x7fffffffe220, height=height@entry=0x7fffffffe224) at ../../../gdk/x11/gdksurface-x11.c:281 #10 0x00007ffff7a4c3b2 in compute_size_idle (user_data=0x55555560e990) at ../../../gdk/x11/gdksurface-x11.c:356 #11 0x00007ffff733f67f in g_main_dispatch (context=0x55555563f6e0) at ../../../glib/gmain.c:3444 #12 g_main_context_dispatch (context=context@entry=0x55555563f6e0) at ../../../glib/gmain.c:4162 #13 0x00007ffff733fa38 in g_main_context_iterate (context=0x55555563f6e0, block=block@entry=1, dispatch=dispatch@entry=1, self=) at ../../../glib/gmain.c:4238 #14 0x00007ffff733fcef in g_main_loop_run (loop=loop@entry=0x5555560874a0) at ../../../glib/gmain.c:4438 #15 0x0000555555557de0 in main (argc=, argv=) at ../src/frames/main.c:68 It perhaps makes sense to warn in these situations, but either way it sounds like gdk_surface_x11_finalize() could enforce the correct behavior by ensuring there is no dangling timeouts/data. This commit does that. --- gdk/x11/gdksurface-x11.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gdk/x11/gdksurface-x11.c b/gdk/x11/gdksurface-x11.c index b77cd2e3ad..f32190b385 100644 --- a/gdk/x11/gdksurface-x11.c +++ b/gdk/x11/gdksurface-x11.c @@ -759,6 +759,8 @@ gdk_x11_surface_finalize (GObject *object) } g_clear_pointer (&impl->surface_is_on_monitor, g_list_free); + g_clear_handle_id (&impl->compute_size_source_id, g_source_remove); + g_clear_pointer (&impl->toplevel_layout, gdk_toplevel_layout_unref); g_free (impl->toplevel);