[Vortex] [BUG] Race condition in vortex_tls_start_negotiation_sync

[Francis Brosnan Blazquez]

Hi Benoit,

> Forgot the actual patch...

Thanks for the patch. I've being checking the issue and I see your race
description. 

Checking your patch I see it implements something that already provides
vortex_tls_start_negotation (which always returns a coherent connection,
no matter the termination status), and at the same time I think the
patch hides a bug in vortex_tls_start_negotation_sync which should
return NULL in the case of timeout for the reasons you have pointed.

In this context I find that the root issue is indeed located at the
incoherent behavior of vortex_tls_start_negotation_sync, so I've updated
its internal implementation to ensure that, if the user receives a
connection reference at the function termination it is because it is
safe and no background TLS process will shutdown/dealloc it, but in the
case a new NULL internal connection is received, due to a timeout in the
middle of TLS, the function returns NULL, and internally the connection
created is finished (if any).

I've added a regression test (test_05-d) to reproduce your bug
description and to check the fix introduced.

This change only applies to Vortex 1.1 series. I didn't apply your
patch. Thanks for reporting Benoit. Cheers!

> > -----Original Message-----
> > From: AMIAUX Benoît
> > Sent: Friday, August 20, 2010 10:27 AM
> > To: 'Vortex'
> > Subject: RE: [BUG] Race condition in vortex_tls_start_negotiation_sync
> >
> > Hello,
> >
> > Attached patch fixes the issue for me.
> >
> > The main point is that we have to ref the connection we're using inside
> > vortex_tls_start_negotiation_sync because it can be closed and un-
> > referred if a timeout happens.
> >
> > Moreover, my version returns NULL in case of timeouts because it seems
> > to be impossible without too many changes to ensure the input
> > vortex_connection* pointer is valid on errors. This is due to the
> > vortex_connection_unref in
> >
> > vortex_tls.c:895
> > vortex_connection_unref (connection_aux, "(vortex tls process)");
> >
> > which is not thread-safe in our case. We cannot guarantee not too many
> > unref happens if we return the old connection pointer.
> >
> > This patch works for me when I start 64 threads at once which tries to
> > create TLS connections and fail repeatedly.
> >
> > Thanks you,
> > Benoît Amiaux
> 
> Ce message et toutes les pièces jointes sont confidentiels et établis à l'intention exclusive de ses destinataires. Toute modification, édition, utilisation ou diffusion non autorisée est interdite. Si vous avez reçu ce message par erreur, merci de nous en avertir immédiatement. ATEME décline toute responsabilité au titre de ce message s'il a été altéré, déformé, falsifié ou encore édité ou diffusé sans autorisation.
> This message and any attachments are confidential and intended solely for the addressees. Any unauthorized modification, edition, use or dissemination is prohibited. If you have received this message by mistake, please notify us immediately. ATEME decline all responsibility for this message if it has been altered, deformed, falsified or even edited or disseminated without authorization.
> _______________________________________________
> Vortex mailing list
> Vortex at lists.aspl.es
> http://lists.aspl.es/cgi-bin/mailman/listinfo/vortex
-- 
Francis Brosnan Blazquez <francis at aspl.es>
ASPL