asio icon indicating copy to clipboard operation
asio copied to clipboard

Enable asio::ssl::stream's ability to be moved to another contexts

Open devgs opened this issue 6 years ago • 9 comments

Basic sockets, such as asio::ip::tcp::socket naturally have an ability to be re-bound to a different context, via release() + assign(), using a native_handle. That's a great thing in a shared-nothing nano-service application, where different nano-services run on a separate threads, each with its own io_context, to avoid CPU cache pollution. In such an architecture there may be some nano-services that establish connections (say, an optional SOCKS) and other nano-services that use established connections without the knowledge of how they are established.

This works well, until we start using asio::ssl::stream. For some reason, asio::ssl::detail::stream_core implementation uses timers to implement a queue of pending operation handlers (read and write). This seems like a bad decision. Times are bound to a single context, the one that was supplied in the stream_core's constructor, at the moment of its creation. This leads to inability to completely move asio::ssl::stream out of it's original context, even though the underlying layer, asio::ip::tcp::socket, can be moved.

Leaving timers in one context and underlying layer in another will inevitably lead to violation of 'single strand usage' requirement and therefore to data races. In my case, this is easily reproduced by issuing async_shutdown() while there is some pending IO already, causing segfaults.

Is it possible to replace timers with an actual handler queue? After all, timers there aren't used to actually track time, so this looks like a hack anyway.

Then, handlers can be invoked in a same way as done today: via fake 0-byte read:

...

          // The SSL operation is done and we can invoke the handler, but we
          // have to keep in mind that this function might be being called from
          // the async operation's initiating function. In this case we're not
          // allowed to call the handler directly. Instead, issue a zero-sized
          // read so the handler runs "as-if" posted using io_context::post().
          if (start)
          {
            next_layer_.async_read_some(
                asio::buffer(core_.input_buffer_, 0),
                ASIO_MOVE_CAST(io_op)(*this));

            // Yield control until asynchronous operation completes. Control
            // resumes at the "default:" label below.
            return;
          }

...

devgs avatar Oct 16 '19 10:10 devgs

That's gonna be a tough nut to crack. To move the ssl stream to a different io_context you would need to cancel the pending handler. But you can't delay the move. So you would need to install an execution_context::service that tracks all of the existing ssl stream objects in a particular io_context to deal with the canceled handler after the associated stream is moved. This is doable but quite a bit of work.

vinniefalco avatar Nov 01 '19 15:11 vinniefalco

@vinniefalco or, library could enforce a verbal requirement: you must not call this function if stream has unfinished handlers attached. Similar requirement already exists: you must not call async_read_some() a second time, before the first handler is executed. Same is true for other operations, ie async_write_some().

devgs avatar Nov 01 '19 16:11 devgs

The problem is that the internal handlers used in the ssl stream core implementation don't belong to the user, and are thus unobservable. To know whether there are unfinished handlers would require knowing about implementation details.

vinniefalco avatar Nov 01 '19 16:11 vinniefalco

Anyway, asio::ssl::stream is not particularly user-friendly, we need something better.

vinniefalco avatar Nov 01 '19 16:11 vinniefalco

@vinniefalco, internal handlers are directly attached to the original handler that is supplied by the user. It can be seen from the source code, and that's a common sense. If I don't request anything from the stream the it should do nothing in background. Whenever stream wants to read/write more data, it stores the original handler for later, when the request could be fulfilled. That is the problem: it stores it via deadline_timer, which is really really odd thing to do.

devgs avatar Nov 01 '19 16:11 devgs

@vinniefalco, Totally agree, we need not only a better stream, but also a sane, modern TLS library, instead of OpenSSL.

devgs avatar Nov 01 '19 16:11 devgs

it stores it via deadline_timer, which is really really odd thing to do.

It does this to reduce the number of template instantiations and keep the size of the compiled code smaller.

vinniefalco avatar Nov 01 '19 17:11 vinniefalco

It has been a long time. Please, consider a proper implementation, finally. Use an op_queue instead of a timer deadline_timer.

devgs avatar Sep 10 '21 07:09 devgs

Few more years have passed us by and I'm here again, asking for a solution.

devgs avatar Oct 11 '24 14:10 devgs