[Asio/Beast] Any non-obvious reasons for no bind_executor_and_allocator?
Hi, As I'm looking to modernize an older C++03 project based on Boost.Asio to use C++17 and the net TS api, I am not ready yet to give up on using lambdas for completion handlers. Boost.Asio and Boost.Beast have some utilities for correctly implementing asynchronous composed operations but from them only asio::bind_executor seems to be applicable to lambdas. And the problem is that it only associates an executor with a (lambda) object; I didn't find any utility for associating an allocator too. Could there be any reason for a utility that associates both an executor and an allocator with an object to not work correctly when implementing asynchronous operations? Or is it just that this utility has not been implemented yet? I did come across the paper "Networking TS Associations for Call Wrappers"[1], but I didn't see any mention there about bind_executor or something similar for allocators or why this kind of approach wouldn't work. I would be very happy if I could write something like: asio::post( socket_.get_executor(), bind_executor_and_allocator_from( handler_, [...] { self_.handler_(...); })); and, unless I'm missing something, such a utility could be implemented similarly to the existing asio::bind_executor. Thank you, Sorin [1] http://open-std.org/JTC1/SC22/WG21/docs/papers/2018/p1133r0.html
On Wed, Dec 19, 2018 at 4:31 PM Sorin Fetche via Boost-users <boost-users@lists.boost.org> wrote:
I did come across the paper "Networking TS Associations for Call Wrappers"[1] but I didn't see any mention there about bind_executor or something similar for allocators or why this kind of approach wouldn't work.
Eh... you have `socket_` which means you have a class, so there is really no reason not to use the intrusive mechanism for associating your class with the executor and allocator associated with `handler_`. This is covered in the Beast composed operation tutorial: <https://www.boost.org/doc/libs/1_69_0/libs/beast/doc/html/beast/using_io/writing_composed_operations.html> When I want to use my own allocator, I usually just add a "wrap" member to the allocator which performs the association: <https://github.com/boostorg/beast/blob/c4813a5ac79d802951cf70b811ded94ce7ef7d79/example/common/session_alloc.hpp#L281> The need for a specific, dedicated function to perform the allocator association has just never come up in practical code. I think this is why Asio / Net.TS doesn't provide it. Regards
On Wed, Dec 19, 2018 at 4:57 PM Vinnie Falco wrote:
On Wed, Dec 19, 2018 at 4:31 PM Sorin Fetche via Boost-users <boost-users@lists.boost.org> wrote:
I did come across the paper "Networking TS Associations for Call Wrappers"[1] but I didn't see any mention there about bind_executor or something similar for allocators or why this kind of approach wouldn't work.
Eh... you have `socket_` which means you have a class, so there is really no reason not to use the intrusive mechanism for associating your class with the executor and allocator associated with `handler_`. This is covered in the Beast composed operation tutorial:
< https://www.boost.org/doc/libs/1_69_0/libs/beast/doc/html/beast/using_io/wri...
Thank you for the link and yes I'm familiar with it. Starting from it and the recent "composed" examples from Boost.Asio, I got to my question about bind_executor and lambdas. And this is what I have in mind: https://github.com/sorf/cpp-playground/blob/master/source/asio_echo.cpp#L45 \code self.socket.async_read_some( read_buffer, asio::bind_executor( self.get_executor(), [self = std::move(self)](error_code ec, std::size_t bytes) mutable { if (!ec) { /* ... */ } else { self.user_completion_handler(ec, bytes); } })); \endcode Basically, no need to define an operator() and implement an explicit state machine in it and no need for class methods and using a flavor of bind for short completion handlers that fit in a small lambda. As it is now it just misses the forwarding of final handler allocator to the internal operations. Best regards, Sorin
On Wed, Dec 19, 2018 at 9:37 PM Sorin Fetche <sorin.fetche@gmail.com> wrote:
And this is what I have in mind: ... self.socket.async_read_some( read_buffer, asio::bind_executor( self.get_executor(), [self = std::move(self)](error_code ec, std::size_t bytes) mutable { if (!ec) { /* ... */ } else { self.user_completion_handler(ec, bytes); } }));
I sense danger here. The order of evaluation of function arguments is not defined. If the lambda is constructed first, then the call to `self.get_executor()` will be performed on a moved-from object. I could be wrong though... Regards
On Wed, Dec 19, 2018 at 10:14 PM Vinnie Falco wrote:
On Wed, Dec 19, 2018 at 9:37 PM Sorin Fetche wrote:
And this is what I have in mind: ... self.socket.async_read_some( read_buffer, asio::bind_executor( self.get_executor(), [self = std::move(self)](error_code ec, std::size_t bytes) mutable { if (!ec) { /* ... */ } else { self.user_completion_handler(ec, bytes); } }));
I sense danger here. The order of evaluation of function arguments is not defined. If the lambda is constructed first, then the call to `self.get_executor()` will be performed on a moved-from object. I could be wrong though...
True, that code is risky as it is right now. And that's why I had to create the read_buffer variable. But a more refined utility function or even base class could help avoid some of the risks and make the code even shorter (e.g. replace with one function call bind_executor(self.get_executor(), ..)).
On Wed, Dec 19, 2018 at 9:37 PM Sorin Fetche <sorin.fetche@gmail.com> wrote:
And this is what I have in mind:
https://github.com/sorf/cpp-playground/blob/master/source/asio_echo.cpp#L45
Have you considered writing a base class which takes ownership of the user's completion handler and has the necessary hooks? Then you can simply derive from it in your function-level class declaration to get all the hooks. Such a base class might look like this: template<class Handler, class Derived> class operation_base { template<class T, class Executor> friend struct boost::asio::associated_executor; Handler handler_; protected: Handler const& handler() const noexcept { return handler_; } Handler& handler() noexcept { return handler_; } public: operation_base(operation_base&&) = default; operation_base(operation_base const&) = default; operation_base& operator=(operation_base&&) = delete; operation_base& operator=(operation_base const&) = delete; explicit operation_base(Handler&& handler) : handler_(std::move(handler)) { } explicit operation_base(Handler const& handler) : handler_(handler) { } using allocator_type = boost::asio::associated_allocator_t<Handler>; allocator_type get_allocator() const noexcept { return boost::asio::get_associated_allocator(handler_); } template<class Function> friend void asio_handler_invoke(Function&& f, operation_base* op) { using boost::asio::asio_handler_invoke; asio_handler_invoke(f, std::addressof(op->handler_)); } friend bool asio_handler_is_continuation(operation_base* op) { using boost::asio::asio_handler_is_continuation; return asio_handler_is_continuation( std::addressof(op->handler_)); } }; namespace boost { namespace asio { template<class Handler, class Derived, class Executor> struct associated_executor< operation_base<Handler, Derived>, Executor> { using type = typename associated_executor<Handler, Executor>::type; static type get(operation_base<Handler, Derived> const& op, Executor const& ex = Executor()) noexcept { return associated_executor< Handler, Executor>::get(op.handler_, ex); } }; } // asio } // boost Disclaimer: Untested. The class uses CRTP in order for the associated_executor specialization to be selected when passing the derived type. Regards
On Wed, Dec 19, 2018 at 10:24 PM Vinnie Falco wrote:
On Wed, Dec 19, 2018 at 9:37 PM Sorin Fetche wrote:
And this is what I have in mind:
https://github.com/sorf/cpp-playground/blob/master/source/asio_echo.cpp#L45
Have you considered writing a base class which takes ownership of the user's completion handler and has the necessary hooks?
<snip> Yes, the base class is a very good idea. It helps reduce the boilerplate in the composed operation and the risk of moving `this` before accessing things in it. Here's how its usage looks like to implement the composed operation: https://github.com/sorf/cpp-playground/blob/master/source/asio_echo_v2.cpp#L... \code struct internal_state : public state_base { /*...*/ static void async_echo(internal_state&& self) { auto read_buffer = asio::buffer(self.echo_buffer); self.socket.async_read_some( read_buffer, self.wrap()([self = std::move(self)](error_code ec, std::size_t bytes) mutable { if (!ec) { /*...*/ } else { self.call_handler(ec, bytes); } })); } void call_handler(error_code ec, std::size_t bytes) { /* Deallocate derived state resources */ this->call_handler_base(ec, bytes); } \endcode I still don't like the risk of moving `this` (or `self`) before members in the derived class are used to initiate the next operation (e.g. the echo_buffer). On Wed, Dec 19, 2018 at 10:24 PM Vinnie Falco wrote:
simply derive from it in your function-level class declaration to get all the hooks. Such a base class might look like this:
template<class Handler, class Derived> class operation_base { template<class T, class Executor> friend struct boost::asio::associated_executor; <snip> template<class Function> friend void asio_handler_invoke(Function&& f, operation_base* op) <snip> friend bool asio_handler_is_continuation(operation_base* op)
It looks like the base class doesn't even need these hooks, at least not directly. The object returned by asio::bind_executor and its equivalent for allocator, which doesn't exist yet, should have them. I'll take a shot at this in the next version of the example code. Best regards
On Thu, Dec 20, 2018 at 9:25 AM Sorin Fetche wrote:
On Wed, Dec 19, 2018 at 10:24 PM Vinnie Falco wrote:
Have you considered writing a base class which takes ownership of the user's completion handler and has the necessary hooks?
<snip>
Yes, the base class is a very good idea. It helps reduce the boilerplate in the composed operation and the risk of moving `this` before accessing things in it.
Here's how its usage looks like to implement the composed operation: https://github.com/sorf/cpp-playground/blob/master/source/asio_echo_v2.cpp#L...
<snip>
I still don't like the risk of moving `this` (or `self`) before members in the derived class are used to initiate the next operation (e.g. the echo_buffer).
With some additional work on the async state helper class, the example composed asynchronous operation becomes pretty compact: https://github.com/sorf/cpp-playground/blob/master/source/asio_echo_v3.cpp#L... \code template <typename StreamSocket, typename CompletionToken> auto async_echo_rw(StreamSocket &socket, CompletionToken &&token) -> /*...*/ { struct state_data { /*...*/}; using state_type = async_state</*...*/, state_data>; state_type state{std::move(completion.completion_handler), socket.get_executor(), socket, /*...*/}; state_data *data = state.get(); data->socket.async_read_some( asio::buffer(data->echo_buffer), state.wrap()([=, state = std::move(state)]( error_code ec, std::size_t bytes) mutable { if (!ec) { asio::async_write(data->socket, /*...*/); } else { state.invoke(ec, bytes); } })); return completion.result.get(); } \endcode The helper class async_state addresses now both the risk of accessing state after it has been moved and the using of the final handler to allocate the internal state.
From this perspective it is similar to beast::handler_ptr. It also uses a bind_allocator utility similar to asio::bind_executor - the thing that started this email thread.
I haven't tested it in depth yet to confirm that the final handler allocator and executor are properly used in the internal operations, but I would welcome feedback about this async_state helper class. Would it worth trying to turn it into some general purpose utility? Any caveats about asynchronous operations it left unaddressed? https://github.com/sorf/cpp-playground/blob/master/source/asio_echo_v3.cpp#L... Best regards, Sorin
participants (2)
-
Sorin Fetche -
Vinnie Falco