[Spice-commits] 5 commits - meson.build src/channel-main.c src/spice-gtk-session.c

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Tue Mar 10 15:55:24 UTC 2020


 meson.build             |    2 
 src/channel-main.c      |   36 ++++++++++++++-
 src/spice-gtk-session.c |  114 +++++++++++++++++++++++++++++++++++++++---------
 3 files changed, 128 insertions(+), 24 deletions(-)

New commits:
commit 039f03bbd0f85cbc843ee52a9ee2e26e199b2417
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date:   Fri Mar 22 14:44:00 2019 +0100

    clipboard: implement CAP_CLIPBOARD_GRAB_SERIAL
    
    Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>

diff --git a/src/channel-main.c b/src/channel-main.c
index aa14a9c..1e85a36 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -111,6 +111,7 @@ struct _SpiceMainChannelPrivate  {
     guint                       migrate_delayed_id;
     spice_migrate               *migrate_data;
     int                         max_clipboard;
+    uint32_t                    clipboard_serial[G_MAXUINT8+1];
 
     gboolean                    agent_volume_playback_sync;
     gboolean                    agent_volume_record_sync;
@@ -223,6 +224,7 @@ static const char *agent_caps[] = {
     [ VD_AGENT_CAP_MONITORS_CONFIG_POSITION ] = "monitors config position",
     [ VD_AGENT_CAP_FILE_XFER_DISABLED ] = "file transfer disabled",
     [ VD_AGENT_CAP_CLIPBOARD_NO_RELEASE_ON_REGRAB ] = "no release on re-grab",
+    [ VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL ] = "clipboard grab serial",
 };
 #define NAME(_a, _i) ((_i) < SPICE_N_ELEMENTS(_a) ? (_a[(_i)] ?: "?") : "?")
 
@@ -412,6 +414,7 @@ static void spice_main_channel_reset_agent(SpiceMainChannel *channel)
 
     spice_main_channel_reset_all_xfer_operations(channel);
     file_xfer_flushed(channel, FALSE);
+    memset(c->clipboard_serial, 0, sizeof(c->clipboard_serial));
 }
 
 /* main or coroutine context */
@@ -1335,6 +1338,7 @@ static void agent_announce_caps(SpiceMainChannel *channel)
     VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_MONITORS_CONFIG_POSITION);
     VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_FILE_XFER_DETAILED_ERRORS);
     VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_CLIPBOARD_NO_RELEASE_ON_REGRAB);
+    VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL);
 
     agent_msg_queue(channel, VD_AGENT_ANNOUNCE_CAPABILITIES, size, caps);
     g_free(caps);
@@ -1365,14 +1369,23 @@ static void agent_clipboard_grab(SpiceMainChannel *channel, guint selection,
         return;
     }
 
