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

Subject: Re: [boost] [ASIO] async SSL operations unnecessary serialized?
From: Lars Viklund (zao_at_[hidden])
Date: 2012-09-16 09:50:55

On Sat, Sep 15, 2012 at 07:53:54PM +0200, Michał Madziar wrote:
> 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? 

The bloke in IRC. I don't use Asio's SSL myself.

> 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)

I'm going solely based on the input of the very upset IRC channel
visitor that violently argued that Asio SSL was broken beyond
recognition.

> > 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.

Excellent.

> > 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.

Indeed. I was of the impression that this was something rather new.

> > 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

In order to not lose track of this, file a comprehensive bug on the
Boost Trac [1] under the right component. Well-tested patches are
probably welcome as well.

[1] https://svn.boost.org/trac/boost/

-- 
Lars Viklund | zao_at_[hidden]