From 468d2cb7f22b790cae22e5110bc232085ebf1661 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Tue, 30 May 2023 17:42:59 +0200 Subject: [PATCH 1/5] gtkselectioninputstream-x11: Do not add EOF marker twice to the chunks queue We were adding the same EOF marker two times back to the chunks queue, one implicitly, and the other time could happen when exiting the loop. --- gdk/x11/gdkselectioninputstream-x11.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gdk/x11/gdkselectioninputstream-x11.c b/gdk/x11/gdkselectioninputstream-x11.c index e6bc574409..7ea92ef368 100644 --- a/gdk/x11/gdkselectioninputstream-x11.c +++ b/gdk/x11/gdkselectioninputstream-x11.c @@ -89,7 +89,7 @@ gdk_x11_selection_input_stream_fill_buffer (GdkX11SelectionInputStream *stream, if (size == 0) { /* EOF marker, put it back */ - g_async_queue_push_front_unlocked (priv->chunks, bytes); + g_async_queue_push_front_unlocked (priv->chunks, g_steal_pointer (&bytes)); break; } else if (size > count) From bce1e0bfddd960a39d8041a19ab5085ca6f980be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Tue, 30 May 2023 17:43:57 +0200 Subject: [PATCH 2/5] gtkselectioninputstream-x11: Do not add unreffed bytes to the chunks queue This should never happen, but we may exit the loop because of count value with an unreffed bytes pointer being added back to the chunks queue. --- gdk/x11/gdkselectioninputstream-x11.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gdk/x11/gdkselectioninputstream-x11.c b/gdk/x11/gdkselectioninputstream-x11.c index 7ea92ef368..0f4ce37d4c 100644 --- a/gdk/x11/gdkselectioninputstream-x11.c +++ b/gdk/x11/gdkselectioninputstream-x11.c @@ -107,7 +107,7 @@ gdk_x11_selection_input_stream_fill_buffer (GdkX11SelectionInputStream *stream, memcpy (buffer, g_bytes_get_data (bytes, NULL), size); } - g_bytes_unref (bytes); + g_bytes_unref (g_steal_pointer (&bytes)); result += size; if (buffer) buffer += size; From 371e860184c95b9d6ec77369d5d6266072f32010 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Tue, 30 May 2023 17:50:56 +0200 Subject: [PATCH 3/5] gtkselectioninputstream-x11: Do not add an extra reference to the returned stream We create a new stream during gdk_x11_selection_input_stream_new_async() then such stream is referenced when passed to the task via g_task_return_pointer(), so there's no need to reference it again before returning it, or we'd end up leaking. Fixes: https://gitlab.gnome.org/GNOME/gtk/-/issues/4892 --- gdk/x11/gdkselectioninputstream-x11.c | 1 - 1 file changed, 1 deletion(-) diff --git a/gdk/x11/gdkselectioninputstream-x11.c b/gdk/x11/gdkselectioninputstream-x11.c index 0f4ce37d4c..5d480cf6ab 100644 --- a/gdk/x11/gdkselectioninputstream-x11.c +++ b/gdk/x11/gdkselectioninputstream-x11.c @@ -577,7 +577,6 @@ gdk_x11_selection_input_stream_new_finish (GAsyncResult *result, *type = priv->type; if (format) *format = priv->format; - g_object_ref (stream); } return G_INPUT_STREAM (stream); From 4fcf899852373caadd898b7f7e8039c12815aca2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Tue, 30 May 2023 17:54:00 +0200 Subject: [PATCH 4/5] gdkselectioninputstream-x11: Make it clearer how we manage the stream ownership It gets unreffed during gdk_x11_selection_input_stream_complete, so use APIs that make this clearer. --- gdk/x11/gdkselectioninputstream-x11.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/gdk/x11/gdkselectioninputstream-x11.c b/gdk/x11/gdkselectioninputstream-x11.c index 5d480cf6ab..e62bd96035 100644 --- a/gdk/x11/gdkselectioninputstream-x11.c +++ b/gdk/x11/gdkselectioninputstream-x11.c @@ -416,7 +416,7 @@ gdk_x11_selection_input_stream_xevent (GdkDisplay *display, "%s:%s: got PropertyNotify erroring out of INCR", priv->selection, priv->target); /* error, should we signal one? */ - gdk_x11_selection_input_stream_complete (stream); + g_clear_pointer (&stream, gdk_x11_selection_input_stream_complete); } else if (g_bytes_get_size (bytes) == 0 || type == None) { @@ -424,7 +424,7 @@ gdk_x11_selection_input_stream_xevent (GdkDisplay *display, "%s:%s: got PropertyNotify ending INCR", priv->selection, priv->target); g_bytes_unref (bytes); - gdk_x11_selection_input_stream_complete (stream); + g_clear_pointer (&stream, gdk_x11_selection_input_stream_complete); } else { @@ -467,7 +467,7 @@ gdk_x11_selection_input_stream_xevent (GdkDisplay *display, G_IO_ERROR, G_IO_ERROR_NOT_FOUND, _("Format %s not supported"), priv->target); - gdk_x11_selection_input_stream_complete (stream); + g_clear_pointer (&stream, gdk_x11_selection_input_stream_complete); } else { @@ -478,7 +478,7 @@ gdk_x11_selection_input_stream_xevent (GdkDisplay *display, if (bytes == NULL) { - gdk_x11_selection_input_stream_complete (stream); + g_clear_pointer (&stream, gdk_x11_selection_input_stream_complete); } else { @@ -500,7 +500,7 @@ gdk_x11_selection_input_stream_xevent (GdkDisplay *display, g_bytes_get_size (bytes)); g_async_queue_push (priv->chunks, bytes); - gdk_x11_selection_input_stream_complete (stream); + g_clear_pointer (&stream, gdk_x11_selection_input_stream_complete); } } From be4f6ff3dad7adce085a95ca652dda31a20a00d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Tue, 30 May 2023 17:59:06 +0200 Subject: [PATCH 5/5] gdkselectioninputstream-x11: Explicitly handle stream ownership in signal The display xevent signal connection takes the ownership of the stream until we get a valid event, so it should manage the stream lifetime. So make this clearer, by automatically removing the stream reference when we disconnect from the xevent signal handler. --- gdk/x11/gdkselectioninputstream-x11.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/gdk/x11/gdkselectioninputstream-x11.c b/gdk/x11/gdkselectioninputstream-x11.c index e62bd96035..6682639f44 100644 --- a/gdk/x11/gdkselectioninputstream-x11.c +++ b/gdk/x11/gdkselectioninputstream-x11.c @@ -165,9 +165,7 @@ gdk_x11_selection_input_stream_complete (GdkX11SelectionInputStream *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_input_stream_xevent, - stream); - - g_object_unref (stream); + g_steal_pointer (&stream)); } static gssize @@ -541,7 +539,10 @@ gdk_x11_selection_input_stream_new_async (GdkDisplay *display, priv->property = g_strdup_printf ("GDK_SELECTION_%p", stream); priv->xproperty = gdk_x11_get_xatom_by_name_for_display (display, priv->property); - g_signal_connect (display, "xevent", G_CALLBACK (gdk_x11_selection_input_stream_xevent), stream); + g_signal_connect_data (display, "xevent", + G_CALLBACK (gdk_x11_selection_input_stream_xevent), + g_steal_pointer (&stream), + (GClosureNotify) g_object_unref, 0); XConvertSelection (GDK_DISPLAY_XDISPLAY (display), priv->xselection,