[Boost-users] ASIO: custom service which works with spawn/yield

[Cristian Morales Vega via Boost-users]

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.
_______________________________________________
Boost-users mailing list
Boost...@lists.boost.org
https://lists.boost.org/mailman/listinfo.cgi/boost-users

[Vinnie Falco via Boost-users]

On Wed, Jul 25, 2018 at 7:25 AM, Cristian Morales Vega via Boost-users
<boost...@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

[Cristian Morales Vega via Boost-users]

On 25 July 2018 at 17:13, Vinnie Falco via Boost-users

<boost...@lists.boost.org> wrote:
> On Wed, Jul 25, 2018 at 7:25 AM, Cristian Morales Vega via Boost-users
> <boost...@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".

[Vinnie Falco via Boost-users]

On Wed, Jul 25, 2018 at 7:25 AM, Cristian Morales Vega via Boost-users
<boost...@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>

[Cristian Morales Vega via Boost-users]

On 25 July 2018 at 19:12, Vinnie Falco via Boost-users

<boost...@lists.boost.org> wrote:
> On Wed, Jul 25, 2018 at 7:25 AM, Cristian Morales Vega via Boost-users
> <boost...@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>

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.

[Bjorn Reese via Boost-users]

On 07/25/18 19:12, Vinnie Falco via Boost-users wrote:

> It looks like your implementation of async_method() is missing one or
> two executor_work_guard objects:

post() is an asynchronous operation, so it takes care of that (see
section 13.23.)

[Vinnie Falco via Boost-users]

On Thu, Jul 26, 2018 at 5:33 AM, Bjorn Reese via Boost-users
<boost...@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.

[Vinnie Falco via Boost-users]

On Thu, Jul 26, 2018 at 4:23 AM, Cristian Morales Vega
<cris...@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

[Bjorn Reese via Boost-users]

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.

[Vinnie Falco via Boost-users]

On Thu, Jul 26, 2018 at 6:31 AM, Bjorn Reese via Boost-users
<boost...@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

[Bjorn Reese via Boost-users]

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.

[Vinnie Falco via Boost-users]

On Thu, Jul 26, 2018 at 7:23 AM, Bjorn Reese via Boost-users
<boost...@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

[Bjorn Reese via Boost-users]

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.

[Gavin Lambert via Boost-users]

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.)

[Vinnie Falco via Boost-users]

On Thu, Jul 26, 2018 at 6:03 PM, Gavin Lambert via Boost-users
<boost...@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

[james via Boost-users]

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. 

[Richard Hodges via Boost-users]

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.

[Vinnie Falco via Boost-users]

On Thu, Aug 2, 2018 at 8:24 AM, james <dirty...@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...@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

[Bjorn Reese via Boost-users]

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.)

[Vinnie Falco via Boost-users]

On Thu, Aug 2, 2018 at 1:02 PM, Richard Hodges <hodg...@gmail.com> wrote:
> I think it's worth mentioning that although your code is very well written
> and laid out, the horrid internals of asio mean that only ultra-experts can
> read it. It really doesn't 'flow'.
>
> Surely something can be done (in asio and networking TS) in terms of
> providing a cleaner, more logical interface for io_object and service
> developers?

I have thought about this for a few years and even made efforts to
provide algorithms and types to make authoring code easier, but they
were not successful. I don't think a "grand unifying theory" exists.
However, I also think there is a LOT of room for documentation to grow
and mature, and there is opportunity for authors to develop strong,
well commented examples (something I strive for in Beast).

With Networking TS as part of C++20 (or later), at last there will be
one correct interface for doing asynchrony. This will incentivize book
publishers and library authors to focus specifically on this
networking interface to provide educational resources.

In my Cppcon 2018 presentation I am also dedicating a couple of slides
with guidance on how to learn Asio effectively and write correct
programs.

Regards