Re: Assigning to a boost::asio::ssl::context while asynchronous operations are using it

From: Martijn Otto (boost_at_[hidden])
Date: 2019-09-27 07:01:35

On Fri, 2019-09-27 at 06:30 +0500, Dmitrij V via Boost-users wrote:
> Yes, you have no problem for now just by pure luck.
> In the destructor of the context freeing all related to SSL
> resources.
> 
> How you can avoid the problems in future:
> 
> as an one of soltions, in your loop thread (if you have it, like me
> for example):
> 
> 1) io_.stop();
> 2) while (io_.poll()) {}
> 3) HERE your code to assign the context (perhaps you will need to
> rehandshake alived sessions)
> 4) io_.restart();
> 5) acceptor_.async_accept(...);
> 
> PS: In my past project I had very likely approach - restart server
> without stopping the process app. I had reject all my beginnings and
> forced clients to reconnect by losed the connections (there are many
> bytes would needs to store in shared memory (related to session
> data))
> 
> --
> regards
> _______________________________________________
> Boost-users mailing list
> Boost-users_at_[hidden]
> https://lists.boost.org/mailman/listinfo.cgi/boost-users

It makes sense for the destructor to indeed clean up resources. That's
what it's for. However, looking at the OpenSSL documentation, there are
three relevant functions:

SSL_CTX_new(), creating an SSL_CTX instance with refcount 1
SSL_CTX_up_ref(), increasing the refcount
SSL_CTX_free(), decreasing the refcount, freeing when reaching 0

Now, given that OpenSSL uses a reference-counted system for their TLS
contexts, it would make sense if boost::asio were to increment the
reference count when starting the handshake, and decrement it when this
is done, which could easily make what I'm doing safe.

However, looking at the source, it appears that's not what it's doing.
I believe it wouldn't be much work to rectify this. In ssl/stream.hpp,
we are already taking the underlying SSL_CTX object as a member and
store it in core_. I think it'd be trivial to add an SSL_CTX_up_ref()
to the constructor and an SSL_CTX_free() to the destructor. If we do
this, we avoid potential problems, like the context being freed while
the stream is active.

I could make a pull-request to implement these changes, if so desired.
Does this have any change of getting merged?