Skip to content

Feature request: Type-erased handler wrapper #1100

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ecorm opened this issue Jul 27, 2022 · 34 comments
Closed

Feature request: Type-erased handler wrapper #1100

ecorm opened this issue Jul 27, 2022 · 34 comments

Comments

@ecorm
Copy link

ecorm commented Jul 27, 2022

We need a wrapper that type-erases an asynchronous handler and bundles its associated executor and allocator. This would prevent the forcing of higher-level networking libraries to be header-only. It would also allow such higher-level networking libraries to expose known types in its API, instead of templatized function arguments.

This proposed type-erased handler could be passed to boost::asio::post and the like. It would also have to support move-only underlying handlers.

Some of us still care about compile times and being able to apply the Pimpl idiom. This is not possible when our libraries are forced to be header-only because of Asio.

I have found such a wrapper in the wild at https://github.com/inetic/asio-utp/blob/master/include/asio_utp/detail/handler.hpp . However it lacks small buffer optimization and has additional stuff that seems specific to that library.

@ecorm
Copy link
Author

ecorm commented Jul 27, 2022

It would also allow such higher-level networking libraries to expose known types in its API, instead of templatized function arguments.

Upon further thought, the API would still need to expose template functions taking any completion token so that it may perform an asio::async_initiate. The intermediate handler passed to the initiating function could then be moved into the proposed any_handler and passed to a private implementation function that can be defined in a cpp file and thus be compiled.

@mabrarov
Copy link

mabrarov commented Jul 28, 2022

Could you please take a look at https://github.com/mabrarov/asio_samples/blob/master/libs/ma_handler_storage/include/ma/handler_storage.hpp (https://github.com/mabrarov/asio_samples/blob/master/tests/ma_handler_storage_test/src/handler_storage_test.cpp) - if it does the same as you mentioned? I understand, that it’s pre-Network / Executors TS, i.e. doesn’t deal with Asio executors, allocators and completion tokens (but supports Asio legacy custom handler allocation and execution). I believe, there was discussion about the same somewhere here or in mailing list (cannot find it right now) and that discussion had an outcome (maybe new feature in Asio). I feel like my intention around ma::handler_storage class is exactly the same as described in this issue.

@mabrarov
Copy link

Here is that discussion (idea is close to this topic, but described from another point of view / use cases): #891

@chriskohlhoff
Copy link
Owner

chriskohlhoff commented Aug 4, 2022

I hacked together a prototype on this branch:

https://github.com/chriskohlhoff/asio/tree/any-completion-handler

Example found here:

https://github.com/chriskohlhoff/asio/tree/any-completion-handler/asio/src/examples/cpp20/type_erasure

(The implementation should work on C++11 or later btw, even though the example is for C++20.)

@klemens-morgenstern
Copy link
Contributor

I've written that one over here: https://github.com/madmongo1/asio_experiments/blob/master/include/asioex/async_op.hpp

Also containing a type-erased op

@ecorm
Copy link
Author

ecorm commented Aug 4, 2022

Example found here:

That example covers most of my intended usages nicely. Thanks!

Another use case is handlers that are stored for a long duration and executed whenever an event occurs. Such handlers need to be executed multiple times, and must therefore be copyable when passed to asio::post and the like. For example, an RPC handler that performs the work of the remote procedure and sends back the result. Perhaps a wrapper that couples a std::function with a asio::execution::any_executor would be more appropriate in this case.

I've cobbled my own type-erased handler (still in a development branch) in the interim, and was faced with these challenges:

  • Supporting both copyable and one-shot move-only handler functions.
  • Using the wrapper for two purposes: one-shot completion handlers, as well as repeatable event handlers that need to be executed via a user's executor.
  • When using use_future, the result of asio::get_associated_executor cannot be stored in an asio::any_io_executor. I had to use asio::execution::any_executor<asio::execution::blocking_t::never_t>>.
  • Being able to pass the type-erased handler to lambda functions in C++11 where move capture is not supported.

@ecorm
Copy link
Author

ecorm commented Aug 4, 2022

