On Fri, 7 Dec 2018 at 02:00, Gavin Lambert via Boost-users
<boost-users@lists.boost.org> wrote:
>
> On 7/12/2018 12:59, Damian Jarek wrote:
> > Here's an example of what might happen if a composed operation doesn't
> > maintain work guards properly:
> > https://wandbox.org/permlink/aqsGDNJWTmFd7PdC

Nice example!


> Why use conditional logic and extra storage in operator() when the
> compiler can do it for you?
>    https://wandbox.org/permlink/HzOlDt8S6txfLNB6
>
> > Without the work_guard the coroutine never completes. If you add the
> > work_guard, everything works correctly.
>
> I can see it happening in your example, but I still don't really grok
> why this occurs.
>
> Isn't the point of composed operations to ensure that they use the same
> executor for all handlers?  So it's the same executor as the underlying
> timer.  When it's in a wait operation, the timer should be taking care
> of it.  And while in the direct call context of operator() then the
> executor itself should know there is work in progress.
>
> So the only time where the work_guard should be having any effect is
> either if async_foo itself yields (which it does, but only after
> creating the op and making a call to async_wait, so that should keep it
> alive) or if the call to handler_ yields somewhere else (which it doesn't).
>
> (handler_() might internally post and yield rather than executing
> synchronously, especially cross-context, but in that case its executor
> should know that it's doing something.)
>
> So what am I missing?
>
> (I guess one of the things that I might be struggling with is that in Ye
> Olde Asio, as long as you always had an async_* in flight at all times
> then you never needed any io_service::work.  Usually that was easy
> because you typically have a listen or read in flight.)

If you look at https://www.boost.org/doc/libs/develop/doc/html/boost_asio/reference/asynchronous_operations.html
under "Outstanding work" it says it will keep the work guards "Until
the asynchronous operation has completed". That raises the question of
what "completed" means. It's explained at the top, it says "The
lifecycle of an asynchronous operation..."

— Phase 2: The asynchronous operation is now completed.
— Event 3: The completion handler is called with the result of the
asynchronous operation.

So, it looks like timer_ does keep a work guard for ctx1. But it
destroys it before we can call handler_(ec), before calling the
completion handler. I though it was not "completed" until after the
completion handler was called, but was wrong and that was what
confused me.

In that example:
- ctx2 calls timer_.async_wait(std::move(*this));
- the operation keeps work guards for ctx1 and ctx2 (not that the one
for ctx1 matters here, ctx1.run() has not yet been called)
- ctx1.run_for(std::chrono::seconds{5}) is called, the timer_ work
guards get destroyed and the timer_ completion handler ends up in ctx2
queue
- ctx1.run_for returns because it has no work
- ctx2 runs the completion handler, which ends up calling
timer_.async_wait again
- t1 is long gone and that async_wait will never complete

In https://wandbox.org/permlink/eNBzNmM2FbL4MHlE "T1 complete" is
printed before "Completion Handler called".

When there is only one thread involved "timer_ completion handler ends
up in ctx2 queue" is not true, the handler gets executed directly
without going through the queue. In those cases I guess the work guard
is likely to stay there until the completion handler has executed. But
having to go through the queue means the timer_ work guards disappear
too soon and you can't rely on them.


But... if I have got it right, then the echo_op example from Beast is
wrong, isn't it? It's doing the right thing keeping a work guard for
the AsyncStream associated executor. But it should *also* keep a work
guard for the handler associated executor.
Damian said: "The work guard is not necessary in such a case because
the operation at the bottom maintains a work guard for the handler's
executor", but that doesn't seem to be true... for long enough.