Fix #11613 crash Wayland flatpak unreffing GBytes window handle

Refactor: extract method gimp_widget_free_native_handle.
This reduces duplication of code and encapsulates Wayland specific code.

Call the new function in more places.
This is expected to fix #11613 but it is hard to be sure
since the exact sequence of events in 11613 was never determined
and only reproduceable in some flatpak builds.

Calling the new function in more places also should eliminate leaks.
But I did not test there was a leak prior to this fix.
This commit is contained in:
bootchk 2024-06-27 07:32:05 -04:00 committed by Lloyd Konneker
parent bdddc94151
commit 0dae61772a
7 changed files with 75 additions and 37 deletions

View File

@ -23,10 +23,6 @@
#include <gegl.h>
#include <gtk/gtk.h>
#ifdef GDK_WINDOWING_WAYLAND
#include <gdk/gdkwayland.h>
#endif
#include "libgimpbase/gimpbase.h"
#include "libgimpmath/gimpmath.h"
#include "libgimpcolor/gimpcolor.h"
@ -853,16 +849,7 @@ gimp_display_shell_dispose (GObject *object)
shell->display = NULL;
if (shell->window_handle != NULL)
{
#ifdef GDK_WINDOWING_WAYLAND
if (GDK_IS_WAYLAND_DISPLAY (gdk_display_get_default ()) &&
/* The GdkWindow is likely already destroyed. */
gtk_widget_get_window (GTK_WIDGET (shell)) != NULL)
gdk_wayland_window_unexport_handle (gtk_widget_get_window (GTK_WIDGET (shell)));
#endif
g_clear_pointer (&shell->window_handle, g_bytes_unref);
}
gimp_widget_free_native_handle (GTK_WIDGET (shell), &shell->window_handle);
G_OBJECT_CLASS (parent_class)->dispose (object);
}

View File

@ -395,6 +395,8 @@ gimp_file_dialog_dispose (GObject *object)
g_clear_pointer (&dialog->automatic_help_id, g_free);
g_clear_pointer (&dialog->automatic_label, g_free);
g_clear_pointer (&dialog->file_filter_label, g_free);
gimp_widget_free_native_handle (GTK_WIDGET (dialog), &dialog->window_handle);
}
static gboolean

View File

@ -24,18 +24,6 @@
#include <gegl.h>
#include <gtk/gtk.h>
#ifdef GDK_WINDOWING_WIN32
#include <gdk/gdkwin32.h>
#endif
#ifdef GDK_WINDOWING_X11
#include <gdk/gdkx.h>
#endif
#ifdef GDK_WINDOWING_WAYLAND
#include <gdk/gdkwayland.h>
#endif
#include "libgimpwidgets/gimpwidgets.h"
#include "gimpuitypes.h"
@ -121,16 +109,7 @@ gimp_progress_bar_dispose (GObject *object)
bar->progress_callback = NULL;
}
if (priv->window_handle != NULL)
{
#ifdef GDK_WINDOWING_WAYLAND
if (GDK_IS_WAYLAND_DISPLAY (gdk_display_get_default ()) &&
/* The GdkWindow is likely already destroyed. */
gtk_widget_get_window (GTK_WIDGET (bar)) != NULL)
gdk_wayland_window_unexport_handle (gtk_widget_get_window (GTK_WIDGET (bar)));
#endif
g_clear_pointer (&priv->window_handle, g_bytes_unref);
}
gimp_widget_free_native_handle (GTK_WIDGET (bar), &priv->window_handle);
G_OBJECT_CLASS (parent_class)->dispose (object);
}

View File

