[Spice-devel] [spice-gtk] Keep GSocketConnection around after initial connect

Christophe Fergeau cfergeau at redhat.com
Wed Mar 27 07:53:20 PDT 2013


There has been reports of recent spice-gtk versions not working on
RHEL6 or Ubuntu 10.04. This happens because these systems have
an older glib version without:

commit a0e1b226a21ca498b301981b0c89e89ad9a31eb1
Author: Dan Winship <danw at gnome.org>
Date:   Fri Apr 23 08:47:18 2010 -0400

    GSocketConnection: don't close the socket if it's still reffed

    When disposing a GSocketConnection, don't explicitly close the
    underlying GSocket. The GSocket will close itself if it gets
    destroyed, and if it doesn't get destroyed, that presumably means the
    app still wants to use it. Eg, this lets you use GSocketClient to
    create a GSocketConnection, and then take the GSocket and destroy the
    GSocketConnection.

    https://bugzilla.gnome.org/show_bug.cgi?id=616855

and spice-gtk commit 0f9a432c "session: allow to connect via HTTP CONNECT
proxy" changed spice_session_channel_open_host to get its socket by doing:

open_host->socket = g_socket_connection_get_socket(connection);
g_object_ref(open_host->socket);
g_object_unref(connection);
(see socket_client_connect_ready)

If glib does not have the commit mentioned above, then this won't
work as expected as open_host->socket will get closed when 'connection'
gets destroyed.

This commit changes spice_session_channel_open_host to return a
GSocketConnection rather than a GSocket so that we can keep the
socket open even on older glib versions.

Huge thanks go to Brad Campbell <brad at fnarfbargle.com> for doing all the
spice-gtk/glib bisecting work.
---
 gtk/spice-channel-priv.h |  1 +
 gtk/spice-channel.c      | 10 ++++++++--
 gtk/spice-session-priv.h |  4 ++--
 gtk/spice-session.c      | 22 +++++++++++-----------
 4 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/gtk/spice-channel-priv.h b/gtk/spice-channel-priv.h
index 5e6ec1e..b2d8a14 100644
--- a/gtk/spice-channel-priv.h
+++ b/gtk/spice-channel-priv.h
@@ -80,6 +80,7 @@ struct _SpiceChannelPrivate {
     SSL                         *ssl;
     SpiceOpenSSLVerify          *sslverify;
     GSocket                     *sock;
+    GSocketConnection           *conn;
 
 #if HAVE_SASL
     sasl_conn_t                 *sasl_conn;
diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
index 7b9807b..040dc08 100644
--- a/gtk/spice-channel.c
+++ b/gtk/spice-channel.c
@@ -2197,8 +2197,8 @@ static void *spice_channel_coroutine(void *data)
     }
 
 reconnect:
-    c->sock = spice_session_channel_open_host(c->session, channel, c->tls);
-    if (c->sock == NULL) {
+    c->conn = spice_session_channel_open_host(c->session, channel, c->tls);
+    if (c->conn == NULL) {
         if (!c->tls) {
             CHANNEL_DEBUG(channel, "trying with TLS port");
             c->tls = true; /* FIXME: does that really work with provided fd */
@@ -2209,6 +2209,7 @@ reconnect:
             goto cleanup;
         }
     }
+    c->sock = g_socket_connection_get_socket(c->conn);
 
     c->has_error = FALSE;
 
@@ -2453,6 +2454,11 @@ static void channel_reset(SpiceChannel *channel, gboolean migrating)
         c->ctx = NULL;
     }
 
+    if (c->conn) {
+        g_object_unref(c->conn);
+        c->sock = NULL;
+        c->conn = NULL;
+    }
     if (c->sock) {
         g_object_unref(c->sock);
         c->sock = NULL;
diff --git a/gtk/spice-session-priv.h b/gtk/spice-session-priv.h
index 7ee6b6c..d5df378 100644
--- a/gtk/spice-session-priv.h
+++ b/gtk/spice-session-priv.h
@@ -114,8 +114,8 @@ void spice_session_set_connection_id(SpiceSession *session, int id);
 int spice_session_get_connection_id(SpiceSession *session);
 gboolean spice_session_get_client_provided_socket(SpiceSession *session);
 
-GSocket* spice_session_channel_open_host(SpiceSession *session, SpiceChannel *channel,
-                                         gboolean use_tls);
+GSocketConnection* spice_session_channel_open_host(SpiceSession *session, SpiceChannel *channel,
+                                                   gboolean use_tls);
 void spice_session_channel_new(SpiceSession *session, SpiceChannel *channel);
 void spice_session_channel_destroy(SpiceSession *session, SpiceChannel *channel);
 void spice_session_channel_migrate(SpiceSession *session, SpiceChannel *channel);
diff --git a/gtk/spice-session.c b/gtk/spice-session.c
index 6fa8699..2010c75 100644
--- a/gtk/spice-session.c
+++ b/gtk/spice-session.c
@@ -1639,7 +1639,7 @@ struct spice_open_host {
     int port;
     GCancellable *cancellable;
     GError *error;
-    GSocket *socket;
+    GSocketConnection *connection;
     GSocketClient *client;
 };
 
@@ -1655,11 +1655,9 @@ static void socket_client_connect_ready(GObject *source_object, GAsyncResult *re
     if (connection == NULL)
         goto end;
 
-    open_host->socket = g_socket_connection_get_socket(connection);
-    g_object_ref(open_host->socket);
+    open_host->connection = connection;
 
 end:
-    g_clear_object(&connection);
     coroutine_yieldto(open_host->from, NULL);
 }
 
@@ -1711,7 +1709,7 @@ static gboolean open_host_idle_cb(gpointer data)
     SpiceSessionPrivate *s = SPICE_SESSION_GET_PRIVATE(open_host->session);
 
     g_return_val_if_fail(open_host != NULL, FALSE);
-    g_return_val_if_fail(open_host->socket == NULL, FALSE);
+    g_return_val_if_fail(open_host->connection == NULL, FALSE);
 
 #if GLIB_CHECK_VERSION(2,26,0)
     open_host->proxy = s->proxy;
@@ -1742,8 +1740,8 @@ static gboolean open_host_idle_cb(gpointer data)
 
 /* coroutine context */
 G_GNUC_INTERNAL
-GSocket* spice_session_channel_open_host(SpiceSession *session, SpiceChannel *channel,
-                                         gboolean use_tls)
+GSocketConnection* spice_session_channel_open_host(SpiceSession *session, SpiceChannel *channel,
+                                                   gboolean use_tls)
 {
     SpiceSessionPrivate *s = SPICE_SESSION_GET_PRIVATE(session);
     spice_open_host open_host = { 0, };
@@ -1774,13 +1772,15 @@ GSocket* spice_session_channel_open_host(SpiceSession *session, SpiceChannel *ch
     if (open_host.error != NULL) {
         g_warning("%s", open_host.error->message);
         g_clear_error(&open_host.error);
-    } else if (open_host.socket != NULL) {
-        g_socket_set_blocking(open_host.socket, FALSE);
-        g_socket_set_keepalive(open_host.socket, TRUE);
+    } else if (open_host.connection != NULL) {
+        GSocket *socket;
+        socket = g_socket_connection_get_socket(open_host.connection);
+        g_socket_set_blocking(socket, FALSE);
+        g_socket_set_keepalive(socket, TRUE);
     }
 
     g_clear_object(&open_host.client);
-    return open_host.socket;
+    return open_host.connection;
 }
 
 
-- 
1.8.1.4



More information about the Spice-devel mailing list