ASIO: custom service which works with spawn/yield
Hi, I have implemented a custom service using Boost 1.67 which works mostly OK, but I have noticed it always returns success when using spawn/yield. The following simplified sample shows the problem: ------------------------------------------------ #include <boost/asio.hpp> #include <boost/asio/spawn.hpp> #include <boost/beast/core/bind_handler.hpp> #include <iostream> class Service : public boost::asio::io_context::service { public: static boost::asio::io_context::id id; typedef int implementation_type; Service(boost::asio::io_context &io_context) : boost::asio::io_context::service(io_context) {} void construct(implementation_type &) {} void destroy(implementation_type &) {} }; boost::asio::io_context::id Service::id; template <typename Service> class BasicObject : protected boost::asio::basic_io_object<Service> { public: explicit BasicObject(boost::asio::io_context &io_context) : boost::asio::basic_io_object<Service>(io_context) {} template <typename ConnectHandler> auto async_method(ConnectHandler &&handler) { BOOST_ASIO_CONNECT_HANDLER_CHECK(ConnectHandler, handler) typeCheck; boost::asio::async_completion<ConnectHandler, void(const boost::system::error_code &)> init(handler); boost::system::error_code ec = boost::asio::error::already_started; boost::asio::post( boost::beast::bind_handler(std::move(init.completion_handler), std::move(ec))); return init.result.get(); } }; int main() { boost::asio::io_context ioContext; BasicObject<Service> myObject(ioContext); #if 1 boost::asio::spawn(ioContext, [&](boost::asio::yield_context yield) { boost::system::error_code ec; myObject.async_method(yield[ec]); std::clog << ec << std::endl; }); #else myObject.async_method([](const boost::system::error_code &ec) { std::clog << ec << std::endl; }); #endif ioContext.run(); return 0; } ------------------------------------------------ The version not using spawn prints "system:114", the one using spawn prints "system:0". What am I doing wrong? Thanks.
On Wed, Jul 25, 2018 at 7:25 AM, Cristian Morales Vega via Boost-users <boost-users@lists.boost.org> wrote:
boost::system::error_code ec = boost::asio::error::already_started; boost::asio::post( boost::beast::bind_handler(std::move(init.completion_handler), std::move(ec)));
Two things you can try: 1. Don't use move on ec: boost::system::error_code ec = boost::asio::error::already_started; boost::asio::post(boost::beast::bind_handler( std::move(init.completion_handler), ec); 2. Try the tip of the Beast master branch (or the 1.68 beta). In particular, this change: <https://github.com/boostorg/beast/commit/f948c9cbe57552f2a2b1306235ba578584b1d2c0> Thanks
On 25 July 2018 at 17:13, Vinnie Falco via Boost-users <boost-users@lists.boost.org> wrote:
On Wed, Jul 25, 2018 at 7:25 AM, Cristian Morales Vega via Boost-users <boost-users@lists.boost.org> wrote:
boost::system::error_code ec = boost::asio::error::already_started; boost::asio::post( boost::beast::bind_handler(std::move(init.completion_handler), std::move(ec)));
Two things you can try:
1. Don't use move on ec:
boost::system::error_code ec = boost::asio::error::already_started; boost::asio::post(boost::beast::bind_handler( std::move(init.completion_handler), ec);
2. Try the tip of the Beast master branch (or the 1.68 beta). In particular, this change:
<https://github.com/boostorg/beast/commit/f948c9cbe57552f2a2b1306235ba578584b1d2c0>
Unfortunately it makes no difference. Actually even using "std::bind(std::move(init.completion_handler), ec);" I still get "system:0".
On Wed, Jul 25, 2018 at 7:25 AM, Cristian Morales Vega via Boost-users <boost-users@lists.boost.org> wrote:
The following simplified sample shows the problem:
First of all thank you for posting a minimal, complete, and verifiable example. It looks like your implementation of async_method() is missing one or two executor_work_guard objects: Per N4734[1] 13.2.7.10 Outstanding Work [async.reqmts.async.work] 1. Until the asynchronous operation has completed, the asynchronous operation shall maintain: (1.1) — an object work1 of type executor_work_guard, initialized as work1(ex1), and where work1.owns_work() == true; and (1.2) — an object work2 of type executor_work_guard, initialized as work2(ex2), and where work2.owns_work() == true. I don't think you can get away with just calling `post` from `async_method` you need to write a "real" composed operation class with all of the bells and whistles including forwarding of the associated executor and associated allocator. In this class you can store the two required executor_work_guard objects as data members. A tutorial for writing composed operations may be found here: <https://www.boost.org/doc/libs/1_67_0/libs/beast/doc/html/beast/using_io/writing_composed_operations.html> Thanks [1] <http://cplusplus.github.io/networking-ts/draft.pdf>
On 25 July 2018 at 19:12, Vinnie Falco via Boost-users <boost-users@lists.boost.org> wrote:
On Wed, Jul 25, 2018 at 7:25 AM, Cristian Morales Vega via Boost-users <boost-users@lists.boost.org> wrote:
The following simplified sample shows the problem:
First of all thank you for posting a minimal, complete, and verifiable example.
It looks like your implementation of async_method() is missing one or two executor_work_guard objects:
Per N4734[1] 13.2.7.10 Outstanding Work [async.reqmts.async.work]
1. Until the asynchronous operation has completed, the asynchronous operation shall maintain:
(1.1) — an object work1 of type executor_work_guard, initialized as work1(ex1), and where work1.owns_work() == true; and
(1.2) — an object work2 of type executor_work_guard, initialized as work2(ex2), and where work2.owns_work() == true.
I don't think you can get away with just calling `post` from `async_method` you need to write a "real" composed operation class with all of the bells and whistles including forwarding of the associated executor and associated allocator. In this class you can store the two required executor_work_guard objects as data members.
A tutorial for writing composed operations may be found here:
Thanks
13.2.7.1 says The life cycle of an asynchronous operation is comprised of the following events and phases: (2.1) — Event 1: The asynchronous operation is started by a call to the initiating function. (2.2) — Phase 1: The asynchronous operation is now outstanding. (2.3) — Event 2: The externally observable side effects of the asynchronous operation, if any, are fully established. The completion handler is submitted to an executor. (2.4) — Phase 2: The asynchronous operation is now completed. By this definition of "completed" shouldn't my sample code be fine? This is not supposed to be a composed operation. In any case I was looking at basic_socket::async_connect() and I noticed I was using async_completion differently. After changing "boost::asio::async_completion<ConnectHandler, void(const boost::system::error_code &)>" for "boost::asio::async_completion<ConnectHandler, void(boost::system::error_code)>" (error_code by value, not const reference) my sample code has started working correctly. I'm not sure I *really* understand what was going on (or if this couldn't have been detected at compile time). But... it kind of makes sense.
On Thu, Jul 26, 2018 at 4:23 AM, Cristian Morales Vega <cristian@samknows.com> wrote:
By this definition of "completed" shouldn't my sample code be fine?
Actually, running the code under the Visual Studio debugger, I realize now that there are multiple bugs. First, there is the incorrect signature passed to async_completion as you noted. Second, you are using the wrong overload of post. Your code should read: boost::asio::post( this->get_executor(), boost::beast::bind_handler(std::move(init.completion_handler), ec)); The consequence of using the 1-argument version of `post` is that your completion handler will run on a system_executor rather than the executor associated with the I/O service of your object. This can be easily verified in the debugger: <https://i.imgur.com/BsZYgHM.png> And finally there is the original problem where there is no executor_work_guard for the io_context.
This is not supposed to be a composed operation.
You're right about that, and my apologies for the confusing terminology. I use "composed operation" to also refer to a particular style of implementation where a class is written as an invocable function object and is its own completion handler (disclaimer, untested code): template <class Handler> class composed_op { io_context& ioc_; Handler h_; executor_work_guard <io_context::executor_type> w_; public: template <class Deduced> composed_op (io_context& ioc, Deduced&& h) : ioc_ (ioc) : h_ (std::forward<Deduced>(h)) , w_ (ioc) { boost::asio::post(ioc_, std::move(*this)); } using allocator_type = boost::asio::associated_allocator_t<Handler>; allocator_type get_allocator() const noexcept { return (boost::asio::get_associated_allocator)(h_); } using executor_type = boost::asio::associated_executor_t<Handler, io_context::executor_type>; executor_type get_executor() const noexcept { return (boost::asio::get_associated_executor)(h_, ioc_.get_executor()); } void operator()() { h(); } }; I know, its a lot. Thanks
On Thu, Jul 26, 2018 at 5:33 AM, Bjorn Reese via Boost-users <boost-users@lists.boost.org> wrote:
On 07/25/18 19:12, Vinnie Falco via Boost-users wrote: post() is an asynchronous operation, so it takes care of that (see section 13.23.)
I'm certain that the executor_work_guard is needed, this guidance comes from Chris. The note in 13.23 is non-normative and probably misleading in this context. I'll raise the point with the editor.
On 07/26/18 15:12, Vinnie Falco via Boost-users wrote:
I'm certain that the executor_work_guard is needed, this guidance comes from Chris. The note in 13.23 is non-normative and probably misleading in this context. I'll raise the point with the editor.
I was referring to 13.23 in its entirety, not just the non-normative note at the beginning. Specifically, look at paragraph 13.23/6.2.2.
On Thu, Jul 26, 2018 at 6:31 AM, Bjorn Reese via Boost-users <boost-users@lists.boost.org> wrote:
I was referring to 13.23 in its entirety, not just the non-normative note at the beginning. Specifically, look at paragraph 13.23/6.2.2.
I'm still certain that an executor_work_guard is needed. 13.23/6.2.2 specifies that `post` only constructs the work guard for the completion handler's associated executor. It says nothing about a work guard for the executor associated with the I/O object, see 13.2.7.8/2.1 and 13.2.7.10/1.1 You're right that `post` satisfies 13.2.7.10/1.2 Thanks
On 07/26/18 15:42, Vinnie Falco wrote:
I'm still certain that an executor_work_guard is needed. 13.23/6.2.2 specifies that `post` only constructs the work guard for the completion handler's associated executor. It says nothing about a work guard for the executor associated with the I/O object, see 13.2.7.8/2.1 and 13.2.7.10/1.1
The post operation is handed off to the executor (13.23/6.3) so I would assume that the definition of "outstanding work" in 14.2 rather than 13.2.7.10 applies.
On Thu, Jul 26, 2018 at 7:23 AM, Bjorn Reese via Boost-users <boost-users@lists.boost.org> wrote:
The post operation is handed off to the executor (13.23/6.3) so I would assume that the definition of "outstanding work" in 14.2 rather than 13.2.7.10 applies.
Right, so we know that `post` will construct executor_work_guard for the executor associated with a completion handler. And we know that the asynchronous operation is then responsible for constructing the executor_work_guard for the executor associated with the I/O object (in this case it is an object of type io_context::executor_type). If the completion handler has no associated executor, for example if it is a lambda or a bind wrapper, then the executor of the I/O context will be used (assuming the 2-arg version of `post` is called, which is required for correctness), and everything will seem to work even without the second work guard. However, if the completion handler has an associated executor, then omitting the second work guard results in an io_context with no outstanding work. Therefore, io_context::run returns immediately, and the completion handler is not invoked. This is what I see in Visual Studio. Linux users may see a different result on account of some implementation differences. Stackful coroutines launched with `spawn` have an explicit `strand`, which means the associated executor is NOT the same as the executor for the I/O context upon which the strand was launched. Therefore, omitting the executor_work_guard from an asynchronous operation which only calls `post` will not meet the asynchronous requirements for the case where the completion handler is a stackful coroutine. If anything, this just highlights the complexity of [networking.ts]. Users will definitely be making these mistakes repeatedly, especially the one where the 1-arg version of `post` is called instead of the 2-arg version. Thanks
On 27/07/2018 02:32, Vinnie Falco wrote:
If anything, this just highlights the complexity of [networking.ts]. Users will definitely be making these mistakes repeatedly, especially the one where the 1-arg version of `post` is called instead of the 2-arg version.
This raises a concern that I've had for a while; if it's this hard to implement asynchronous operations correctly, I can't see it being very successful in practice. If you look at other languages and frameworks, once you introduce fundamental asynchronous operations they then quickly "infect" higher layer operations that want to be asynchronously composed of other operations, and so forth, until even the top-most UI layer wants to be largely asynchronous to avoid blocking the main event loops. "Async all the way down." This means that everybody, including general application developers, needs to be able to easily write asynchronous methods, without having to worry about falling into these pitfalls. (It needs to be a "pit of success", not requiring several code reviews by experienced library developers in order to spot the mistakes.)
On Thu, Jul 26, 2018 at 6:03 PM, Gavin Lambert via Boost-users <boost-users@lists.boost.org> wrote:
This raises a concern that I've had for a while; if it's this hard to implement asynchronous operations correctly, I can't see it being very successful in practice. ... This means that everybody, including general application developers, needs to be able to easily write asynchronous methods, without having to worry about falling into these pitfalls. (It needs to be a "pit of success", not requiring several code reviews by experienced library developers in order to spot the mistakes.)
I feel you. But I do not see any obvious improvements or alternatives to the current design of [networking.ts], which provide the same or greater levels of expressive power and performance. There is nothing "easy" about writing concurrent asynchronous code in C++, which I believe is a consequence of having zero-cost abstractions. The following is an excerpt from a recent paper of mine: "From the author's experience and the visible cascade of problems with the example code in this paper, writing initiating functions and composed operations correctly requires a demanding amount of experience and skill. This is especially true when considering that such algorithms must also implicitly exhibit correct behavior in multi-threaded environments. This is accomplished by achieving a deep and thorough understanding of Asio or [networking.ts] and applying that understanding to code. The author and contributors to Boost.Beast have explored library solutions to provide some correct boilerplate and higher level idioms to users in order to make authoring composed operations easier to write. A little bit of progress has been made on this front, but comprehensive improvements have been elusive. We suspect that grand unifying solutions simply do not exist, and that the level of complexity is inherent to the domain." Regards
On Thu, Aug 2, 2018 at 2:44 PM, Vinnie Falco via Boost-users < boost-users@lists.boost.org> wrote:
On Thu, Jul 26, 2018 at 6:03 PM, Gavin Lambert via Boost-users <boost-users@lists.boost.org> wrote:
This raises a concern that I've had for a while; if it's this hard to implement asynchronous operations correctly, I can't see it being very successful in practice. ... This means that everybody, including general application developers, needs to be able to easily write asynchronous methods, without having to worry about falling into these pitfalls. (It needs to be a "pit of success", not requiring several code reviews by experienced library developers in order to spot the mistakes.)
I feel you. But I do not see any obvious improvements or alternatives to the current design of [networking.ts], which provide the same or greater levels of expressive power and performance. There is nothing "easy" about writing concurrent asynchronous code in C++, which I believe is a consequence of having zero-cost abstractions. The following is an excerpt from a recent paper of mine:
"From the author's experience and the visible cascade of problems with the example code in this paper, writing initiating functions and composed operations correctly requires a demanding amount of experience and skill. This is especially true when considering that such algorithms must also implicitly exhibit correct behavior in multi-threaded environments. This is accomplished by achieving a deep and thorough understanding of Asio or [networking.ts] and applying that understanding to code. The author and contributors to Boost.Beast have explored library solutions to provide some correct boilerplate and higher level idioms to users in order to make authoring composed operations easier to write. A little bit of progress has been made on this front, but comprehensive improvements have been elusive. We suspect that grand unifying solutions simply do not exist, and that the level of complexity is inherent to the domain."
Networking is complex? Not necessarily so. ASIO is confusing and over-engineered, and I bet there's a lot of identical boilerplate code out that has "this works, don't touch it" comments. Networking.TS has unfortunately tied itself to executors, there was no need for that. To be fair, it's probably easier to use libuv and put up with the loss of type safety. At least you'll be able to set timeouts properly.
On Thu, 2 Aug 2018 at 17:25, james via Boost-users < boost-users@lists.boost.org> wrote:
On Thu, Aug 2, 2018 at 2:44 PM, Vinnie Falco via Boost-users < boost-users@lists.boost.org> wrote:
On Thu, Jul 26, 2018 at 6:03 PM, Gavin Lambert via Boost-users <boost-users@lists.boost.org> wrote:
This raises a concern that I've had for a while; if it's this hard to implement asynchronous operations correctly, I can't see it being very successful in practice. ... This means that everybody, including general application developers, needs to be able to easily write asynchronous methods, without having to worry about falling into these pitfalls. (It needs to be a "pit of success", not requiring several code reviews by experienced library developers in order to spot the mistakes.)
I feel you. But I do not see any obvious improvements or alternatives to the current design of [networking.ts], which provide the same or greater levels of expressive power and performance. There is nothing "easy" about writing concurrent asynchronous code in C++, which I believe is a consequence of having zero-cost abstractions. The following is an excerpt from a recent paper of mine:
"From the author's experience and the visible cascade of problems with the example code in this paper, writing initiating functions and composed operations correctly requires a demanding amount of experience and skill. This is especially true when considering that such algorithms must also implicitly exhibit correct behavior in multi-threaded environments. This is accomplished by achieving a deep and thorough understanding of Asio or [networking.ts] and applying that understanding to code. The author and contributors to Boost.Beast have explored library solutions to provide some correct boilerplate and higher level idioms to users in order to make authoring composed operations easier to write. A little bit of progress has been made on this front, but comprehensive improvements have been elusive. We suspect that grand unifying solutions simply do not exist, and that the level of complexity is inherent to the domain."
Networking is complex? Not necessarily so. ASIO is confusing and over-engineered, and I bet there's a lot of identical boilerplate code out that has "this works, don't touch it" comments.
In my view, asio suffers from: a) incredibly terse documentation (it needs a lot more human 'chat' about 'why we do it this way', 'how we got here' and 'what this is actually trying to achieve') b) No documentation at all for writing your own services which execute handlers properly - it's easy to write services 'wrong' (i.e. they end up posting a call to a functor) but I've never managed to write a service which works correctly with asio::use_future or a coroutine - it's just too much work to unpick all the non-documented detail in Chris's incredible work! This is a terrible shame as in all other respects I consider ASIO to me a masterpiece.
Networking.TS has unfortunately tied itself to executors, there was no need for that.
To be fair, it's probably easier to use libuv and put up with the loss of type safety. At least you'll be able to set timeouts properly.
Sadly true until someone documents how to write a service properly. And I don't mean a trivial one as in the boost examples. A proper service which hands off work to a another thread and then marshals results back to the initiating object's executor.
_______________________________________________ Boost-users mailing list Boost-users@lists.boost.org https://lists.boost.org/mailman/listinfo.cgi/boost-users
On Thu, Aug 2, 2018 at 8:24 AM, james <dirtydroog@gmail.com> wrote:
Networking is complex? Not necessarily so.
It is not the networking that is complex but rather, the asynchronous model. In order to program it effectively, authors must have a deep understanding of the following: * Associated executors and associated allocators * Initiating function return type customization (i.e. async_result) * 13.2.7 Requirements on asynchronous operations [async.reqmts.async] * Implicit versus explicit strands
ASIO is confusing and over-engineered
It could probably use better documentation but I disagree with being over-engineered. Take away anything from Asio (and consequently, Networking TS) and the result is less functionality for users.
Networking.TS has unfortunately tied itself to executors, there was no need for that.
You have it backwards; the asio_handler_invoke mechanism was the basis of executors, and predates executors by many years. Executors TS is a refinement of the original Asio mechanism for specifying the algorithm used to invoke a function object.
To be fair, it's probably easier to use libuv and put up with the loss of type safety.
That is an apples to oranges comparison. libuv is an implementation while Networking TS is the specification of an implementation. There are no independent implementations of libuv, because libuv is not a standard. Furthermore libuv is written in C and therefore offers little to nothing in terms of abstractions. While Networking TS offers very powerful abstractions, such as: * ConstBufferSequence, MutableBufferSequence, DynamicBuffer * SyncReadStream, SyncWriteStream * AsyncReadStream, AsyncWriteStream * ...much more The benefit of these abstractions is that authors can more precisely describe and achieve their intent with respect to implementation.
At least you'll be able to set timeouts properly.
Unlike libuv, Networking TS does not make odd choices on behalf of the user. Important decisions such as how to allocate memory or how to implement timeouts can be decided by the application. There are many ways to implements timeouts, but libuv offers only one while Asio gives you the tools to implement them in any fashion. For example here is an asychronous stream wrapper which implements read timeouts transparently, and does so very efficiently: It only uses a single timer and a single thread: <https://github.com/vinniefalco/beast/blob/65f08791d0d7310a1427ad5d2bdd913287643d0d/include/boost/beast/experimental/core/timed_socket.hpp> <https://github.com/vinniefalco/beast/blob/65f08791d0d7310a1427ad5d2bdd913287643d0d/include/boost/beast/experimental/core/impl/timed_socket.ipp> <https://github.com/vinniefalco/beast/blob/65f08791d0d7310a1427ad5d2bdd913287643d0d/include/boost/beast/experimental/core/detail/timed_socket_service.hpp> Test: <https://github.com/vinniefalco/beast/blob/65f08791d0d7310a1427ad5d2bdd913287643d0d/test/beast/experimental/timed_socket.cpp> On Thu, Aug 2, 2018 at 9:14 AM, Richard Hodges via Boost-users <boost-users@lists.boost.org> wrote:
A proper service which hands off work to a another thread and then marshals results back to the initiating object's executor.
See above. Thanks
On 08/02/18 17:24, james via Boost-users wrote:
Networking.TS has unfortunately tied itself to executors, there was no need for that.
Can you elaborate Are you talking about the split between io_context and executors?
To be fair, it's probably easier to use libuv and put up with the loss of type safety. At least you'll be able to set timeouts properly.
This is surprising observation, because I have the exact opposite experience. Can you elaborate? When I compare my Asio and libuv wrappers, the libuv timer code is more complex because the cancel operation needs to do a deferred deletion. The rest of the wrappers are roughly comparable in complexity (if we ignore all the type-casting needed by the libuv wrapper.)
On 07/26/18 16:32, Vinnie Falco wrote:
If anything, this just highlights the complexity of [networking.ts]. Users will definitely be making these mistakes repeatedly, especially the one where the 1-arg version of `post` is called instead of the 2-arg version.
I wholeheartedly agree with this.
participants (1)
-
Bjorn Reese -
Cristian Morales Vega -
Gavin Lambert -
james -
Richard Hodges -
Vinnie Falco