asio_handler_invoke hook removal suspected of strand guarantee violations for intermediate completion handlers

Dear @chriskohlhoff and Asio maintainers,

First of all : thanks for the awesome work on the library, I never had to report anything in the 10years that I write code with asio every now and then 🙂. I'm still a non-expert user though, so please forgive if I'm completely off with this issue.

After removing code that was working thanks to asio_handler_invoke ADL hooks we started seeing failing assertions on strand.running_in_this_thread().

The documentation of Boost.Asio informed a while ago already users of the deprecated asio_handler_invoke ADLs hooks to change towards associated_executor traits to perform similar tasks.

As Boost 1.84.0 asio_handler_invoke ADL hooks were completely removed, we noticed by transitioning a codebase from Boost.Asio 1.81.0 to Boost.Asio 1.85.0 that the abstraction does not always easily get replaced and we believe that (the deprecated) strand.wrap has been broken by the removal.

The code here used to reproduce the issue has a handler_proxy_t type to proxy calls to actual handlers, this was used in C++03 code to centralize code for all Asio ADL hooks among other things. It is an highly simplified + anonymized version of production code.

Intermediate completion handlers were built thanks to this idiom and the ADL hooks with base class relationship would be called to handle proxying the call to the underlying handlers.

It happens that this Base class relationship doesn’t play well when switching from asio_handler_invoke to associated_executor, in the example we use an hack that is used to define the associated_executor traits for the proxied handlers to their respective executors ( See #1495 ).

After converting the code to remove asio_handler_invoke in favor of associated_executors, data races started to show and assertions checking on io_context::strand.running_in_this_thread to trigger, while it did previously work.

It happens that in the past, asio_handler_invoke_helpers::invoke was called by asio::wrapped_handler when handler_work did not detect any owning executor : https://github.com/chriskohlhoff/asio/blob/7609450f71434bdc9fbd9491a9505b423c2a8496/asio/include/asio/detail/handler_work.hpp#L518-L528

But it now changed to a plain call to function : https://github.com/chriskohlhoff/asio/blob/12e0ce9e0500bf0f247dbd1ae894272656456079/asio/include/asio/detail/handler_work.hpp#L464-L474

Indeed when the completion handlers were strand-wrapped ( i.e. io_context::strand.wrap() ) asio::detail::wrapped_handler had an asio_handler_invoke overload which would instantiate a rewrapped_handler and set it's dispatcher to the strand :

See former built-in asio_handler_invoke overload for wrapped_handler before asio-1-30-2 / Boost 1.84.0 : https://github.com/chriskohlhoff/asio/blob/7609450f71434bdc9fbd9491a9505b423c2a8496/asio/include/asio/detail/wrapped_handler.hpp#L239-L244

This had the effect of redirecting work to the strand of the strand-wrapped handler despite the handler_work not having any owning executor.

Together with the removal of asio_handler_invoke this built-in logic in asio::detail::wrapped_handler was removed as well and in my understanding breaks io_context::strand.wrap(), which now results in a violation of the strand guarantee. Such handler are called without interlocking.

As asio now performs a plain call to the function it doesn’t go through the rewrapping done by the built-in ADL anymore which was previously resulting in dispatching to the strand.

strand::wrap is marked deprecated, but the behaviour changed due to these missing asio-builtin ADL hooks. As it is not yet removed and only visible at runtime and difficult to spot there might be interest in either documenting or fixing it.

If one migrates strand.wrap calls to bind_executor(strand) the issue gets solved and running_in_this_thread() returns true again as the call is directly dispatched on the strand (taking the base1_type::dispatch else branch of the code quoted above). This works as the strand is put as an owning executor of the asio::detail::handler_work object.

As this seem to be a solution, possibly adding to the documentation of strand.wrap a link to bind_executor(strand) could be useful.

My understanding of the issue so far is that strand wrapped completion handler doesn't have the strand as bound executor by default, which perhaps should be the case ?

This might also be the intended behaviour and I might just be seeing a bug where there isn't, but migrating to a Boost.Asio post 1.84.0 and hence removing the ADL hooks and replacing with associated_executor traits we started to have these failing assertions on strand.running_in_this_thread() and hence the strand guarantees not being held anymore.

Hereafter is an example to reproduce the changed behaviour that can be compiled in both versions of Asio and with this example we can see that when asio_handlerinvoke is implemented the intermediate completion handler call to `assert(strand.running_in_this_thread());` pass, while otherwise they fail.

Boost 1.82.0 with asio_handler_invoke implemented (-DASIO_HANDLERINVOKE=1) passes `assert(strand.running_in_this_thread());`

Outputs :

running in this thread : 1
async_accept completed: system:0
ec: system:0 - 10
running in this thread : 1
ios.poll() n= 4

Boost 1.82.0 without asio_handler_invoke (-DASIO_HANDLERINVOKE=0) crashes on `assert(strand.running_in_this_thread());`

Outputs :

running in this thread : 1
async_accept completed: system:0
ec: system:0 - 10
running in this thread : 0
Assertion failed: (strand_.running_in_this_thread()), function operator(), file boost_asio_strandwrap.cpp, line 221.

Boost 1.85.0 without asio_handler_invoke (-DASIO_HANDLERINVOKE=0) crashes too on `assert(strand.running_in_this_thread());`

Boost.Asio version doesn't really matter, the problem is fully reproducible in older versions of Asio, but it's impossible to get the working state by using strand.wrap in newer versions as the built-in asio_handler_invoke doesn't exists anymore.

Outputs :

running in this thread : 1
async_accept completed: system:0
ec: system:0 - 10
running in this thread : 0
Assertion failed: (strand_.running_in_this_thread()), function operator(), file boost_asio_strandwrap.cpp, line 221.

chriskohlhoff / asio