+    if (test_agent_cap(channel, VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL)) {
+        size += sizeof(uint32_t);
+    }
+
     msg = g_alloca(size);
     memset(msg, 0, size);
 
     grab = (VDAgentClipboardGrab *)msg;
 
     if (test_agent_cap(channel, VD_AGENT_CAP_CLIPBOARD_SELECTION)) {
-        msg[0] = selection;
-        grab = (VDAgentClipboardGrab *)(msg + 4);
+        *(uint8_t *)grab = selection;
+        grab = (void *)grab + sizeof(uint32_t);
+    }
+
+    if (test_agent_cap(channel, VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL)) {
+        *(uint32_t *)grab = GUINT32_TO_LE(c->clipboard_serial[selection]++);
+        grab = (void *)grab + sizeof(uint32_t);
     }
 
     for (i = 0; i < ntypes; i++) {
@@ -1974,6 +1987,7 @@ static void main_agent_handle_msg(SpiceChannel *channel,
     SpiceMainChannel *self = SPICE_MAIN_CHANNEL(channel);
     SpiceMainChannelPrivate *c = self->priv;
     guint8 selection = VD_AGENT_CLIPBOARD_SELECTION_CLIPBOARD;
+    guint32 serial;
 
     g_return_if_fail(msg->protocol == VD_AGENT_PROTOCOL);
 
@@ -2045,6 +2059,21 @@ static void main_agent_handle_msg(SpiceChannel *channel,
     case VD_AGENT_CLIPBOARD_GRAB:
     {
         gboolean ret;
+
+        if (test_agent_cap(self, VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL)) {
+            serial = GUINT32_FROM_LE(*((guint32 *)payload));
+            payload = ((guint8 *)payload) + sizeof(uint32_t);
+            msg->size -= sizeof(uint32_t);
+
+            if (serial == c->clipboard_serial[selection]) {
+                c->clipboard_serial[selection]++;
+            } else {
+                CHANNEL_DEBUG(channel, "grab discard, serial:%u != c->serial:%u",
+                              serial, c->clipboard_serial[selection]);
+                break;
+            }
+        }
+
         g_coroutine_signal_emit(self, signals[SPICE_MAIN_CLIPBOARD_SELECTION_GRAB], 0, selection,
                           (guint8*)payload, msg->size / sizeof(uint32_t), &ret);
         if (selection == VD_AGENT_CLIPBOARD_SELECTION_CLIPBOARD)
diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
index ce79b51..d17bb9c 100644
--- a/src/spice-gtk-session.c
+++ b/src/spice-gtk-session.c
@@ -1117,7 +1117,6 @@ static gboolean clipboard_release_timeout(gpointer user_data)
  *
  * Workaround this problem by delaying the release event by 0.5 sec,
  * unless the no-release-on-regrab capability is present.
- * FIXME: protocol change to solve the conflict and set client priority.
  */
 #define CLIPBOARD_RELEASE_DELAY 500 /* ms */
 
commit d46ddd21b4ae5c0d4f40fb29be14382b67acc6b1
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date:   Fri Mar 22 14:36:52 2019 +0100

    clipboard: pre-condition on selection value <= 255
    
    The protocol uses a u8 for the selection value. Make sure the given
    argument value fits there, or throw a critical.
    
    The other places seem to use u8 variables already.
    
    Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>

diff --git a/src/channel-main.c b/src/channel-main.c
index 25bcf77..aa14a9c 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -1354,6 +1354,7 @@ static void agent_clipboard_grab(SpiceMainChannel *channel, guint selection,
     if (!c->agent_connected)
         return;
 
+    g_return_if_fail(selection <= G_MAXUINT8);
     g_return_if_fail(test_agent_cap(channel, VD_AGENT_CAP_CLIPBOARD_BY_DEMAND));
 
     size = sizeof(VDAgentClipboardGrab) + sizeof(uint32_t) * ntypes;
commit a723bf67c5eae9db1e7ed224bb4152071d4b47ca
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date:   Thu Mar 21 17:30:19 2019 +0100

    clipboard: do not delay release if agent has "no release on regrab"
    
    Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>

diff --git a/meson.build b/meson.build
index 6b0e71a..995268b 100644
--- a/meson.build
+++ b/meson.build
@@ -81,7 +81,7 @@ endforeach
 #
 # check for mandatory dependencies
 #
-spice_protocol_version='>= 0.12.15'
+spice_protocol_version='>= 0.14.1'
 
 glib_version = '2.46'
 glib_version_info = '>= @0@'.format(glib_version)
diff --git a/src/channel-main.c b/src/channel-main.c
index f81cecb..25bcf77 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -222,6 +222,7 @@ static const char *agent_caps[] = {
     [ VD_AGENT_CAP_AUDIO_VOLUME_SYNC   ] = "volume-sync",
     [ VD_AGENT_CAP_MONITORS_CONFIG_POSITION ] = "monitors config position",
     [ VD_AGENT_CAP_FILE_XFER_DISABLED ] = "file transfer disabled",
+    [ VD_AGENT_CAP_CLIPBOARD_NO_RELEASE_ON_REGRAB ] = "no release on re-grab",
 };
 #define NAME(_a, _i) ((_i) < SPICE_N_ELEMENTS(_a) ? (_a[(_i)] ?: "?") : "?")
 
@@ -1333,6 +1334,7 @@ static void agent_announce_caps(SpiceMainChannel *channel)
     VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_CLIPBOARD_SELECTION);
     VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_MONITORS_CONFIG_POSITION);
     VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_FILE_XFER_DETAILED_ERRORS);
+    VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_CLIPBOARD_NO_RELEASE_ON_REGRAB);
 
     agent_msg_queue(channel, VD_AGENT_ANNOUNCE_CAPABILITIES, size, caps);
     g_free(caps);
diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
index ebf87ba..ce79b51 100644
--- a/src/spice-gtk-session.c
+++ b/src/spice-gtk-session.c
@@ -1115,7 +1115,8 @@ static gboolean clipboard_release_timeout(gpointer user_data)
  * sides, client and remote, racing for the clipboard grab, and
  * believing each other is the owner.
  *
- * Workaround this problem by delaying the release event by 0.5 sec.
+ * Workaround this problem by delaying the release event by 0.5 sec,
+ * unless the no-release-on-regrab capability is present.
  * FIXME: protocol change to solve the conflict and set client priority.
  */
 #define CLIPBOARD_RELEASE_DELAY 500 /* ms */
@@ -1134,6 +1135,12 @@ static void clipboard_release_delay(SpiceMainChannel *main, guint selection,
 
     clipboard_release_delay_remove(self, selection, true);
 
+    if (spice_main_channel_agent_test_capability(s->main,
+                                                 VD_AGENT_CAP_CLIPBOARD_NO_RELEASE_ON_REGRAB)) {
+        clipboard_release(self, selection);
+        return;
+    }
+
     rel = g_new0(SpiceGtkClipboardRelease, 1);
     rel->self = self;
     rel->selection = selection;
commit 2b6ffaa369e994d358b36eda7decc03ea87276d0
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date:   Thu Mar 21 12:59:43 2019 +0100

    clipboard: do not release between remote grabs
    
    Delay the release events for 0.5 sec. If no further grab comes in,
    then release the grab. Otherwise, let's skip the release. This avoids
    some races with clipboard managers.
    
    Related to:
    https://gitlab.freedesktop.org/spice/spice-gtk/issues/82
    
    Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>

diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
index cb00fa5..ebf87ba 100644
--- a/src/spice-gtk-session.c
+++ b/src/spice-gtk-session.c
@@ -59,6 +59,7 @@ struct _SpiceGtkSessionPrivate {
     gboolean                clip_hasdata[CLIPBOARD_LAST];
     gboolean                clip_grabbed[CLIPBOARD_LAST];
     gboolean                clipboard_by_guest[CLIPBOARD_LAST];
+    guint                   clipboard_release_delay[CLIPBOARD_LAST];
     /* auto-usbredir related */
     gboolean                auto_usbredir_enable;
     int                     auto_usbredir_reqs;
@@ -95,6 +96,7 @@ struct _SpiceGtkSessionPrivate {
 
 /* ------------------------------------------------------------------ */
 /* Prototypes for private functions */
+static void clipboard_release(SpiceGtkSession *self, guint selection);
 static void clipboard_owner_change(GtkClipboard *clipboard,
                                    GdkEventOwnerChange *event,
                                    gpointer user_data);
@@ -255,6 +257,24 @@ static void spice_gtk_session_dispose(GObject *gobject)
         G_OBJECT_CLASS(spice_gtk_session_parent_class)->dispose(gobject);
 }
 
+static void clipboard_release_delay_remove(SpiceGtkSession *self, guint selection,
+                                           gboolean release_if_delayed)
+{
+    SpiceGtkSessionPrivate *s = self->priv;
+
+    if (!s->clipboard_release_delay[selection]) {
+        return;
+    }
+
+    if (release_if_delayed) {
+        SPICE_DEBUG("delayed clipboard release, sel:%u", selection);
+        clipboard_release(self, selection);
+    }
+
+    g_source_remove(s->clipboard_release_delay[selection]);
+    s->clipboard_release_delay[selection] = 0;
+}
+
 static void spice_gtk_session_finalize(GObject *gobject)
 {
     SpiceGtkSession *self = SPICE_GTK_SESSION(gobject);
@@ -264,6 +284,7 @@ static void spice_gtk_session_finalize(GObject *gobject)
     /* release stuff */
     for (i = 0; i < CLIPBOARD_LAST; ++i) {
         g_clear_pointer(&s->clip_targets[i], g_free);
+        clipboard_release_delay_remove(self, i, true);
     }
 
     /* Chain up to the parent class */
@@ -762,6 +783,11 @@ static void clipboard_get(GtkClipboard *clipboard,
     g_return_if_fail(info < SPICE_N_ELEMENTS(atom2agent));
     g_return_if_fail(s->main != NULL);
 
+    if (s->clipboard_release_delay[selection]) {
+        SPICE_DEBUG("not requesting data from guest during delayed release");
+        return;
+    }
+
     ri.selection_data = selection_data;
     ri.info = info;
     ri.loop = g_main_loop_new(NULL, FALSE);
@@ -823,6 +849,8 @@ static gboolean clipboard_grab(SpiceMainChannel *main, guint selection,
     int m, n;
     int num_targets = 0;
 
+    clipboard_release_delay_remove(self, selection, false);
+
     cb = get_clipboard_from_selection(s, selection);
     g_return_val_if_fail(cb != NULL, FALSE);
 
@@ -1052,17 +1080,12 @@ static gboolean clipboard_request(SpiceMainChannel *main, guint selection,
     return TRUE;
 }
 
-static void clipboard_release(SpiceMainChannel *main, guint selection,
-                              gpointer user_data)
+static void clipboard_release(SpiceGtkSession *self, guint selection)
 {
-    g_return_if_fail(SPICE_IS_GTK_SESSION(user_data));
-
-    SpiceGtkSession *self = user_data;
     SpiceGtkSessionPrivate *s = self->priv;
     GtkClipboard* clipboard = get_clipboard_from_selection(s, selection);
 
-    if (!clipboard)
-        return;
+    g_return_if_fail(clipboard != NULL);
 
     s->nclip_targets[selection] = 0;
 
@@ -1072,6 +1095,54 @@ static void clipboard_release(SpiceMainChannel *main, guint selection,
     s->clipboard_by_guest[selection] = FALSE;
 }
 
+typedef struct SpiceGtkClipboardRelease {
+    SpiceGtkSession *self;
+    guint selection;
+} SpiceGtkClipboardRelease;
+
+static gboolean clipboard_release_timeout(gpointer user_data)
+{
+    SpiceGtkClipboardRelease *rel = user_data;
+
+    clipboard_release_delay_remove(rel->self, rel->selection, true);
+
+    return G_SOURCE_REMOVE;
+}
+
+/*
+ * The agents send release between two grabs. This may trigger
+ * clipboard managers trying to grab the clipboard. We end up with two
+ * sides, client and remote, racing for the clipboard grab, and
+ * believing each other is the owner.
+ *
+ * Workaround this problem by delaying the release event by 0.5 sec.
+ * FIXME: protocol change to solve the conflict and set client priority.
+ */
+#define CLIPBOARD_RELEASE_DELAY 500 /* ms */
+
+static void clipboard_release_delay(SpiceMainChannel *main, guint selection,
+                                    gpointer user_data)
+{
+    SpiceGtkSession *self = SPICE_GTK_SESSION(user_data);
+    SpiceGtkSessionPrivate *s = self->priv;
+    GtkClipboard* clipboard = get_clipboard_from_selection(s, selection);
+    SpiceGtkClipboardRelease *rel;
+
+    if (!clipboard) {
+        return;
+    }
+
+    clipboard_release_delay_remove(self, selection, true);
+
+    rel = g_new0(SpiceGtkClipboardRelease, 1);
+    rel->self = self;
+    rel->selection = selection;
+    s->clipboard_release_delay[selection] =
+        g_timeout_add_full(G_PRIORITY_DEFAULT, CLIPBOARD_RELEASE_DELAY,
+                           clipboard_release_timeout, rel, g_free);
+
+}
+
 static void channel_new(SpiceSession *session, SpiceChannel *channel,
                         gpointer user_data)
 {
@@ -1088,7 +1159,7 @@ static void channel_new(SpiceSession *session, SpiceChannel *channel,
         g_signal_connect(channel, "main-clipboard-selection-request",
                          G_CALLBACK(clipboard_request), self);
         g_signal_connect(channel, "main-clipboard-selection-release",
-                         G_CALLBACK(clipboard_release), self);
+                         G_CALLBACK(clipboard_release_delay), self);
     }
     if (SPICE_IS_INPUTS_CHANNEL(channel)) {
         spice_g_signal_connect_object(channel, "inputs-modifiers",
commit 2a75ddb97dc22b81becd46bdc281ee6b0a0b054d
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date:   Wed Mar 20 19:09:09 2019 +0100

    clipboard: do not release between client grabs
    
    On the client side, whenever the grab owner changes (and the clipboard
    was previously grabbed), spice-gtk sends a clipboard release followed
    immediately by a new grab. But some clipboard managers on the remote
    side react to clipboard release events by taking a clipboard grab,
    presumably to avoid empty clipboards.
    
    The two grabs, coming from the client and from the remote sides, will
    race in both directions, which may confuse the client & remote side,
    as both believe the other side is the current grab owner, and thus
    further clipboard data requests are likely to fail.
    
    Let's avoid sending a release event when re-grabing.
    
    The race described above may still happen in other rare circunstances,
    and will require a protocol change. To avoid the conflict, a discussed
    solution could use a clipboard serial number.
    
    Tested with current linux & windows vdagent. Looking at earlier
    version of the code, it doesn't seem like subsequent grabs will be
    treated as an error.
    
    Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>

diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
index 60399d0..cb00fa5 100644
--- a/src/spice-gtk-session.c
+++ b/src/spice-gtk-session.c
@@ -577,8 +577,7 @@ static void clipboard_get_targets(GtkClipboard *clipboard,
     g_return_if_fail(selection != -1);
 
     if (s->clip_grabbed[selection]) {
-        SPICE_DEBUG("Clipboard is already grabbed, ignoring %d atoms", n_atoms);
-        return;
+        SPICE_DEBUG("Clipboard is already grabbed, re-grab: %d atoms", n_atoms);
     }
 
     /* Set all Atoms that matches our current protocol implementation */
@@ -660,18 +659,14 @@ static void clipboard_owner_change(GtkClipboard        *clipboard,
         return;
     }
 
-    /* In case we sent a grab to the agent, we need to release it now as
-     * previous clipboard data should not be reachable anymore */
-    if (s->clip_grabbed[selection]) {
-        s->clip_grabbed[selection] = FALSE;
-        if (spice_main_channel_agent_test_capability(s->main, VD_AGENT_CAP_CLIPBOARD_BY_DEMAND)) {
-            spice_main_channel_clipboard_selection_release(s->main, selection);
-        }
-    }
-
-    /* We are mostly interested when owner has changed in which case
-     * we would like to let agent know about new clipboard data. */
     if (event->reason != GDK_OWNER_CHANGE_NEW_OWNER) {
+        if (s->clip_grabbed[selection]) {
+            /* grab was sent to the agent, so release it */
+            s->clip_grabbed[selection] = FALSE;
+            if (spice_main_channel_agent_test_capability(s->main, VD_AGENT_CAP_CLIPBOARD_BY_DEMAND)) {
+                spice_main_channel_clipboard_selection_release(s->main, selection);
+            }
+        }
         s->clip_hasdata[selection] = FALSE;
         return;
     }


More information about the Spice-commits mailing list