[Spice-devel] [PATCH spice-gtk 6/6] spice-session: make spice_session_channel_open_host() async again

Christophe Fergeau cfergeau at redhat.com
Thu Oct 18 09:08:16 PDT 2012


On Fri, Aug 24, 2012 at 03:58:19PM +0200, Marc-André Lureau wrote:
> The previous patches (introducing GSocketClient & GProxy usage) turned
> the code into only sync calls.

This would be much nicer to review if the GProxy changes were done
separately I think, or folded into patch 4/6


Christophe

> 
> By using async() variants of resolve & connect functions, make the
> open host connection asynchronous again.
> ---
>  gtk/spice-session.c | 195 +++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 123 insertions(+), 72 deletions(-)
> 
> diff --git a/gtk/spice-session.c b/gtk/spice-session.c
> index ae70b73..1688c58 100644
> --- a/gtk/spice-session.c
> +++ b/gtk/spice-session.c
> @@ -28,6 +28,7 @@
>  #include "gio-coroutine.h"
>  #include "glib-compat.h"
>  #include "wocky-http-proxy.h"
> +#include "spice-proxy.h"
>  
>  struct channel {
>      SpiceChannel      *channel;
> @@ -1511,59 +1512,123 @@ gboolean spice_session_has_channel_type(SpiceSession *session, gint type)
>  /* ------------------------------------------------------------------ */
>  /* private functions                                                  */
>  
> -static GSocketAddress* get_proxy_address(SpiceSession *session, guint port, GError **error)
> +static SpiceProxy* get_proxy(GError **error)
>  {
> -    SpiceSessionPrivate *s = SPICE_SESSION_GET_PRIVATE(session);
> -    GSocketAddress *address = NULL;
> -    const gchar *proxy = g_getenv("SPICE_PROXY");
> -    GList *addresses = NULL, *it;
> +    SpiceProxy *proxy;
>  
> -    if (proxy == NULL)
> +    const gchar *proxy_env = g_getenv("SPICE_PROXY");
> +    if (proxy_env == NULL || strlen(proxy_env) == 0)
>          return NULL;
>  
> -    /* FIXME: use GUri when it is ready... */
> -    if (g_ascii_strncasecmp("http://", proxy, 7) == 0)
> -        proxy += 7;
> -
> -    gchar **proxyv = g_strsplit(proxy, ":", 0);
> -    guint16 pport = 3128;
> -    const gchar *proxy_host = NULL, *proxy_port = NULL;
> -
> -    proxy_host = proxyv[0];
> -    if (proxy_host != NULL)
> -        proxy_port = proxyv[1];
> -
> -    if (proxy_port != NULL) {
> -        char *endptr;
> -        pport = strtoul(proxy_port, &endptr, 10);
> -        if (*endptr != '\0') {
> -            g_set_error(error, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> -                        "Invalid proxy port: %s", proxy_port);
> -            goto end;
> -        }
> -    }
> +    proxy = spice_proxy_new();
> +    if (!spice_proxy_parse(proxy, proxy_env, error))
> +        g_clear_object(&proxy);
> +
> +    return proxy;
> +}
> +
> +typedef struct spice_open_host spice_open_host;
>  
> -    addresses = g_resolver_lookup_by_name(g_resolver_get_default(),
> -                                          proxy_host, NULL, error);
> -    if (*error != NULL)
> +struct spice_open_host {
> +    struct coroutine *from;
> +    SpiceSession *session;
> +    SpiceChannel *channel;
> +    SpiceProxy *proxy;
> +    int port;
> +    GCancellable *cancellable;
> +    GError *error;
> +    GSocket *socket;
> +};
> +
> +static void socket_client_connect_ready(GObject *source_object, GAsyncResult *result,
> +                                        gpointer data)
> +{
> +    GSocketClient *client = G_SOCKET_CLIENT(source_object);
> +    spice_open_host *open_host = data;
> +    GSocketConnection *connection = NULL;
> +
> +    SPICE_DEBUG("connect ready");
> +    connection = g_socket_client_connect_finish(client, result, &open_host->error);
> +    if (connection == NULL)
>          goto end;
>  
> -    /* FIXME: iterate over all addresses
> -     * gproxy makes it quite unconvenient to deal with hostname,
> -     * it would make sense to do it once earlier?
> -     */
> +    open_host->socket = g_socket_connection_get_socket(connection);
> +    g_object_ref(open_host->socket);
> +
> +end:
> +    g_object_unref(connection);
> +    g_object_unref(client);
> +
> +    coroutine_yieldto(open_host->from, NULL);
> +}
> +
> +/* main context */
> +static void open_host_connectable_connect(spice_open_host *open_host, GSocketConnectable *connectable)
> +{
> +    GSocketClient *client;
> +
> +    SPICE_DEBUG("connecting %p...", open_host);
> +    client = g_socket_client_new();
> +    g_socket_client_connect_async(client, connectable, open_host->cancellable,
> +                                  socket_client_connect_ready, open_host);
> +}
> +
> +/* main context */
> +static void proxy_lookup_ready(GObject *source_object, GAsyncResult *result,
> +                               gpointer data)
> +{
> +    spice_open_host *open_host = data;
> +    SpiceSessionPrivate *s = SPICE_SESSION_GET_PRIVATE(open_host->session);
> +    GList *addresses = NULL, *it;
> +    GSocketAddress *address;
> +
> +    SPICE_DEBUG("proxy lookup ready");
> +    addresses = g_resolver_lookup_by_name_finish(G_RESOLVER(source_object),
> +                                                 result, &open_host->error);
> +    if (addresses == NULL || open_host->error) {
> +        coroutine_yieldto(open_host->from, NULL);
> +        return;
> +    }
> +
>      for (it = addresses; it != NULL; it = it->next) {
> -        address = g_proxy_address_new(G_INET_ADDRESS(it->data), pport, "http",
> -                                      s->host, port, NULL, NULL);
> +        address = g_proxy_address_new(G_INET_ADDRESS(it->data),
> +                                      spice_proxy_get_port(open_host->proxy), "http",
> +                                      s->host, open_host->port, NULL, NULL);
>          if (address != NULL)
>              break;
>      }
>  
> -end:
> +    open_host_connectable_connect(open_host, G_SOCKET_CONNECTABLE(address));
>      g_resolver_free_addresses(addresses);
> -    g_strfreev(proxyv);
> +}
> +
> +/* main context */
> +static gboolean open_host_idle_cb(gpointer data)
> +{
> +    spice_open_host *open_host = 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);
> +
> +    open_host->proxy = get_proxy(&open_host->error);
> +    if (open_host->proxy) {
> +        g_resolver_lookup_by_name_async(g_resolver_get_default(),
> +                                        spice_proxy_get_hostname(open_host->proxy),
> +                                        open_host->cancellable,
> +                                        proxy_lookup_ready, open_host);
> +    } else if (open_host->error != NULL) {
> +        coroutine_yieldto(open_host->from, NULL);
> +        return FALSE;
> +    } else
> +        open_host_connectable_connect(open_host,
> +                                      g_network_address_new(s->host, open_host->port));
> +
> +    SPICE_DEBUG("open host %s:%d", s->host, open_host->port);
> +    if (open_host->proxy != NULL)
> +        SPICE_DEBUG("(with proxy %p)", open_host->proxy);
>  
> -    return address;
> +    return FALSE;
>  }
>  
>  /* coroutine context */
> @@ -1572,47 +1637,33 @@ GSocket* spice_session_channel_open_host(SpiceSession *session, SpiceChannel *ch
>                                           gboolean use_tls)
>  {
>      SpiceSessionPrivate *s = SPICE_SESSION_GET_PRIVATE(session);
> -    GSocketClient *client = NULL;
> -    GSocketConnection *connection = NULL;
> -    GSocketAddress *address = NULL;
> -    GSocketConnectable *connectable = NULL;
> -    GSocket *socket = NULL;
> -    GError *error = NULL;
> -    int port;
> +    spice_open_host open_host = { 0, };
>  
>      if ((use_tls && !s->tls_port) || (!use_tls && !s->port))
>          return NULL;
>  
> -    port = atoi(use_tls ? s->tls_port : s->port);
> -
> -    /* FIXME: make all of this async again */
> -    client = g_socket_client_new();
> -    address = get_proxy_address(session, port, &error);
> -    if (error != NULL) {
> -        g_critical("%s", error->message);
> -        goto end;
> -    }
> +    open_host.from = coroutine_self();
> +    open_host.session = session;
> +    open_host.channel = channel;
> +    open_host.port = atoi(use_tls ? s->tls_port : s->port);
> +    g_idle_add(open_host_idle_cb, &open_host);
>  
> -    if (address != NULL)
> -        connectable = G_SOCKET_CONNECTABLE(address);
> -    else
> -        connectable = g_network_address_new(s->host, port);
> +    /* switch to main loop and wait for connection */
> +    coroutine_yield(NULL);
> +    if (open_host.error != NULL) {
> +        g_return_val_if_fail(open_host.socket == NULL, NULL);
>  
> -    connection = g_socket_client_connect(client, connectable, NULL, &error);
> -    if (connection == NULL)
> -        goto end;
> -
> -    socket = g_socket_connection_get_socket(connection);
> -    g_socket_set_blocking(socket, FALSE);
> -    g_socket_set_keepalive(socket, TRUE);
> -    g_object_ref(socket);
> +        g_warning("%s", open_host.error->message);
> +        g_clear_error(&open_host.error);
> +    } else {
> +        g_return_val_if_fail(open_host.socket != NULL, NULL);
>  
> -end:
> -    g_clear_object(&connection);
> -    g_clear_object(&connectable);
> -    g_clear_object(&client);
> +        g_socket_set_blocking(open_host.socket, FALSE);
> +        g_socket_set_keepalive(open_host.socket, TRUE);
> +    }
>  
> -    return socket;
> +    g_clear_object(&open_host.proxy);
> +    return open_host.socket;
>  }
>  
>  
> -- 
> 1.7.11.4
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20121018/4cdb4501/attachment.pgp>


More information about the Spice-devel mailing list