@chriskohlhoff Would it be possible to add an any_completion_handler constructor overload that takes a fallback executor? Something like:

  template <typename H, typename E, typename Handler = typename decay<H>::type>
  any_completion_handler(H&& h, E&& e)
      : fn_table_(&detail::any_completion_handler_fn_table_instance<Handler, Signatures...>::value),
      impl_(detail::any_completion_handler_impl<Handler>::create(
          (get_associated_executor)(h, std::forward<E>(e)),
          (get_associated_cancellation_slot)(h), std::forward<H>(h)))
  {
  }

@ecorm
Copy link
Author

ecorm commented Aug 4, 2022

Would it be possible to add an any_completion_handler constructor overload that takes a fallback executor?

@chriskohlhoff I think I can (and probably should) refactor my code so that the fallback executor is passed at the time of posting the handler, instead of being bound early to the any_completion_handler.

@ecorm
Copy link
Author

ecorm commented Aug 5, 2022

@chriskohlhoff I'm getting compile errors when doing:

boost::asio::post( exec, std::bind(std::move(f), std::forward<Ts>(args)...) );

where f is an instance of any_completion_handler<void (MyArg)>.

I'll write a minimal example that duplicates the problem, but in the meantime, is what I'm attempting even possible?

@ecorm
Copy link
Author

ecorm commented Aug 5, 2022

Narrowed down the problem to std::result_of<any_completion_handler<Sig>> not working in the std::bind implementation. Now looking to see whether std::result_of can be extended to user types.

@chriskohlhoff
Copy link
Owner

chriskohlhoff commented Aug 5, 2022

bind doesn't like the rvalue reference qualifier on operator(). I've removed this and pushed an update to the branch (with some other fixes, including the use_future problem).

On the requirement for multi-shot handlers, a while back I extended async_result to support reference qualification on completion signatures for this exact reason, however I haven't really done anything with them yet. For example:

  • void(error_code) -> current behaviour: one-shot, calling consumes the completion handler
  • void(error_code) & -> multi-shot, calling does not consume handler, implies copyability
  • void(error_code) && -> synonym for one-shot

any_completion_handler could be made to support this, and automatically apply move-only or copyable semantics accordingly. (Note that dispatching on to the correct executor would still be the responsibility of the caller of the completion handler, as it is now.)

@ecorm
Copy link
Author

ecorm commented Aug 5, 2022

On the requirement for multi-shot handlers

This was easy enough for me to implement via a wrapper class containing both std::function<Signature> and asio::execution::any_executor (still needs troubleshooting, though). Having an Asio-provided one isn't as high-prority as the one-shot consuming one you kindly provided the prototype for.

@ecorm
Copy link
Author

ecorm commented Aug 6, 2022

@chriskohlhoff I'm finally out of compile error hell due to the necessity to move any_completion_handler everwhere (and having to revert back to old-school function objects due to the lack of move-captures in C++11). I now get the following linker error:

any_completion_handler.hpp:302: error: undefined reference to `boost::asio::detail::any_completion_handler_fn_table_instance<boost::asio::detail::promise_handler<void (wamp::ErrorOr<unsigned long>), std::allocator<void> >, void (wamp::ErrorOr<unsigned long>)>::value'

The problem is due to this:

template <typename Handler, typename... Signatures>
struct any_completion_handler_fn_table_instance
{
  static constexpr any_completion_handler_fn_table<Signatures...>
    value = any_completion_handler_fn_table<Signatures...>(
        &any_completion_handler_destroy_fn::impl<Handler>,
        &any_completion_handler_executor_fn::impl<Handler>,
        &any_completion_handler_call_fn<Signatures>::template impl<Handler>...);
};

where static constexpr member variables are not implicity inline in C++11. I think it should be replaced with a static constexpr function.

@ecorm
Copy link
Author

ecorm commented Aug 6, 2022

I think it should be replaced with a static constexpr function.

No, that didn't work as static variables are not permitted in constexpr functions. I ended up adding this in the same any_completion_handler.hpp header file:

template <typename H, typename... Ss>
constexpr any_completion_handler_fn_table<Ss...>
  any_completion_handler_fn_table_instance<H, Ss...>::value;

I'm not sure whether this is safe, because different translation units will have different tables.

@ecorm
Copy link
Author

ecorm commented Aug 7, 2022

@chriskohlhoff , my CppWAMP code (not yet pushed) is now working with a "boostified" version of your any_completion_handler with the following modifications:

  • Replaced any_io_executor with asio::execution::any_executor<asio::execution::blocking_t::never_t> due to the use_future token not working with any_io_executor (while still being able to use previous releases of Boost.Asio with the unfixed promise_executor).
  • Added the missing any_completion_handler_fn_table_instance definition required for pre-C++17.

Thanks again for writing the any_completion_handler prototype, and for your prompt fixes.

Is there a way I can non-intrusively "fix" promise_executor so that I can revert back to the any_completion_handler that uses any_io_executor?

For that matter, why must it be any_io_executor that type-erases the executor? Why impose that executors for handlers have an I/O context?

May I suggest that a new any_completion_executor type alias be introduced for use with any_completion_handler?

@ecorm
Copy link
Author

ecorm commented Aug 7, 2022

@chriskohlhoff Other finishing touches to your any_completion_handler would be to make it more std::function-like:

  • Default constructible
  • explicit operator bool
  • nullptr_t support: construction, assignment, comparison
  • swap
  • associated_allocator and associated_cancellation_slot support (if applicable)

@chriskohlhoff
Copy link
Owner

For that matter, why must it be any_io_executor that type-erases the executor? Why impose that executors for handlers have an I/O context?

May I suggest that a new any_completion_executor type alias be introduced for use with any_completion_handler?

You're correct, and this is something i've been planning too, so I've added it to the branch. (However, before merging this I want to investigate relaxing the requirements on a completion executor some more, and that requires some further thought.)

I have also added the other things you mentioned (except cancellation which was already there).

@ecorm
Copy link
Author

ecorm commented Aug 9, 2022

I have also added the other things you mentioned

Awesome!

There is no asio/impl/any_completion_executor.ipp file (which is included by any_completion_executor.hpp) in the any_completion_branch. Did you forget to add it to the repo?

@chriskohlhoff
Copy link
Owner

So I did. Fixed.

@ecorm
Copy link
Author

ecorm commented Aug 9, 2022

In the absence of asio/impl/any_completion_executor.ipp, I tried defining BOOST_ASIO_USE_TS_EXECUTOR_AS_DEFAULT, but got the following compile errors:

<snipped>/impl/executor.hpp:76: error: ‘class boost::asio::any_io_executor’ has no member named ‘on_work_started’
   78 |     executor_.on_work_started();
      |     ~~~~~~~~~~^~~~~~~~~~~~~~~

and similar for on_work_finished, dispatch, post, and defer.

@ecorm
Copy link
Author

ecorm commented Aug 10, 2022

So I did. Fixed.

Thanks! With the any_completion_executor.ipp in hand, I reverted back to leaving BOOST_ASIO_USE_TS_EXECUTOR_AS_DEFAULT undefined. Everything works: my test suite which uses callback functions and stackful coroutines, as well as my examples using C++20 coroutines, stackless coroutines, and std::futures.

I must say I'm impressed with how quickly you put all this together and that it works with just a small example to test it against!

I still need to run Valgrind to check for memory leaks, but everything seems to be working well so far.

If you make any changes or additions to any_completion_handler or any_completion_executor, please notify me here and I'll be happy to test it against my CppWAMP library.

Your any_completion_handler, along with any_completion_executor, will be useful tools for library writers wanting to introduce a "compiler firewall" between their users and Asio, while at the same time allowing users their choice of completion tokens.

I used to have separate APIs in my library for traditional callbacks and stackful coroutines, but now I'm able to collapse them into a single API that accepts any completion token, while still being able to compile my library.

@chriskohlhoff
Copy link
Owner

I pushed an updated branch with some changes to ensure that the associated_executor specialisation won't throw (and consequently std::terminate) if constructing the any_completion_executor needs to allocate but allocation fails.

@ecorm
Copy link
Author

ecorm commented Aug 18, 2022

EDIT: Nevermind, brainfart.


Those latest no-throw changes result in compile errors that I can't easily decipher. The first of them is:

cppwamp/include/cppwamp/bundled/boost_asio_any_completion_executor.hpp:145:7: error: no matching constructor for
initialization of 'execution::any_executor<execution::blocking_t::never_t, execution::prefer_only<execution::blocking_t::possibly_t>,
execution::prefer_only<execution::outstanding_work_t::tracked_t>,
execution::prefer_only<execution::outstanding_work_t::untracked_t>, execution::prefer_only<execution::relationship_t::fork_t>,
execution::prefer_only<execution::relationship_t::continuation_t>>' (aka 'any_executor<never_t<0>, prefer_only<possibly_t<0>>,
prefer_only<tracked_t<0>>, prefer_only<untracked_t<0>>, prefer_only<fork_t<0>>, prefer_only<continuation_t<0>>>')
    : base_type(std::nothrow, BOOST_ASIO_MOVE_CAST(OtherAnyExecutor)(e))
      ^         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

It occurs when I attempt to boost::asio::post an any_completion_handler wrapping a lamdba function handler passed through async_initiate.

I'm still on Boost 1.79; perhaps updating to Boost 1.80 would solve the problem.

If you care to compile my library yourself to see the complete error traces, here's a snapshot of my library where the errors occur: https://github.com/ecorm/cppwamp/tree/e0455d11601fdb7bbf56dcb705ccc0407e410cf8

To compile:

git clone https://github.com/ecorm/cppwamp
cd cppwamp
git checkout e0455d11601fdb7bbf56dcb705ccc0407e410cf8
cmake -DCPPWAMP_OPT_VENDORIZE=On -DCPPWAMP_OPT_WITH_CORO20 -S . -B ./_build
cmake --build ./_build

The above will download Boost and will link against it, instead of your system's Boost libraries. Your any_completion_stuff lives in the cppwamp/include/cppwamp/bundled directory, and was modified for use with Boost.Asio.

I apologize in advance for the convoluted design; I'm currently working on simplifying it.

@ecorm
Copy link
Author

ecorm commented Aug 18, 2022

Oh, I see the problem now. You made changes to any_io_executor and execution::any_executor<> in the any-completion-handler branch. Those changes are obviously not in Boost 1.79, nor Boost 1.80. Until any_completion_handler is merged and provided in the next Boost release, I'm stuck with whatever you had before you made changes to any_io_executor and execution::any_executor<>.

Sorry about the brainfart.

Edit: I see now I can just manually remove the std::nothrow_t overloads in the future revisions of any_completion_executor within the any-completion-handler branch.

@ecorm
Copy link
Author

ecorm commented Aug 24, 2022

@chriskohlhoff It's currently not possible to use any_completion_executor with the Boost.Coroutine-based spawn with ASIO_DISABLE_BOOST_COROUTINE undefined. It complains that:

‘const class asio::any_completion_executor’ has no member named ‘context’

I tried with this using the head of your any-completion-handler branch:

#include <iostream>
#include <asio/any_completion_executor.hpp>
#include <asio/detached.hpp>
#include <asio/io_context.hpp>
#include <asio/spawn.hpp>

//------------------------------------------------------------------------------
int main()
{
    asio::io_context ioctx;
    asio::any_completion_executor exec(ioctx.get_executor());

    asio::spawn(
        exec,
        [](asio::basic_yield_context<asio::any_completion_executor> yield)
        {
            std::cout << "Hello" << std::endl;
        }
#ifdef ASIO_DISABLE_BOOST_COROUTINE
         , asio::detached
#endif
    );

    ioctx.run();

    return 0;
}

With ASIO_DISABLE_BOOST_COROUTINE defined, it works as expected.

@ecorm
Copy link
Author

ecorm commented Aug 24, 2022

As a workaround, I'm using this hack:

template <typename E, typename F>
void spawn_workaround(E& executor, F&& function)
{
    using namespace asio;

    auto ex1 = executor.template target<typename io_context::executor_type>();
    if (ex1 != nullptr)
    {
        spawn(*ex1, std::forward<F>(function));
        return;
    }

    auto ex2 = executor.template target<any_io_executor>();
    if (ex2 != nullptr)
    {
        spawn(*ex2, std::forward<F>(function));
        return;
    }

    throw std::logic_error("Executor must originate from io_service::executor_type "
                           "or any_io_executor");
}

@chriskohlhoff
Copy link
Owner

That legacy non-completion-token overload of spawn wraps the executor in a strand, so cannot work with any_completion_executor. You'll need to use the completion-token overload for compatibility with any_completion_executor.

@ecorm
Copy link
Author

ecorm commented Aug 25, 2022

You'll need to use the completion-token overload for compatibility with any_completion_executor.

I mistakenly thought those overloads where not available when ASIO_DISABLE_BOOST_COROUTINE is undefined. My bad.

@mabrarov
Copy link

Hi @chriskohlhoff and @ecorm,

First of all I want to thank you for idea and implementation of this feature.

I'd like to notice (and get your feedback) one issue which users of asio::any_completion_handler may have (maybe it requires to be highlighted in respective documentation):

  1. Most if not all of Asio classes providing async_ methods support the case when completion handler prolongs lifetime of instance of class which async_* method is called with that completion handler, e.g. completion handler can include std::shared_ptr to the instances of asio::ip::tcp::socket which async_read_some method is called with that completion handler (refer to Asio examples), and this doesn't cause issues with lifetime, because destructor of asio::io_context invokes shutdown method for respective service ensuring that asynchronous operations are completed / cancelled and respective completion handlers are invoked / destroyed (i.e. breaks circular dependency).
  2. If user of asio::any_completion_handler wants to implement a class providing async_* method and uses non-static class member of asio::any_completion_handler type to keep completion handler (provided as parameter of that async_* method) till completion of asynchronous operation, then consumer of the class can assume that the same guarantee works for that class too, e.g. the consumer can intentionally use completion handler which prolongs lifetime of instance of the class and this can lead to a circular dependency, which is not broken by destructor of asio::io_context, because there is no instance of asio::io_context which tracks instances of asio::any_completion_handler.

This case is the reason I created (ugly) ma::handler_storage class, so I hope that your feedback can help me to get rid of this class.

@ecorm
Copy link
Author

ecorm commented Sep 23, 2022

@mabrarov I'm having difficulty understanding your second point. Can you clarify with some pseudocode?

@ecorm
Copy link
Author

ecorm commented Jan 31, 2023

@chriskohlhoff I've updated to Boost 1.81.0 which now includes any_completion_handler and any_completion_executor (thank you!). I now get compile errors whenever I try to post to an any_completion_executor:

#include <iostream>
#include <boost/asio/any_completion_handler.hpp>
#include <boost/asio/io_context.hpp>
#include <boost/asio/post.hpp>

int main()
{
    namespace asio = boost::asio;
    asio::io_context ioctx;
    asio::any_completion_executor exec(ioctx.get_executor());
    asio::any_completion_handler<void ()> handler([](){std::cout << "Hello";});
    asio::post(exec, std::move(handler)); // error: no matching function for call to `post`
    // asio::post(ioctx, std::move(handler)); // This compiles
    // exec.execute(std::move(handler)); // And this compiles
    ioctx.run();
    return 0;
}

@ecorm
Copy link
Author

ecorm commented Jan 31, 2023

I now get compile errors whenever I try to post to an any_completion_executor

I think I stumbled upon an explanation in this New in Boost 1.81 blog post, under Asio -> Semantic Changes. I'm gonna need to re-read it a few times to make sense of it.

I think this is what I should be doing:

asio::post(ioctx, asio::bind_executor(exec, std::move(handler)));

@ecorm
Copy link
Author

ecorm commented Feb 1, 2023

Boost 1.81 is now working with my library, which uses any_completion_handler and any_completion_executor rather extensively. Thanks again!

@ecorm ecorm closed this as completed Jul 18, 2023
@ecorm
Copy link
Author

ecorm commented Jul 18, 2023

Added #1330, a feature request for a multi-shot equivalent of any_completion_handler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants