In response to [gemu-gtk-clipboard freezing and crashing guests](https://gitlab.com/qemu-project/qemu/-/issues/1150).

I think I might have a solution for the gtk clipboard sometimes crashing guests.

@kolAflash I couldn't have done it without you, figuring out `gtk_clipboard_wait_is_text_available(clipboard)` was the issue is half the work.

The real issue is that it's blocking and I'd wager that's a big no-no since qemu & KVM have to run the VM + OS, preferably as real-time as possible. Something times out and you get a core dump.

So as a replacement, `gtk_clipboard_request_text`, which is async and non-blocking is a better choice, hopefully.
It requires an additional function to handle receiving text.

Signed-off-by: Edmund Raile <edmund.raile@proton.me>
From 530db8b6c7adc99f540d7d8cc6122320868326e6 Mon Sep 17 00:00:00 2001
From: Edmund Raile <edmund.raile@proton.me>
Date: Sun, 24 Sep 2023 09:46:27 +0200
Subject: [PATCH 1/1] qemu-ui-gtk clipboard possible fix for crashes

---
ui/gtk-clipboard.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/ui/gtk-clipboard.c b/ui/gtk-clipboard.c
index 8d8a636fd1..64d4f7ac9d 100644
--- a/ui/gtk-clipboard.c
+++ b/ui/gtk-clipboard.c
@@ -153,6 +153,18 @@ static void gd_clipboard_request(QemuClipboardInfo *info,
}
}

