Movable ssl::stream

[viccpp]

Is there any reason why ssl::stream is still not movable? As far as I can see, the feature was requested 2 years ago here https://svn.boost.org/trac/boost/ticket/9724

[vinniefalco]

[EDITED]

I'm thinking about writing this wrapper:

...

• I removed this code because it is out of date, and Beast has something even better (see below)

[vinniefalco]

[EDITED]

@2underscores-vic This compiles, but is untested:

• I removed this code because it is out of date, and Beast has something even better (see below)

[viccpp]

Yes, I would use the same workaround - allocation on heap, but in my project I'm trying to minimize heap allocations, so it's used only on highest levels of abstraction.

BTW, move operations can be easily added to ssl::stream_base (as far as I cant see). And ssl::stream object consists of ssl::stream_base + socket. Is there any thechnical difficulties to do this?

[vinniefalco]

in my project I'm trying to minimize heap allocations

Have you looked at the implementation of ssl::stream? There are allocations everywhere, through std::vector, and inside OpenSSL itself. A single, additional allocation for each connection is not going to hurt.

Is there any thechnical difficulties to do this

I don't know. You might look at the declaration for ssl::detail::engine and ssl::detail::stream_core. I assume there's a good reason that ssl::stream is not movable, considering that other types such as basic_socket are movable.

You're proposing changing the source code to Boost.Asio. That might work for your application but its not really a good solution to force everyone who wants to use some piece of code to modify their installation of Boost.

[viccpp]

You're proposing changing the source code to Boost.Asio

No, I use standalone Asio 1.11. And why others have to modify their code if they don't/can't use moving today?

[viccpp]

BTW, move operations can be easily added to ssl::stream_base

Sorry, I meant detail::stream_core. Data members are:

asio::steady_timer/asio::deadline_timer pending_read_;
asio::steady_timer/asio::deadline_timer pending_write_;
std::vector<unsigned char> output_buffer_space_;
const asio::mutable_buffers_1 output_buffer_;
std::vector<unsigned char> input_buffer_space_;
const asio::mutable_buffers_1 input_buffer_;
asio::const_buffer input_;

const members are not move assignable but move copyable/movable. Can we just make them non-const?
@chriskohlhoff answer something.

[vinniefalco]

why others have to modify their code if they don't/can't use moving today?

I can't add a feature to Beast (my HTTP library) that requires users to modify their Asio sources.
https://github.com/vinniefalco/Beast/

[viccpp]

I don't understand how this feature request to Asio relates to Beast or any other library...

[trivigy]

This feature request has been open for almost two years? I am using beast library and the immovability of the ssl::stream makes it super hard to pass a stream that has a handshake initiated on it into other classes. For example: handshare a stream in http class and hand it off to websocket class.

Any update on the progress would be much appreciated.

[vinniefalco]

I am using beast library and the immovability of the ssl::stream makes it super hard to pass a stream that has a handshake initiated on it into other classes. For example: handshare a stream in http class and hand it off to websocket class.

This is already a solved problem, the Beast "advanced server" examples implement WebSocket upgrade handoff of the ssl stream from the HTTP request to the WebSocket handler. A movable version of ssl::stream comes with the examples:
https://github.com/boostorg/beast/blob/4d660a5e54dee428e99ec0520620e028c1975727/example/common/ssl_stream.hpp#L22

Advanced server examples may be found here:
http://www.boost.org/doc/libs/1_66_0/libs/beast/doc/html/beast/examples.html#beast.examples.servers_advanced

[trivigy]

Hey @vinniefalco thanks for sharing. I went through that code example with a fine comb. Really cool example. I ended up refactoring the code and using the ssl_stream but instead of all the generics and separate classes I used only two classes (Http, Websocket) and used boost::variant to pass both plain and secured sockets around.

[omartijn]

It's nice that we have a working example emulating a proper, movable ssl::stream. It is still confusing for developers though - you have to really search for how to do this instead of it "just" working.

Reading the comments on this issue, the only reason it seems we don't have movable streams is that the buffers are made const. I think the added value we get from those members being const in no way outweighs the benefits of having movable streams.

I would really like to hear @chriskohlhoff take on this. If he offers no objections, I'd consider making a PR to fix this once and for all.

[vinniefalco]

Beast offers an ssl_stream which is not only movable but overcomes a performance limitation of asio's ssl::stream when performing writes with buffer sequences having length greater than one: boostorg/asio#100

Beast's ssl_stream: https://github.com/boostorg/beast/blob/9f8cf7d5990afc2943de55511eca655676d6d393/include/boost/beast/_experimental/core/ssl_stream.hpp#L55

This will become an "official" interface in an upcoming Boost release, but it can be relied upon today. Also note that Boost.Beast 1.70 will ship beast::tcp_stream which is a stream that has built-in timeouts. You can declare

beast::ssl_stream<
    beast::tcp_stream<
        net::io_context::strand>> stranded_stream(ioc);

// or, without the strand:
beast::ssl_stream<
    beast::tcp_stream<
        net::io_context::executor_type>> stream(ioc);

To get a stream with these features:

• Uses tcp::socket at the bottom
• Supports configurable built-in timeouts:
  • beast::error::timeout delivered to your completion handler
• Supports configurable bandwidth management (limits on reading and writing)
• Built-in strand (optional), never have to call bind_executor again!
• Workaround for ssl::stream writes inefficient when buffer sequences have length > 1 boostorg/asio#100

In-development version of tcp_stream is here https://github.com/boostorg/beast/blob/9f8cf7d5990afc2943de55511eca655676d6d393/include/boost/beast/core/basic_stream.hpp#L192

[maddanio]

did you propose this to asio? if not, why not. I mean: great for the workaround!! but seems a bit messy to have to do this, especially since they are both boost libraries.

[vinniefalco]

if not, why not

What would be the difference between being in beast or being in asio?

[omartijn]

What would be the difference between being in beast or being in asio?

Visibility. When you think about "how do I sent something over this TCP socket with a timeout", the most logical place to look for something like this is asio. You won't find it there and might conclude you have to write it all on your own.

The things you write here are very useful. It'd be a shame if people don't use them because they don't find them.

[maddanio]

or in the original case for which this ticket was opened if the ssl stream in asio where to be made movable then users of asio, and also would directly pick up the movable streams and have less head aches. also they would not have to depend on beast to use it.

[vinniefalco]

beast's ssl_stream contains a workaround for the lack of scatter/gather I/O in ssl::stream. I don't think that workaround is appropriate in ASIO.

[maddanio]

How so?

[viccpp]

Resolved in 658614d?

[omartijn]

Oh, this looks very promising indeed! @vinniefalco Would you take a PR that switches out the std::unique_ptr<stream_type for a simple stream_type?

I guess to be properly compatible with standalone asio (which might have an older version) we could simply make a type trait that checks whether or not stream_type is move-constructible and assignable.

[vinniefalco]

The latest Asio makes beast::ssl_stream obsolete, so no.

[viccpp]

Resolved in 658614d?

No.

1. Move constructor isn't noexcept.
2. No move-assignment at all.