[gnome-remote-desktop] clipboard: Handle SelectionRead() operations in an async way

[Jonas Ådahl <jadahl src gnome org>]

commit 703ea94ca2cbc0091421343fa78753265aa69c9c
Author: Pascal Nowack <Pascal Nowack gmx de>
Date:   Tue Apr 27 08:52:11 2021 +0200

    clipboard: Handle SelectionRead() operations in an async way
    
    Currently, reading the mime type content happens by directly calling
    read() on a fd.
    This is problematic, as it will only work, when the other end also
    supplies the mime type content.
    This is not always the case and in such cases gnome-remote-desktop will
    freeze with 100% cpu usage in read().
    To get rid of this situation, the read() operation must happen in an
    async way.
    If a certain timeout passes, the read() operation needs to be aborted.
    
    Do this using a GTask, which runs in another thread, which then runs
    g_input_stream_read(), which listens on the read fd and on the
    cancellable fd.
    This allows g-r-d to stay responsive, even when an application does not
    supply the mime type content.
    
    When the GTask is done with the async operation, it will create a
    GSource and attach it to the thread, that created the GTask.
    Since that GSource function can also run, when the client is already
    gone, abort the current read operation with a GCancellable, when
    disposing the clipboard.
    
    If a new mime type list is advertised by the server or client, abort
    the current read operation and flush the current mime type content.
    This ensures, that the order in which all clipboard operations happen
    is kept, as the RDP clipboard, for example, does not define how pending
    FormatDataRequests are handled, when a new FormatList is advertised.
    If the read operation was successful, but the mime type content was not
    submitted yet, submit the mime type content.
    Otherwise, inform the backend about the abortion of the operation.
    For clipboard implementations that support delayed rendering of
    clipboard data (RDP), this ensures that the client is notified about
    the read result.
    In the case of RDP this means, that the FormatDataResponse with the
    fail flag is sent.
    
    When the read() operation is aborted, wait for the GTask thread
    function to complete.
    Do this using a GCond. This ensures that the read result can be
    retrieved without any race conditions.
    This also ensures that mutter won't deny any new SelectionRead()
    requests from gnome-remote-desktop, as the fd of the pipe is closed on
    gnome-remote-desktops side, before the GCond is signalled, so that
    mutter won't deny the request with the "reading in parallel" error.
    
    Fixes: https://gitlab.gnome.org/GNOME/gnome-remote-desktop/-/issues/60

 src/grd-clipboard.c | 241 ++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 214 insertions(+), 27 deletions(-)
---
diff --git a/src/grd-clipboard.c b/src/grd-clipboard.c
index cd39d2a..3c121bc 100644
--- a/src/grd-clipboard.c
+++ b/src/grd-clipboard.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2020 Pascal Nowack
+ * Copyright (C) 2020-2021 Pascal Nowack
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as
@@ -21,10 +21,26 @@
 
 #include "grd-clipboard.h"
 
-#include <glib-unix.h>
+#include <gio/gio.h>
+#include <gio/gunixinputstream.h>
 
 #include "grd-session.h"
 
+#define MAX_READ_TIME 4000
+
+typedef struct _ReadMimeTypeContentContext
+{
+  GrdClipboard *clipboard;
+  int fd;
+  GCancellable *cancellable;
+} ReadMimeTypeContentContext;
+
+typedef struct _ReadMimeTypeContentResult
+{
+  uint8_t *data;
+  uint32_t size;
+} ReadMimeTypeContentResult;
+
 typedef struct _GrdClipboardPrivate
 {
   GrdSession *session;
@@ -32,10 +48,74 @@ typedef struct _GrdClipboardPrivate
   gboolean enabled;
 
   GHashTable *client_mime_type_tables;
+
+  ReadMimeTypeContentResult *read_result;
+  GCancellable *read_cancellable;
+  unsigned int abort_read_source_id;
+  gboolean has_pending_read_operation;
+
+  GCond pending_read_cond;
+  GMutex pending_read_mutex;
 } GrdClipboardPrivate;
 
 G_DEFINE_TYPE_WITH_PRIVATE (GrdClipboard, grd_clipboard, G_TYPE_OBJECT);
 