+/* non-blocking clipboard receiver implementation */
+static void gd_clipboard_text_received_callback(GtkClipboard *clipboard, const gchar *text, gpointer data)
+{
+ QemuClipboardInfo *info = (QemuClipboardInfo *)data;
+ if (text) {
+ info->types[QEMU_CLIPBOARD_TYPE_TEXT].available = true;
+ }
+
+ qemu_clipboard_update(info);
+ qemu_clipboard_info_unref(info);
+}
+
static void gd_owner_change(GtkClipboard *clipboard,
GdkEvent *event,
gpointer data)
@@ -170,12 +182,8 @@ static void gd_owner_change(GtkClipboard *clipboard,
switch (event->owner_change.reason) {
case GDK_OWNER_CHANGE_NEW_OWNER:
info = qemu_clipboard_info_new(&gd->cbpeer, s);
- if (gtk_clipboard_wait_is_text_available(clipboard)) {
- info->types[QEMU_CLIPBOARD_TYPE_TEXT].available = true;
- }
-
- qemu_clipboard_update(info);
- qemu_clipboard_info_unref(info);
+ /* gtk_clipboard_wait_is_text_available (blocking) was used here previously and crashed guests */
+ gtk_clipboard_request_text(clipboard, gd_clipboard_text_received_callback, info);
break;
default:
qemu_clipboard_peer_release(&gd->cbpeer, s);
--
2.42.0

don't forget to configure with --enable-gtk-clipboard before building

I'd say my gvt-g win10 VM has become a lot more responsive (was using gtk-clipboard besides being broken).
Paste from the VM is a bit delayed sometimes but I can live with that.
So far my VM hasn't crashed yet.

I'd like to ask you for help in evaluating my patch.
The issue linked to in the first line has instructions on the crash case.

It's my first time on the mailing list, I hope I've done this right.

Mr. Lureau CCed here had this to add:
Blocking the signal handler isn't great either, as we may miss clipboard updates. I think we could "reuse" the serial field on info and check in the callback if we don't have the latest, just ignore the result and free.

Hi

On Mon, Sep 25, 2023 at 7:08 PM edmund.raile
<edmund.raile@protonmail.com> wrote:
>
> In response to [gemu-gtk-clipboard freezing and crashing guests](https://gitlab.com/qemu-project/qemu/-/issues/1150).
>
> I think I might have a solution for the gtk clipboard sometimes crashing guests.
>
> @kolAflash I couldn't have done it without you, figuring out `gtk_clipboard_wait_is_text_available(clipboard)` was the issue is half the work.
>
> The real issue is that it's blocking and I'd wager that's a big no-no since qemu & KVM have to run the VM + OS, preferably as real-time as possible. Something times out and you get a core dump.
>
> So as a replacement, `gtk_clipboard_request_text`, which is async and non-blocking is a better choice, hopefully.
> It requires an additional function to handle receiving text.
>
> Signed-off-by: Edmund Raile <edmund.raile@proton.me>
> From 530db8b6c7adc99f540d7d8cc6122320868326e6 Mon Sep 17 00:00:00 2001
> From: Edmund Raile <edmund.raile@proton.me>
> Date: Sun, 24 Sep 2023 09:46:27 +0200
> Subject: [PATCH 1/1] qemu-ui-gtk clipboard possible fix for crashes

Thanks for the patch, but this is not a proper way to send patches on the ML.
See: https://www.qemu.org/docs/master/devel/submitting-a-patch.html#submitting-your-patches

>
> ---
>  ui/gtk-clipboard.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/ui/gtk-clipboard.c b/ui/gtk-clipboard.c
> index 8d8a636fd1..64d4f7ac9d 100644
> --- a/ui/gtk-clipboard.c
> +++ b/ui/gtk-clipboard.c
> @@ -153,6 +153,18 @@ static void gd_clipboard_request(QemuClipboardInfo *info,
>      }
>  }
>
> +/* non-blocking clipboard receiver implementation */
> +static void gd_clipboard_text_received_callback(GtkClipboard *clipboard, const gchar *text, gpointer data)
> +{
> +    QemuClipboardInfo *info = (QemuClipboardInfo *)data;
> +    if (text) {
> +        info->types[QEMU_CLIPBOARD_TYPE_TEXT].available = true;
> +    }
> +
> +    qemu_clipboard_update(info);
> +    qemu_clipboard_info_unref(info);
> +}
> +
>  static void gd_owner_change(GtkClipboard *clipboard,
>                              GdkEvent *event,
>                              gpointer data)
> @@ -170,12 +182,8 @@ static void gd_owner_change(GtkClipboard *clipboard,
>      switch (event->owner_change.reason) {
>      case GDK_OWNER_CHANGE_NEW_OWNER:
>          info = qemu_clipboard_info_new(&gd->cbpeer, s);
> -        if (gtk_clipboard_wait_is_text_available(clipboard)) {
> -            info->types[QEMU_CLIPBOARD_TYPE_TEXT].available = true;
> -        }
> -
> -        qemu_clipboard_update(info);
> -        qemu_clipboard_info_unref(info);
> +        /* gtk_clipboard_wait_is_text_available (blocking) was used here previously and crashed guests */
> +        gtk_clipboard_request_text(clipboard, gd_clipboard_text_received_callback, info);

Although this prevents the main loop from blocking immediately in
QEMU, it's unclear why GTK would block in the given issue.

Iow, it may very well be that something else is broken at GTK level,
and we never get the callback either.

Also, there is another "blocking" call in gd_clipboard_request() which
could have the same issue easily and should probably be addressed with
the same patch.


>          break;
>      default:
>          qemu_clipboard_peer_release(&gd->cbpeer, s);
> --
> 2.42.0
>
> don't forget to configure with --enable-gtk-clipboard before building
>
> I'd say my gvt-g win10 VM has become a lot more responsive (was using gtk-clipboard besides being broken).
> Paste from the VM is a bit delayed sometimes but I can live with that.
> So far my VM hasn't crashed yet.
>
> I'd like to ask you for help in evaluating my patch.
> The issue linked to in the first line has instructions on the crash case.
>
> It's my first time on the mailing list, I hope I've done this right.
>
> Mr. Lureau CCed here had this to add:
> Blocking the signal handler isn't great either, as we may miss clipboard updates. I think we could "reuse" the serial field on info and check in the callback if we don't have the latest, just ignore the result and free.

If you don't intend to use the "serial" approach to discard previous
requests, we should mention in the patch that it assumes Gtk will keep
the requests in order.

-- 
Marc-André Lureau