[haiku-commits] Re: haiku: hrev46679 - src/kits/network/libnetapi headers/os/net

  • From: Adrien Destugues <pulkomandy@xxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Thu, 16 Jan 2014 09:42:18 +0100

> >    * Instead of creating an OpenSSL context ofor each socket, use a global
> >    one and initialize it lazily when the first SecureSocket is created
> 
> And this indeed works without locking? Libraries usually have this
> context stuff to handle threading correctly (for example FreeType). I
> don't know how it works in OpenSSL, but why is there a context in the
> first place, if not for that purpose?

From the OpenSSL FAQ:

"
1. Is OpenSSL thread-safe?

Yes (with limitations: an SSL connection may not concurrently be used by 
multiple threads). On Windows and many Unix systems, OpenSSL automatically 
uses the multi-threaded versions of the standard libraries. If your platform 
is not one of these, consult the INSTALL file.
"

The context is a way to store default values for most of the parameters, 
instead of configuring each and every connection manually. This includes two 
things we use here, the certificates store and the callback to call when a 
certificate chain couldn't be validated.

On the other hand, we are actually using the SSL connections from different 
threads, as we attempt to close it from outside the protocol loop thread to 
unlock blocking reads as we do for regular sockets.

> 
> >    Review of the API and implementation is welcome, before I start making
> >    use of this in HttpRequest and WebKit to allow the user to accept new
> >    certificates.
> 
> I had only little time so far to look over the patch, will try to make
> some more time to have a closer look. I spotted many coding style
> violations and also stuff which looks very much like bugs, for example
> you return time_t somewhere and use it as bigtime_t elsewhere. The
> initialization of the context has a race condition, since there is no
> locking, too.

sInitOnce is a vint32 (volatile), which I was told was enough when I wrote 
the Locale Kit catalog loading which works the same way. If that actually 
doesn't work, I can implement it using pthread init_once features, and also 
fix the catalog loading to do the same.

I'll fix the bigtime_t issues, and the coding style problems if I can see 
them.

-- 
Adrien.

Other related posts: