On Wed, Feb 12, 2014 at 12:04:25PM +0100, Marc-André Lureau wrote:
> On Wed, Feb 12, 2014 at 11:59 AM, Christophe Fergeau
> <cfergeau@xxxxxxxxxx> wrote:
> > Missed that initially, but tlsconn is leaked in error cases, and I think we
> > will leak a reference to it in success case too. My understanding is that
>
> if error, function returns NULL
tlsconn = g_tls_client_connection_new (io_stream,
G_SOCKET_CONNECTABLE(proxy_address),
error);
if (!g_tls_connection_handshake (G_TLS_CONNECTION (tlsconn), cancellable, error))
goto error;
error:
if (data_in != NULL)
g_object_unref (data_in);
g_free (buffer);
return NULL;
}
tlsconn will be leaked.
>
> > we don't own the io_stream passed as an argument, so we need to ref it in
> > the http case, but in the https case, we already own a ref on the io_stream
> > (which is tlsconn), so we ref it one too many.
>
> Where do you see we ref something?
> I remember I did check with valgrind and debugging than all references
> where correct. But I could be wrong. Please give further details.
>
tlsconn = g_tls_client_connection_new (io_stream,
G_SOCKET_CONNECTABLE(proxy_address),
error);
io_stream = tlsconn;
return g_object_ref (io_stream);
I think we are taking an extra ref on tlsconn here (ref which is probably
needed in the !tls case).
> > tlsconn is leaked here (dunno if it's guaranteed to be NULL when error is
> > set).
> >
> Yes it is guaratee, I can add a warning and g_clear_object if you want.
a g_warn_if_fail() cannot hurt as it's not explicit in the API doc (or
fixing the API doc would work too ;)
>
> >> + return;
> >> + }
> >> +
> >> + g_return_if_fail (tlsconn != NULL);
> >> +
> >> + GTlsCertificateFlags tls_validation_flags = G_TLS_CERTIFICATE_VALIDATE_ALL;
> >> +#ifdef DEBUG
> >> + tls_validation_flags -= G_TLS_CERTIFICATE_UNKNOWN_CA + G_TLS_CERTIFICATE_BAD_IDENTITY;
> >> +#endif
> >> + g_tls_client_connection_set_validation_flags (G_TLS_CLIENT_CONNECTION (tlsconn),
> >> + tls_validation_flags);
> >> + g_tls_connection_handshake_async (G_TLS_CONNECTION (tlsconn),
> >> + G_PRIORITY_DEFAULT, cancellable,
> >> + handshake_completed, data);
> >
> > A g_object_unref(tlsconn) is probably needed here.
> >
>
> no, the async is running and we need to keep the ref.
Ah, my bad, I thought glib async functions were ref'ing their arg as
needed, but I'm wrong from a quick glance at gio/glib-networking.
Christophe
Attachment:
pgpCreKKsAc7e.pgp
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel