From 645d4807c3ab0b6ac20cb08f16f39893a95bd2f2 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Mon, 13 Dec 2021 01:41:04 +0100 Subject: [PATCH 1/4] x11: Keep a reference to the SelectionOutputStream while writing This ensures close() isn't called from dispose() while we're still busy writing. In theory this should never happen, but in practice it just did. --- gdk/x11/gdkselectionoutputstream-x11.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gdk/x11/gdkselectionoutputstream-x11.c b/gdk/x11/gdkselectionoutputstream-x11.c index b9aad4c3f3..7b45638dd3 100644 --- a/gdk/x11/gdkselectionoutputstream-x11.c +++ b/gdk/x11/gdkselectionoutputstream-x11.c @@ -57,7 +57,7 @@ struct _GdkX11SelectionOutputStreamPrivate { GTask *pending_task; guint incr : 1; - guint delete_pending : 1; + guint delete_pending : 1; /* owns a reference */ }; struct _GdkX11PendingSelectionNotify @@ -292,6 +292,7 @@ gdk_x11_selection_output_stream_perform_flush (GdkX11SelectionOutputStream *stre priv->notify = NULL; } + g_object_ref (stream); priv->delete_pending = TRUE; g_cond_broadcast (&priv->cond); g_mutex_unlock (&priv->mutex); @@ -628,6 +629,7 @@ gdk_x11_selection_output_stream_xevent (GdkDisplay *display, if (gdk_x11_selection_output_stream_needs_flush (stream) && gdk_x11_selection_output_stream_can_flush (stream)) gdk_x11_selection_output_stream_perform_flush (stream); + g_object_unref (stream); /* from unsetting the delete_pending */ return FALSE; default: From 66f1fef0830337de50731d0e44d5f1b133fe5208 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Mon, 13 Dec 2021 01:43:24 +0100 Subject: [PATCH 2/4] x11: Explicitly close_async() the output stream We need to be very careful when writing data, because if we aren't, sync functions will be called on the output stream and X11 does not like that at all. --- gdk/x11/gdkclipboard-x11.c | 28 ++++++++++++++++++++++++++-- gdk/x11/gdkdrag-x11.c | 28 ++++++++++++++++++++++++++-- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/gdk/x11/gdkclipboard-x11.c b/gdk/x11/gdkclipboard-x11.c index 404c8c7cd6..4335674102 100644 --- a/gdk/x11/gdkclipboard-x11.c +++ b/gdk/x11/gdkclipboard-x11.c @@ -76,11 +76,30 @@ print_atoms (GdkX11Clipboard *cb, }); } +static void +gdk_x11_clipboard_default_output_closed (GObject *stream, + GAsyncResult *result, + gpointer user_data) +{ + GError *error = NULL; + + if (!g_output_stream_close_finish (G_OUTPUT_STREAM (stream), result, &error)) + { + GDK_NOTE (CLIPBOARD, + g_printerr ("-------: failed to close stream: %s\n", + error->message)); + g_error_free (error); + } + + g_object_unref (stream); +} + static void gdk_x11_clipboard_default_output_done (GObject *clipboard, GAsyncResult *result, gpointer user_data) { + GOutputStream *stream = user_data; GError *error = NULL; if (!gdk_clipboard_write_finish (GDK_CLIPBOARD (clipboard), result, &error)) @@ -90,6 +109,12 @@ gdk_x11_clipboard_default_output_done (GObject *clipboard, GDK_X11_CLIPBOARD (clipboard)->selection, error->message)); g_error_free (error); } + + g_output_stream_close_async (stream, + G_PRIORITY_DEFAULT, + NULL, + gdk_x11_clipboard_default_output_closed, + NULL); } static void @@ -103,8 +128,7 @@ gdk_x11_clipboard_default_output_handler (GOutputStream *stream, G_PRIORITY_DEFAULT, NULL, gdk_x11_clipboard_default_output_done, - NULL); - g_object_unref (stream); + stream); } static GInputStream * diff --git a/gdk/x11/gdkdrag-x11.c b/gdk/x11/gdkdrag-x11.c index 5804085a56..7e7904904b 100644 --- a/gdk/x11/gdkdrag-x11.c +++ b/gdk/x11/gdkdrag-x11.c @@ -1612,11 +1612,30 @@ gdk_x11_drag_set_hotspot (GdkDrag *drag, } } +static void +gdk_x11_drag_default_output_closed (GObject *stream, + GAsyncResult *result, + gpointer user_data) +{ + GError *error = NULL; + + if (!g_output_stream_close_finish (G_OUTPUT_STREAM (stream), result, &error)) + { + GDK_NOTE (DND, + g_printerr ("failed to close stream: %s\n", + error->message)); + g_error_free (error); + } + + g_object_unref (stream); +} + static void gdk_x11_drag_default_output_done (GObject *drag, GAsyncResult *result, gpointer user_data) { + GOutputStream *stream = user_data; GError *error = NULL; if (!gdk_drag_write_finish (GDK_DRAG (drag), result, &error)) @@ -1624,6 +1643,12 @@ gdk_x11_drag_default_output_done (GObject *drag, GDK_DISPLAY_NOTE (gdk_drag_get_display (GDK_DRAG (drag)), DND, g_printerr ("failed to write stream: %s\n", error->message)); g_error_free (error); } + + g_output_stream_close_async (stream, + G_PRIORITY_DEFAULT, + NULL, + gdk_x11_drag_default_output_closed, + NULL); } static void @@ -1637,8 +1662,7 @@ gdk_x11_drag_default_output_handler (GOutputStream *stream, G_PRIORITY_DEFAULT, NULL, gdk_x11_drag_default_output_done, - NULL); - g_object_unref (stream); + stream); } static gboolean From 6fc5e04d7c868b9b4efdd957d03ae5d3bbd91b12 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Mon, 13 Dec 2021 01:52:30 +0100 Subject: [PATCH 3/4] x11: Explicitly track end of stream The OutputStream needs to write a 0 byte end of stream Property. We need to track if that has been written, and we do that with that new property. We also use that property to always request flushes when the stream is being closed, so that we don't wait for another flush() call. --- gdk/x11/gdkselectionoutputstream-x11.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/gdk/x11/gdkselectionoutputstream-x11.c b/gdk/x11/gdkselectionoutputstream-x11.c index 7b45638dd3..7371f1c0f3 100644 --- a/gdk/x11/gdkselectionoutputstream-x11.c +++ b/gdk/x11/gdkselectionoutputstream-x11.c @@ -57,6 +57,7 @@ struct _GdkX11SelectionOutputStreamPrivate { GTask *pending_task; guint incr : 1; + guint sent_end_of_stream : 1; guint delete_pending : 1; /* owns a reference */ }; @@ -176,12 +177,16 @@ gdk_x11_selection_output_stream_needs_flush_unlocked (GdkX11SelectionOutputStrea { GdkX11SelectionOutputStreamPrivate *priv = gdk_x11_selection_output_stream_get_instance_private (stream); - if (priv->data->len == 0 && priv->notify == NULL) + if (priv->sent_end_of_stream) return FALSE; - if (g_output_stream_is_closing (G_OUTPUT_STREAM (stream))) + if (g_output_stream_is_closing (G_OUTPUT_STREAM (stream)) || + g_output_stream_is_closed (G_OUTPUT_STREAM (stream))) return TRUE; + if (priv->data->len == 0 && priv->notify == NULL) + return FALSE; + if (priv->flush_requested) return TRUE; @@ -284,6 +289,8 @@ gdk_x11_selection_output_stream_perform_flush (GdkX11SelectionOutputStream *stre g_byte_array_remove_range (priv->data, 0, n_elements * element_size); if (priv->data->len < element_size) priv->flush_requested = FALSE; + if (!priv->incr || n_elements == 0) + priv->sent_end_of_stream = TRUE; } if (priv->notify) From 252b9fc39cfb0b717b23d422dc05b02f7bb4088c Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Mon, 13 Dec 2021 01:54:21 +0100 Subject: [PATCH 4/4] x11: Don't delete important signal handlers randomly We finish the write to the output stream long after the stream has been closed, so we want to keep the event handler around to do just that. Instead, remove the handler on finalize. --- gdk/x11/gdkselectionoutputstream-x11.c | 64 +++----------------------- 1 file changed, 7 insertions(+), 57 deletions(-) diff --git a/gdk/x11/gdkselectionoutputstream-x11.c b/gdk/x11/gdkselectionoutputstream-x11.c index 7371f1c0f3..da77d5db82 100644 --- a/gdk/x11/gdkselectionoutputstream-x11.c +++ b/gdk/x11/gdkselectionoutputstream-x11.c @@ -502,60 +502,6 @@ gdk_x11_selection_output_stream_flush_finish (GOutputStream *stream, return g_task_propagate_boolean (G_TASK (result), error); } -static gboolean -gdk_x11_selection_output_stream_invoke_close (gpointer stream) -{ - GdkX11SelectionOutputStreamPrivate *priv = gdk_x11_selection_output_stream_get_instance_private (stream); - - GDK_X11_DISPLAY (priv->display)->streams = g_slist_remove (GDK_X11_DISPLAY (priv->display)->streams, stream); - g_signal_handlers_disconnect_by_func (priv->display, - gdk_x11_selection_output_stream_xevent, - stream); - g_object_unref (stream); - - return G_SOURCE_REMOVE; -} - -static gboolean -gdk_x11_selection_output_stream_close (GOutputStream *stream, - GCancellable *cancellable, - GError **error) -{ - g_main_context_invoke (NULL, gdk_x11_selection_output_stream_invoke_close, g_object_ref (stream)); - - return TRUE; -} - -static void -gdk_x11_selection_output_stream_close_async (GOutputStream *stream, - int io_priority, - GCancellable *cancellable, - GAsyncReadyCallback callback, - gpointer user_data) -{ - GTask *task; - - task = g_task_new (stream, cancellable, callback, user_data); - g_task_set_source_tag (task, gdk_x11_selection_output_stream_close_async); - g_task_set_priority (task, io_priority); - - gdk_x11_selection_output_stream_invoke_close (stream); - g_task_return_boolean (task, TRUE); - - g_object_unref (task); -} - -static gboolean -gdk_x11_selection_output_stream_close_finish (GOutputStream *stream, - GAsyncResult *result, - GError **error) -{ - g_return_val_if_fail (g_task_is_valid (result, stream), FALSE); - g_return_val_if_fail (g_async_result_is_tagged (result, gdk_x11_selection_output_stream_close_async), FALSE); - - return g_task_propagate_boolean (G_TASK (result), error); -} - static void gdk_x11_selection_output_stream_finalize (GObject *object) { @@ -565,6 +511,13 @@ gdk_x11_selection_output_stream_finalize (GObject *object) /* not sending a notify is terrible */ g_assert (priv->notify == NULL); + GDK_DISPLAY_NOTE (priv->display, SELECTION, g_printerr ("%s:%s: finalizing\n", + priv->selection, priv->target)); + GDK_X11_DISPLAY (priv->display)->streams = g_slist_remove (GDK_X11_DISPLAY (priv->display)->streams, stream); + g_signal_handlers_disconnect_by_func (priv->display, + gdk_x11_selection_output_stream_xevent, + stream); + g_byte_array_unref (priv->data); g_cond_clear (&priv->cond); g_mutex_clear (&priv->mutex); @@ -587,14 +540,11 @@ gdk_x11_selection_output_stream_class_init (GdkX11SelectionOutputStreamClass *kl output_stream_class->write_fn = gdk_x11_selection_output_stream_write; output_stream_class->flush = gdk_x11_selection_output_stream_flush; - output_stream_class->close_fn = gdk_x11_selection_output_stream_close; output_stream_class->write_async = gdk_x11_selection_output_stream_write_async; output_stream_class->write_finish = gdk_x11_selection_output_stream_write_finish; output_stream_class->flush_async = gdk_x11_selection_output_stream_flush_async; output_stream_class->flush_finish = gdk_x11_selection_output_stream_flush_finish; - output_stream_class->close_async = gdk_x11_selection_output_stream_close_async; - output_stream_class->close_finish = gdk_x11_selection_output_stream_close_finish; } static void