Re: [boost] [ASIO] async SSL operations unnecessary serialized?

Subject: Re: [boost] [ASIO] async SSL operations unnecessary serialized?
From: Michał Madziar (michoo_at_[hidden])
Date: 2012-09-15 13:53:54

On Sat, Sep 15, 2012 at 01:46:00PM +0200, Lars Viklund wrote:
> On Fri, Sep 14, 2012 at 11:06:43PM +0200, Michał Madziar wrote:
> > Hello all.
> > It looks like all openssl_operation handlers are synchronized on  
> > io_service allowing only one connection to be processed at the a time.  
> > As I understand this orginated from the time when libopenSSL didn't  
> > support multi-threaded applications, but it slows down everything a lot.
> 
> It's not long ago someone had a long rant on the IRC channel about the
> insanity of not serializing OpenSSL operations, as it's "well known"
> that it's not generally thread-safe.
"Well known" by who? 

Apart from that - current design makes not only SSL operations but also
handlers execute in strand, and that's really broken. Even if all SSL
operations should remain serialized, user callbacks shouldn't.

> 
> > Adding locking function, and getting rid of strand_->wrap in lines 320,  
> > 385 in file boost/asio/ssl/detail/openssl_operation.hpp allows SSL  
> > connections to be used in multiple threads simultaneously solving the  
> > problem. Or do I miss something?
> >
> > I've attached simple line-based echo server which demonstrates the problem:
> > openssl s_client -connect localhost:10001 #x 2
> > telnet localhost 10000 #x 2
> > After sending something in each connection TCP and one SSL work in  
> > parallel, second SSL waits with receiving until first one finishes it's  
> > write. After patching all connections work in parallel.
> 
> Since which OpenSSL version is this supposed universal lock feature?
According to CSV it's only about 12 years old. (At least that's when they've
changed sample locking function from old macro to CRYPTO_num_locks)

> 
> What kind of macros exist to test the existence of this feature and
> whether it's enabled?
http://www.openssl.org/docs/crypto/threads.html says:
  You can find out if OpenSSL was configured with thread support:
 
  #define OPENSSL_THREAD_DEFINES
  #include <openssl/opensslconf.h>
  #if defined(OPENSSL_THREADS)
    // thread support enabled
  #else
    // no thread support
  #endif

> 
> Do you need some particular flags to your program to enable a
> threading-enabled OpenSSL?
You only have to check whenever your version of library was configured with
thread support.

> 
> What about libraries that are interface-compatible with OpenSSL?
They won't have macro defined I guess.

> 
> Just changing things based on what some bleeding-edge version may
> support sounds a bit eager,
I fully agree, but I also think that 12 years is stable enough.

> particularly considering that many
> environments may ship rather old and stable ones which are quite a
> bother to replace safely.
I'm more concerned about applications developed with boost than library
itself - some developers may have made some assumptions about "ssl in strand".
If any changes are to be made I thing it would be better to let user explicitly
enable them by #define i.e. BOOST_ASIO_ENABLE_SSL_MULTI_THREADS

-- 
Pozdrawiam
Michał 'michoo' Madziar