Don't type-erase the CompletionHandler

[vinniefalco]

This code is incorrect:

    boost::asio::async_completion<CompletionToken, write_signature> c(token);
    do_write(std::move(c.completion_handler));

The problem is that do_write receives the handler as a std::function, which type-erases the handler. Later on, you call boost::asio::post with this object. Since the function object is typed-erased, it loses the associated allocator and more importantly the associated executor. The consequence is that when a user writes:

  auto const bytes_transferred =
    utp_sock.async_write_some(
      buffers,
      boost::asio::bind_executor(strand_, my_handler));

The wrapper returned by bind_executor loses its type information when it is stored in a function object. Then later on when you call post, it doesn't go through the strand.

This is going to sound painful, but if you want your library to work nicely with Networking TS / Asio (and I think you have really nice library here, so you should want this) then you are going to have to do the following:

• Remove all use of std::function (and never use it again)
• Move everything in the .cpp files into header files (this library must be header-only to work right)
• Rewrite your asynchronous operations (connect, read some, write some) as composed operations

These tutorials should help:
https://www.boost.org/doc/libs/1_69_0/libs/beast/doc/html/beast/using_io/writing_composed_operations.html
https://www.boost.org/doc/libs/1_69_0/libs/beast/doc/html/beast/using_io/example_detect_ssl.html

You might also consider studying the implementation of asio and beast. I can help with that by offering pointers and answering questions.

[inetic]

Is this not a duplicate of the issue #1? I'm happy to delete the first one in favor of this one as this one has a better explanation.

[vinniefalco]

Yes, this addresses #1

[inetic]

I believe this is issue is now fixed, please do reopen it if you think otherwise.

However I took a different approach than the one proposed above. Although I did get rid of std::function I'm having trouble agreeing that it's such an evil. I believe if I took care of the executor and the allocator, using std::function wouldn't have been a problem.

I also did not move everything from cpp files to hpp files. The thing is, I use libutp which isn't header only and so I do need to create a shared or a static library in any case. One of my other goals was to not expose any of the libutp header files to the user and also to not mess with the libutp sources by having to move the header files somewhere into asio-utp/include.

You might also consider studying the implementation of asio and beast. I can help with that by offering pointers and answering questions.

Yes, I do this and it has served me quite well in the past. The thing is that only until very recently I've only been using the pre 1.47 asio API (the one without executors) and I also haven't used multi-threaded asio yet so did have the need to use strands either. Thus it is very valuable to me to have your (and the kinds folkds from reddit) feedback in the issues so far.

The way I (believe I) fixed it, was to have created a custom handler structure which internally holds the non-type-erased completion handler, the executor and the allocator as well. It doesn't overload the operator() because I always either need to post it or dispatch it to the correct executor and allocator, which now is the case using handler::post or handler::dispatch.

Thanks for the help.

[ecorm]

I'm vexed with a similar problem and am glad to have stumbled upon this issue report. I think Asio should provide a type-erased handler wrapper (with small buffer optimization) to cater to networking libraries that do not wish to be header-only. Some of us care about compile times and want to be able to apply the Pimpl idiom.

[ecorm]

chriskohlhoff/asio#1100

[vinniefalco]

My opinion is that the type-erasure needs to happen at a higher level, at the application level. That is, the application places inside a translation unit all of the calls to Asio, and then exposes via header file its own abstraction that includes type-erasure. Raw sockets and streams should not be exposed to user-code in headers. This eliminates the need for a type-erased handler in the first place. (or type-erased buffer sequence, or type-erased stream, and so on).

This implies that a particular executor might be imposed on the caller if they are submitting callbacks using say, std::function.

[ecorm]

@vinniefalco The type-erased handler wrappers I propose would be for high-level networking libraries built on top of Asio or Beast. It would provide an additional tool available to library writers who prefer that their library be compilable (and not impose a particular completion token type upon their users).