@ -205,7 +205,8 @@ gimp_dialog_constructed (GObject *object)
static void
gimp_dialog_dispose (GObject *object)
{
GdkDisplay *display = NULL;
GdkDisplay *display = NULL;
GimpDialogPrivate *private = GET_PRIVATE (object);
if (g_main_depth () == 0)
{
@ -213,6 +214,8 @@ gimp_dialog_dispose (GObject *object)
g_object_ref (display);
}
gimp_widget_free_native_handle (GTK_WIDGET (object), &private->window_handle);
G_OBJECT_CLASS (parent_class)->dispose (object);
if (display)

View File

@ -502,6 +502,7 @@ EXPORTS
gimp_unit_store_set_resolution
gimp_unit_store_set_resolutions
gimp_widget_animation_enabled
gimp_widget_free_native_handle
gimp_widget_get_color_profile
gimp_widget_get_color_transform
gimp_widget_get_monitor

View File

@ -1162,6 +1162,63 @@ gimp_widget_set_native_handle (GtkWidget *widget,
gimp_widget_set_handle_on_mapped (widget, NULL, handle);
}
/**
* gimp_widget_free_native_handle:
* @widget: a #GtkWindow
* @window_handle: (out): same pointer previously passed to set_native_handle
*
* Disposes a widget's native window handle created asynchronously after
* a previous call to gimp_widget_set_native_handle.
* This disposes what the pointer points to, a *GBytes, if any.
* Call this when the widget and the window handle it owns is being disposed.
*
* This should be called at least once, paired with set_native_handle.
* This knows how to free @window_handle, especially that on some platforms,
* an asynchronous callback must be canceled else it might call back
* with the pointer, after the widget and its private is freed.
*
* This is safe to call when deferenced @window_handle is NULL,
* when the window handle was never actually set,
* on Wayland where actual setting is asynchronous.
*
* !!! The word "handle" has two meanings.
* A "window handle" is an ID of a window.
* A "handle" also commonly means a pointer to a pointer, in this case **GBytes.
* @window_handle is both kinds of handle.
*/
void
gimp_widget_free_native_handle (GtkWidget *widget,
GBytes **window_handle)
{
g_return_if_fail (GTK_IS_WIDGET (widget));
/* window-handle as pointer to pointer must not be NULL. */
g_return_if_fail (window_handle != NULL);
#ifdef GDK_WINDOWING_WAYLAND
/* Cancel the asynch callback which has a pointer into widget's private.
* Cancel regardless whether callback has already come.
*/
if (GDK_IS_WAYLAND_DISPLAY (gdk_display_get_default ()) &&
/* The GdkWindow is likely already destroyed. */
gtk_widget_get_window (widget) != NULL)
gdk_wayland_window_unexport_handle (gtk_widget_get_window (widget));
#endif
/* On some platforms, window_handle may be NULL when an asynch callback has not come yet.
* The dereferenced pointer is the window handle.
*/
if (*window_handle != NULL)
{
/* On all platforms *window_handle is-a allocated GBytes.
* A GBytes is NOT a GObject and there is no way to ensure is-a GBytes.
*
* g_clear_pointer takes a pointer to pointer and unrefs at the pointer.
*/
g_clear_pointer (window_handle, g_bytes_unref);
}
/* Else no GBytes was ever allocated. */
}
/**
* gimp_widget_animation_enabled:
*
@ -1232,6 +1289,13 @@ _gimp_widget_get_profiles (GtkWidget *widget,
/* Private functions */
#ifdef GDK_WINDOWING_WAYLAND
/* Handler from asynchronous callback from Wayland.
* You can't know when it will be called, if ever.
*
* @phandle is the address of a pointer in some widget's private data.
* Before freeing that private data, you must cancel (unexport)
* so that this callback is not subsequently invoked.
*/
static void
gimp_widget_wayland_window_exported (GdkWindow *window,
const char *handle,

View File

@ -69,6 +69,8 @@ const Babl * gimp_widget_get_render_space (GtkWidget *widget,
void gimp_widget_set_native_handle (GtkWidget *widget,
GBytes **handle);
void gimp_widget_free_native_handle (GtkWidget *widget,
GBytes **window_handle);
gboolean gimp_widget_animation_enabled (void);