[asio] using fork in coroutine composed operations
Hello all, my experiments with custom composed operations in asio have led me to another question regarding the use of such methods. What do I do in order to have multiple handlers in flight, e.g. for a timer? My example is here: https://github.com/MrMoose/moose_tools/blob/master/TimedConnect.hpp I'm trying to realize an old dream in asio. A function that will take a socket and asynchronously do a resolve and connect to the result while having a timeout around the operation. Here in the test cases (lower part) is how I imagine usage: https://github.com/MrMoose/moose_tools/blob/master/test/TestAsioHelpers.cpp#... This works (the unit test does...) but I think it's no good and illustrate the problem. In the coroutine impl I can safely jump in and out and resume the code represented by the coroutine object as was explained to me yesterday. But what can I do with the timer? I cannot std::move(self) into it because it gets moved again moments later and I cannot yield on the timer wait operation. I want the timer handler and the async ops handlers both be in flight and have called whichever completes first. I tried coroutine fork thinking it might be the right tool but that seems to clash with the move-self-ahead pattern which is used here. If I don't yield and set the timer to a fixed lambda (as in the example above) it seems to work but I guess that's not how it's supposed to be, right? Any hints as to how I can make this better? Cheers, Stephan
On Tue, Sep 3, 2019 at 7:22 AM Stephan Menzel via Boost-users <boost-users@lists.boost.org> wrote:
I'm trying to realize an old dream in asio. A function that will take a socket and asynchronously...connect to the result while having a timeout around the operation.
Like this? <https://www.boost.org/doc/libs/1_71_0/libs/beast/doc/html/beast/ref/boost__beast__basic_stream/async_connect/overload1.html> Implementation: <https://github.com/boostorg/beast/blob/b7230f12f16fe7a9f7a1ece5be1f607c8552448a/include/boost/beast/core/impl/basic_stream.hpp#L784> Regards
Hi Vinnie, Am Di., 3. Sept. 2019 um 16:23 Uhr schrieb Vinnie Falco < vinnie.falco@gmail.com>:
On Tue, Sep 3, 2019 at 7:22 AM Stephan Menzel via Boost-users <boost-users@lists.boost.org> wrote:
I'm trying to realize an old dream in asio. A function that will take a socket and asynchronously...connect to the result while having a timeout around the operation.
Like this?
< https://www.boost.org/doc/libs/1_71_0/libs/beast/doc/html/beast/ref/boost__b...
Well, not exactly. I wanted to do the resolve and connect in one go. This one has a timeout though, which already is a huge step in the right direction. Didn't know this existed. Anyway, your implementation did bring me to beast::bind_handler, which eventually led me to a working implementation: https://github.com/MrMoose/moose_tools/blob/master/TimedConnect.hpp as it enabled me to adopt the handler to async ops with different handler requirements. However, since I'm here ;-) may I ask a followup in this regard: I have had success with beast::bind_handler using it in timed_connect like this: template <typename Self> void operator()(Self &n_self, const boost::system::error_code &n_error = boost::system::error_code(), boost::asio::ip::tcp::resolver::results_type n_results = boost::asio::ip::tcp::resolver::results_type(), boost::asio::ip::tcp::endpoint n_connected_ep = boost::asio::ip::tcp::endpoint()) { reenter(m_coro) { .... yield boost::asio::async_connect(m_socket, n_results, boost::beast::bind_handler(std::forward<Self>(n_self), boost::placeholders::_1, boost::asio::ip::tcp::resolver::results_type(), boost::placeholders::_2)); .... }} Now, this is great as it was key to have placeholders and therefore combine async ops with incompatible handler types. However, I tried to adopt the exact same thing in my next operation, which shall wrap up a socks4 handshake. Like this template <typename Self> void operator()(Self &n_self, boost::system::error_code n_error = boost::system::error_code(), std::size_t = 0, boost::asio::ip::tcp::resolver::results_type n_results = boost::asio::ip::tcp::resolver::results_type()) { reenter(m_coro) { .... yield m_resolver.async_resolve(tcp::v4(), *m_target_hostname, *m_target_servicename, tcp::resolver::query::numeric_service, boost::beast::bind_handler(std::move(n_self), boost::placeholders::_1, 0, boost::placeholders::_2)); .... }} and in this context, it fails. I have tried many different variations and all I'm getting is crashes in the async_resolve operation that look like moving the handler fails at runtime. Or strange errors at runtime when the op returns with 995, which I suppose means a similar thing, e.g. m_resolver being destroyed Strangely it doesn't seem to matter which op I use the bind_handler with. There is a reolve, a write and a read. Each one will fail weirdly at runtime when I use the bind_handler to adapt the handler type. Of course, I change the signature for operator() for those trials. I strongly suspect it's got something to do with the moving that fails but there's no difference between this object and the one I use in TimedConnect which works great. Here's the data members: boost::asio::ip::tcp::socket &m_socket; boost::asio::ip::tcp::resolver m_resolver; std::unique_ptr<std::string> m_target_hostname; boost::uint16_t m_target_port; boost::asio::coroutine m_coro; Nothing that can't be moved.... Anyway, maybe change move errors when using bind_handler rings a bell. Thanks for your suggestions, Stephan
On 4/09/2019 02:21, Stephan Menzel wrote:
my experiments with custom composed operations in asio have led me to another question regarding the use of such methods. What do I do in order to have multiple handlers in flight, e.g. for a timer?
Not sure if this helps, but the way that I've done something similar in the past is simply to have different overloads of operator() for each unique operation signature. (Note that my example is for a sequential operation, not a parallel one, but in principle you should be able to do something similar.) For example: operator()() -> kicks off first async_wait operator()(error_code) -> handles result of async_wait and kicks off async_read operator()(error_code, size_t) -> handles result of async_read and kicks off another async_wait This doesn't actually use the coroutine mechanism since the actions are split between multiple methods rather than being multiplexed, but I think it still read pretty clearly this way. If you're switching between two operations that have the same signature (eg. async_read and async_write) then you'll need additional state in the handler object to determine which one was in flight; if you're trying to run them in parallel then you'll need to bind an extra handler argument to disambiguate which one completed first. Where your timer is being used as a timeout (so that you run them in parallel) things get more complex, since you have to deal with cancellation of the timer and/or the read (including coping with the case when you requested cancellation but it actually completed successfully before it could actually cancel; and not accidentally interpreting acknowledgment of cancelling your previous timer wait as a cancellation of a subsequent timer wait). But it should be doable.
Mere moments ago, quoth I:
Where your timer is being used as a timeout (so that you run them in parallel) things get more complex, since you have to deal with cancellation of the timer and/or the read (including coping with the case when you requested cancellation but it actually completed successfully before it could actually cancel; and not accidentally interpreting acknowledgment of cancelling your previous timer wait as a cancellation of a subsequent timer wait). But it should be doable.
Forgot to add: multiple operations concurrently in flight also means that you can't used the moved-handler lifetime model any more, and you'll probably need to switch to a shared-handler lifetime model instead. Or create separate sub-handler operation objects for each parallel action -- but in practice this will probably end up still requiring the original parent object to have a shared lifetime.
Am Mo., 9. Sept. 2019 um 02:31 Uhr schrieb Gavin Lambert via Boost-users < boost-users@lists.boost.org>:
On 4/09/2019 02:21, Stephan Menzel wrote:
my experiments with custom composed operations in asio have led me to another question regarding the use of such methods. What do I do in order to have multiple handlers in flight, e.g. for a timer?
Not sure if this helps, but the way that I've done something similar in the past is simply to have different overloads of operator() for each unique operation signature.
(Note that my example is for a sequential operation, not a parallel one, but in principle you should be able to do something similar.)
For example:
operator()() -> kicks off first async_wait operator()(error_code) -> handles result of async_wait and kicks off async_read operator()(error_code, size_t) -> handles result of async_read and kicks off another async_wait
This doesn't actually use the coroutine mechanism since the actions are split between multiple methods rather than being multiplexed, but I think it still read pretty clearly this way.
Ah, OK. I had tried this before but failed as I thought it is mandatory to have all parameters except Self defaulted. It says so in the example. Since I did this, the handler types collided. I have now implemented this approach in a socks connect function I had trouble with (see my other post) and it works fine without the coroutine: https://github.com/MrMoose/moose_tools/blob/master/SOCKS4.hpp Thanks! It was a great idea to try that again. If you're switching between two operations that have the same signature
(eg. async_read and async_write) then you'll need additional state in the handler object to determine which one was in flight; if you're trying to run them in parallel then you'll need to bind an extra handler argument to disambiguate which one completed first.
The coroutine approach sure looked nicer but having a little state machine for small composed ops like this doesn't hurt. As iong as it doesn't get much more complex we're fine. About your other message: Yes, when implementing the timeout the moving of the handler turned out to be the main reason the idea of forking gave me trouble. I was not aware shared ownership of the handler is an option though. My impl of the timed_connect works now but isn't very nice by and standard. Also, I'll be needing to include the timeout into the socks_connect as well so I will experiment with shared ownership of the handler and see where that takes me. Thanks! Cheers, Stephan
Hello,
For example:
operator()() -> kicks off first async_wait operator()(error_code) -> handles result of async_wait and kicks off async_read operator()(error_code, size_t) -> handles result of async_read and kicks off another async_wait
I sometimes use a single operator() with default values like this: void operator()(const boost::system::error_code &ec= boost::system::error_code(), size_t bytes_transferred=0) It can be called without an argument (for kick-off), with a single error_code (from async_wait) or with two argiments (from async_read). 73, Mario
On 9/09/2019 19:47, Klebsch, Mario wrote:
For example:
operator()() -> kicks off first async_wait operator()(error_code) -> handles result of async_wait and kicks off async_read operator()(error_code, size_t) -> handles result of async_read and kicks off another async_wait
I sometimes use a single operator() with default values like this:
void operator()(const boost::system::error_code &ec= boost::system::error_code(), size_t bytes_transferred=0)
It can be called without an argument (for kick-off), with a single error_code (from async_wait) or with two argiments (from async_read).
Yes, that's the method the OP is currently using. It requires using a coroutine or other state to determine which "step" you're up to, whereas with separate overloads this is inherent in which method is being called. Neither approach is necessarily "better" than the other; it depends what you're doing in there as to which one ends up being more readable.
Am Mo., 9. Sept. 2019 um 09:03 Uhr schrieb Gavin Lambert via Boost-users < boost-users@lists.boost.org>:
It can be called without an argument (for kick-off), with a single error_code (from async_wait) or with two argiments (from async_read).
Yes, that's the method the OP is currently using. It requires using a coroutine or other state to determine which "step" you're up to, whereas with separate overloads this is inherent in which method is being called.
Neither approach is necessarily "better" than the other; it depends what you're doing in there as to which one ends up being more readable.
Well, I got both implementations (async timed connect and socks4 handshake in place now and I do prefer the coroutine approach as it is much easier to read and I wanted to adapt this to more functions very soon. The problem really are the different handler signatures. Those default arguments are not a silver bullet by any means. I have managed to get it done using beast::bind_handler() in my timed_connect impl but pretty much the identical code failed entirely in the socks4 handshake. I can now only assume that using bind_handler() with the move_ahead pattern simply doesn't work and without any kind of bind_handler you will quickly run out of ways to order the arguments in operator() to fit all functions. In any case, I am glad I got the impl as far as this and that's a great step forward in re-using code in my projects. Thanks for all your suggestions, Stephan
On 9/09/2019 22:59, Stephan Menzel wrote:
Well, I got both implementations (async timed connect and socks4 handshake in place now and I do prefer the coroutine approach as it is much easier to read and I wanted to adapt this to more functions very soon.
FWIW, your timeout handler if-check is somewhat redundant. You're capturing the expiry by value (as you have to -- you can't capture it by reference since you're moving the lifetime of the timer), which means that it will always be less than the current time, unless the timer was aborted early due to some error other than operation_aborted. There isn't any code that can change the expiration time seen by the lambda callback (which the comment expresses as the reason for the check).
Am Di., 10. Sept. 2019 um 02:53 Uhr schrieb Gavin Lambert via Boost-users < boost-users@lists.boost.org>:
FWIW, your timeout handler if-check is somewhat redundant.
You're capturing the expiry by value (as you have to -- you can't capture it by reference since you're moving the lifetime of the timer), which means that it will always be less than the current time, unless the timer was aborted early due to some error other than operation_aborted.
There isn't any code that can change the expiration time seen by the lambda callback (which the comment expresses as the reason for the check).
Thanks for pointing this out. The code was copy-pasted around much throughout my efforts and I suppose the comment referred to some other place where I followed the pattern the asio examples do by re-using the same timeout again for different ops and reset the expiry time. I will correct this. You are right. Since it's only used once I don't need the check, so the comment is wrong. I'm in the process of creating more such composed ops for ever recurring use cases and perhaps I find a way to tie this lambdas lifetime to the whole operation. As it happens, my gut felling tells me there are cases when the timer lambda will dangle around and potentially crash, perhaps when the op is used and the io_service being destroyed and not polled before destruction. I couldn't verify this in the unit test yet but there's often cases like this that go overlooked. Cheers, Stephan
On 10/09/2019 17:10, Stephan Menzel wrote:
I'm in the process of creating more such composed ops for ever recurring use cases and perhaps I find a way to tie this lambdas lifetime to the whole operation. As it happens, my gut felling tells me there are cases when the timer lambda will dangle around and potentially crash, perhaps when the op is used and the io_service being destroyed and not polled before destruction. I couldn't verify this in the unit test yet but there's often cases like this that go overlooked.
Yes, you may have a lifetime problem if the timer has not yet expired when m_socket is destroyed. Since the owner of the timer also assumes a shorter lifetime than m_socket (due to capturing it by reference), the timer should get cancelled before m_socket is destroyed (unless you have a different bug). The handler might not have been called yet, but on the cancellation path you immediately return anyway, so it shouldn't be an issue. (Though there might be a corner case if it expires just as you destroy things.) If you do dispose of the io_service before all handlers are called, then they will not be called, but their destructors will still fire. Occasionally this means you need to be careful about the order of destruction when the destructors aren't trivial, but otherwise this behaves sensibly. But yes, async can make it quite hairy to verify that lifetimes occur the right way around -- which is one of the reasons why you see shared_ptr used a lot in async code. When you're trying to provoke these sorts of issues in unit tests, using run_one() can be very helpful.
On Mon, Sep 9, 2019 at 11:10 PM Stephan Menzel via Boost-users <boost-users@lists.boost.org> wrote:
I'm in the process of creating more such composed ops for ever recurring use cases and perhaps I find a way to tie this lambdas lifetime to the whole operation. As it happens, my gut felling tells me there are cases when the timer lambda will dangle around and potentially crash, perhaps when the op is used and the io_service being destroyed and not polled before destruction. I couldn't verify this in the unit test yet but there's often cases like this that go overlooked.
Yes, that can happen depending on your implementation. This is solved by tracking your objects in an execution_context::service and performing cleanup when the io_context is shut down, as beast::websocket::stream does: <https://github.com/boostorg/beast/blob/b7230f12f16fe7a9f7a1ece5be1f607c8552448a/include/boost/beast/websocket/detail/service.ipp#L46> Regards
participants (4)
-
Gavin Lambert -
Klebsch, Mario -
Stephan Menzel -
Vinnie Falco