[Spice-devel] [spice-gtk PATCHv2] Keep GSocketConnection around after initial connect
Marc-André Lureau
mlureau at redhat.com
Wed Mar 27 09:03:40 PDT 2013
----- Mensaje original -----
> 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 | 9 +++++++--
> gtk/spice-session-priv.h | 4 ++--
> gtk/spice-session.c | 22 +++++++++++-----------
> 4 files changed, 21 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..21e2d26 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_object_ref(G_OBJECT(g_socket_connection_get_socket(c->conn)));
The G_OBJECT is unnecessary
> c->has_error = FALSE;
>
> @@ -2453,6 +2454,10 @@ static void channel_reset(SpiceChannel
> *channel, gboolean migrating)
> c->ctx = NULL;
> }
>
> + if (c->conn) {
> + g_object_unref(c->conn);
> + 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;
> }
>
ack
More information about the Spice-devel
mailing list