+static void
+handle_read_result (GrdClipboard              *clipboard,
+                    ReadMimeTypeContentResult *read_result)
+{
+  GrdClipboardClass *klass = GRD_CLIPBOARD_GET_CLASS (clipboard);
+  GrdClipboardPrivate *priv = grd_clipboard_get_instance_private (clipboard);
+
+  priv->has_pending_read_operation = FALSE;
+
+  /* Discard the read_result, if the clipboard is already disabled. */
+  if (!priv->enabled)
+    return;
+
+  if (read_result->data)
+    g_debug ("Clipboard[SelectionRead]: Request successful");
+  else
+    g_debug ("Clipboard[SelectionRead]: Request failed");
+
+  klass->submit_requested_server_content (clipboard, read_result->data,
+                                          read_result->size);
+}
+
+static void
+flush_pending_read_result (GrdClipboard *clipboard)
+{
+  GrdClipboardPrivate *priv = grd_clipboard_get_instance_private (clipboard);
+  ReadMimeTypeContentResult *read_result;
+
+  g_mutex_lock (&priv->pending_read_mutex);
+  while (!priv->read_result)
+    g_cond_wait (&priv->pending_read_cond, &priv->pending_read_mutex);
+  g_mutex_unlock (&priv->pending_read_mutex);
+
+  read_result = g_steal_pointer (&priv->read_result);
+
+  handle_read_result (clipboard, read_result);
+  g_free (read_result);
+}
+
+static void
+abort_current_read_operation (GrdClipboard *clipboard)
+{
+  GrdClipboardPrivate *priv = grd_clipboard_get_instance_private (clipboard);
+
+  if (!priv->has_pending_read_operation)
+    return;
+
+  g_debug ("Clipboard[SelectionRead]: Aborting current read operation");
+  g_cancellable_cancel (priv->read_cancellable);
+
+  g_clear_object (&priv->read_cancellable);
+  g_clear_handle_id (&priv->abort_read_source_id, g_source_remove);
+
+  flush_pending_read_result (clipboard);
+}
+
 void
 grd_clipboard_update_server_mime_type_list (GrdClipboard *clipboard,
                                             GList        *mime_type_tables)
@@ -69,6 +149,8 @@ grd_clipboard_update_server_mime_type_list (GrdClipboard *clipboard,
     }
   else
     {
+      abort_current_read_operation (clipboard);
+
       if (mime_type_tables)
         grd_session_set_selection (priv->session, mime_type_tables);
     }
@@ -77,33 +159,76 @@ grd_clipboard_update_server_mime_type_list (GrdClipboard *clipboard,
   g_list_free (mime_type_tables);
 }
 
-static uint8_t *
-read_mime_type_content_sync (int       fd,
-                             uint32_t *size)
+static void
+async_read_operation_complete (GObject      *source_object,
+                               GAsyncResult *result,
+                               gpointer      user_data)
 {
-  GArray *data;
+  GrdClipboard *clipboard = user_data;
+  GrdClipboardPrivate *priv;
+  ReadMimeTypeContentContext *read_context =
+    g_task_get_task_data (G_TASK (result));
+  ReadMimeTypeContentResult *read_result;
 
-  *size = 0;
-  if (fd == -1)
-    return NULL;
+  if (g_cancellable_is_cancelled (read_context->cancellable))
+    return;
+
+  priv = grd_clipboard_get_instance_private (clipboard);
+  g_assert (priv->has_pending_read_operation);
+
+  g_clear_object (&priv->read_cancellable);
+  g_clear_handle_id (&priv->abort_read_source_id, g_source_remove);
+
+  read_result = g_steal_pointer (&priv->read_result);
+
+  handle_read_result (clipboard, read_result);
+  g_free (read_result);
+}
+
+static void
+clear_read_context (gpointer data)
+{
+  ReadMimeTypeContentContext *read_context = data;
+
+  g_object_unref (read_context->cancellable);
+
+  g_free (data);
+}
+
+static void
+read_mime_type_content_in_thread (GTask        *task,
+                                  gpointer      source_object,
+                                  gpointer      task_data,
+                                  GCancellable *cancellable)
+{
+  ReadMimeTypeContentContext *read_context = task_data;
+  GrdClipboard *clipboard = read_context->clipboard;
+  GrdClipboardPrivate *priv = grd_clipboard_get_instance_private (clipboard);
+  ReadMimeTypeContentResult *read_result;
+  GInputStream *input_stream;
+  GArray *data;
+  gboolean success = FALSE;
+  g_autoptr (GError) error = NULL;
 
+  input_stream = g_unix_input_stream_new (read_context->fd, TRUE);
   data = g_array_new (FALSE, TRUE, sizeof (uint8_t));
+
   while (TRUE)
     {
       int len;
       uint8_t buffer[1024];
 
-      len = read (fd, buffer, G_N_ELEMENTS (buffer));
+      len = g_input_stream_read (input_stream, buffer, G_N_ELEMENTS (buffer),
+                                 read_context->cancellable, &error);
       if (len < 0)
         {
-          if (errno == EAGAIN)
-            continue;
-
-          g_warning ("read() failed: %s", g_strerror (errno));
+          g_warning ("Clipboard[SelectionRead]: Failed to read mime type "
+                     "content: %s", error->message);
           break;
         }
       else if (len == 0)
         {
+          success = TRUE;
           break;
         }
       else
@@ -111,17 +236,69 @@ read_mime_type_content_sync (int       fd,
           g_array_append_vals (data, buffer, len);
         }
     }
-  close (fd);
 
-  if (data->len <= 0)
+  read_result = g_malloc0 (sizeof (ReadMimeTypeContentResult));
+  if (success && data->len > 0)
+    {
+      read_result->size = data->len;
+      read_result->data = (uint8_t *) g_array_free (data, FALSE);
+    }
+  else
     {
       g_array_free (data, TRUE);
-      return NULL;
     }
 
-  *size = data->len;
+  g_object_unref (input_stream);
+
+  g_mutex_lock (&priv->pending_read_mutex);
+  priv->read_result = read_result;
+  g_cond_signal (&priv->pending_read_cond);
+  g_mutex_unlock (&priv->pending_read_mutex);
+}
+
+static gboolean
+abort_mime_type_content_read (gpointer user_data)
+{
+  GrdClipboard *clipboard = user_data;
+  GrdClipboardPrivate *priv = grd_clipboard_get_instance_private (clipboard);
+
+  g_debug ("Clipboard[SelectionRead]: Aborting current read operation "
+           "(Timeout reached)");
+
+  g_assert (priv->has_pending_read_operation);
+  g_assert (priv->abort_read_source_id);
 
-  return (uint8_t *) g_array_free (data, FALSE);
+  priv->abort_read_source_id = 0;
+  abort_current_read_operation (clipboard);
+
+  return G_SOURCE_REMOVE;
+}
+
+static void
+read_mime_type_content_async (GrdClipboard *clipboard,
+                              int           fd)
+{
+  GrdClipboardPrivate *priv = grd_clipboard_get_instance_private (clipboard);
+  ReadMimeTypeContentContext *read_context;
+  GTask *task;
+
+  abort_current_read_operation (clipboard);
+  priv->read_cancellable = g_cancellable_new ();
+  priv->has_pending_read_operation = TRUE;
+  g_assert (!priv->read_result);
+
+  read_context = g_malloc0 (sizeof (ReadMimeTypeContentContext));
+  read_context->clipboard = clipboard;
+  read_context->fd = fd;
+  read_context->cancellable = g_object_ref (priv->read_cancellable);
+
+  task = g_task_new (NULL, NULL, async_read_operation_complete, clipboard);
+  g_task_set_task_data (task, read_context, clear_read_context);
+  g_task_run_in_thread (task, read_mime_type_content_in_thread);
+  g_object_unref (task);
+
+  priv->abort_read_source_id =
+    g_timeout_add (MAX_READ_TIME, abort_mime_type_content_read, clipboard);
 }
 
 void
@@ -131,8 +308,6 @@ grd_clipboard_request_server_content_for_mime_type_async (GrdClipboard *clipboar
   GrdClipboardClass *klass = GRD_CLIPBOARD_GET_CLASS (clipboard);
   GrdClipboardPrivate *priv = grd_clipboard_get_instance_private (clipboard);
   int fd;
-  uint8_t *data;
-  uint32_t size;
 
   g_return_if_fail (klass->submit_requested_server_content);
 
@@ -142,14 +317,15 @@ grd_clipboard_request_server_content_for_mime_type_async (GrdClipboard *clipboar
   g_debug ("Clipboard[SelectionRead]: Requesting data from servers clipboard"
            " (mime type: %s)", grd_mime_type_to_string (mime_type));
   fd = grd_session_selection_read (priv->session, mime_type);
+  if (fd == -1)
+    {
+      g_debug ("Clipboard[SelectionRead]: Request failed");
+      klass->submit_requested_server_content (clipboard, NULL, 0);
 
-  data = read_mime_type_content_sync (fd, &size);
-  if (data)
-    g_debug ("Clipboard[SelectionRead]: Request successful");
-  else
-    g_debug ("Clipboard[SelectionRead]: Request failed");
+      return;
+    }
 
-  klass->submit_requested_server_content (clipboard, data, size);
+  read_mime_type_content_async (clipboard, fd);
 }
 
 void
@@ -201,6 +377,12 @@ grd_clipboard_update_client_mime_type_list (GrdClipboard *clipboard,
   GrdClipboardPrivate *priv = grd_clipboard_get_instance_private (clipboard);
   GList *l;
 
+  /**
+   * Ensure that the response with the read mime type content is sent to the
+   * client first, before sending the new mime type list
+   */
+  abort_current_read_operation (clipboard);
+
   if (!klass->update_client_mime_type_list)
     return;
 
@@ -258,6 +440,8 @@ grd_clipboard_dispose (GObject *object)
   GrdClipboard *clipboard = GRD_CLIPBOARD (object);
   GrdClipboardPrivate *priv = grd_clipboard_get_instance_private (clipboard);
 
+  abort_current_read_operation (clipboard);
+
   g_clear_pointer (&priv->client_mime_type_tables, g_hash_table_destroy);
 
   G_OBJECT_CLASS (grd_clipboard_parent_class)->dispose (object);
@@ -270,6 +454,9 @@ grd_clipboard_init (GrdClipboard *clipboard)
 
   priv->client_mime_type_tables = g_hash_table_new_full (NULL, NULL, NULL,
                                                          free_mime_type_table);
+
+  g_cond_init (&priv->pending_read_cond);
+  g_mutex_init (&priv->pending_read_mutex);
 }
 
 static void