chriskohlhoff / asio

Asio C++ Library
http://think-async.com/Asio
4.95k stars 1.22k forks source link

"Services" example doesn't show how to treat Handlers #306

Open reddwarf69 opened 6 years ago

reddwarf69 commented 6 years ago

AFAIK the only example of how to create your own service is the logger in https://www.boost.org/doc/libs/1_67_0/doc/html/boost_asio/examples/cpp03_examples.html. It's quite helpful, but the log method logger provides doesn't need any Handler, so there is no opportunity to show how it works. If the example could be expanded to, for example, handle errors in log_impl (to be notified via a Handler) it would become a great source of documentation.

reddwarf69 commented 6 years ago

Also, should that example inherit from basic_io_object instead of defining service_type and impl_type itself?

vinniefalco commented 6 years ago

The purpose of a service is to store state per-io_context state information used in the implementation of custom I/O objects. Services do not usually invoke completion handlers unless it is through an initiating function of a related I/O object. Why do you think you need a service?

reddwarf69 commented 6 years ago

Beast doesn't yet support WAMP (https://wamp-proto.org/) ;-) The only library I have found supporting the WAMP features I need is https://github.com/darrenjs/wampcc. But it's kind of blocking/synchronous (it uses a couple of background threads, but...). So I was trying to have an ASIO I/O object with async_connect(), etc. hiding the fact that I'm using wampcc.

I get the general idea of

But I'm not really 100% sure about the lifetime of each thing (I have seen a shared/weak pointer in the basic_resolver implementation which I kind of get... but not too confident about it). The allocation thing from https://github.com/boostorg/asio/blob/develop/include/boost/asio/detail/resolver_service.hpp#L127 is beyond me, and not sure if I'm allowed to use some of the macros used in the basic_resolver implementation or if they are supposed to be private. I didn't notice things like the move_construct until I looked at the basic_resolve code either, I'm sure they are documented somewhere...

In general there are quite a few details that I am unsure of. Seeing an example of "the right way(TM)" of doing it as a reference would make me more confident on my own code.

mabrarov commented 6 years ago

@RedDwarf69, is my understanding of your need correct:

  1. You have some service / code which may even provide synchronous (instead of asynchronous) API.
  2. You want to wrap that service with some Asio-like interface with asynchronous API and handlers (and maybe even with custom memory allocation which is supported for handlers in most of Asio classes having async_ methods). For example, you want your "wrapper" to provide interface like asio::ip::tcp::socket class (in terms of async_ methods).
  3. You are looking for creation of custom Asio service to implement that "wrapper".
  4. There is no example with async interface which methods accept handlers in Asio document (in examples).
  5. You have concerns about lifetime of handlers.

?

reddwarf69 commented 6 years ago

Yep, that's right. About the "custom memory allocation". Technically if I don't use it it would be fine, nobody else is going to use my wrapper. But if I got it right every "correct" Asio service is supposed to handle custom memory allocations, no?

I'm guessing the whole "hide a blocking library behind an ASIO service with a thread" must be a common need. So having the example should be helpful to lots of people.

vinniefalco commented 6 years ago

Implementing a service to hide a synchronous WAMP implementation behind Asio's asynchronous abstractions seems like a LOT of work... I feel like it might be more productive to just implement your own WAMP library yourself using existing components such as Boost.Beast and your favorite JSON library.

mabrarov commented 6 years ago

@RedDwarf69,

Yep, that's right.

I came from the same need to (my) Asio samples project.

Actually I didn't want to create Asio service. Though it's possible. I started from Asio service and later I just found that I don't need to continue till creation of Asio service - it was sufficient for my task to create class which interface provides async_ methods with the same features / guarantees as Asio provides and using asio::io_service as proactor / base of my thread pool (while the same instance of asio::io_service can be still used by other code).

I created ma::handler_storage class (and corresponding ma::handler_storage_service Asio service) to "park" handlers till the time I need to invoke them (till the time async operation completes) passing some data - at least error_code - into the handler.

ma_async_basics example demonstrates how some background activity can be wrapped with async_ method which supports completion handler (async_do_something(Handler)).

"Background activity" actually starts at async_implementation::start_do_something method. When that background activity ends - after 100 messages are printed - then async_implementation::complete_do_something method is called. That method just posts parked handler (provided by caller of async_do_something method).

For the caller - refer to main.cpp - it looks almost like asio::ip::tcp::socket::async_connect.

More complex example - with async_read_some method - can be found at ma_nmea_client.

@vinniefalco,

a LOT of work..

You are right. That's why I created asio_samples and ma::handler_storage / ma::handler_storage_service classes. They are intended to serve as helper for creation of Asio-like asynchronous interfaces when we need stored handler somewhere till the time logical operation completes.

mabrarov commented 6 years ago

BTW, I suggested @chriskohlhoff to include smth like ma::handler_storage class into Asio but he answered that asio::deadline_timer can be used for the same (though it doesn't provide ability to pass some data - result of completion of my async operation - from my class with async_ method to the handler).

reddwarf69 commented 6 years ago

Ignoring my specific needs. I have tried to create such an example in https://github.com/RedDwarf69/asio_service_attempt. Anybody want to comment on it? I have not bother to create a PR because I know the example is still not good (see all the "XXX" comments).

Things like https://github.com/RedDwarf69/asio_service_attempt/blob/master/SillySocketService.h#L61 are probably more of a problem with my lack of C++ experience than anything else. But it would be nice to make it easier to do even for C++ beginners.

reddwarf69 commented 6 years ago

I have just noticed this @vinniefalco -> https://www.boost.org/doc/libs/1_67_0/libs/beast/doc/html/beast/using_io/writing_composed_operations.html

It's great documentation! I don't think there is a similar example for plain ASIO. It seems weird to have it in Beast but not in ASIO.

reddwarf69 commented 6 years ago

One specific question @vinniefalco. Isn't https://www.boost.org/doc/libs/1_67_0/boost/beast/core/bind_handler.hpp a better version of https://www.boost.org/doc/libs/1_67_0/doc/html/boost_asio/reference/bind_executor.html that does exactly the same thing for the executor, but also handles the allocator?

vinniefalco commented 6 years ago

Isn't bind_handler a better version of bind_executor?

The two functions perform completely different tasks. bind_handler is analagous to std::bind. It creates a nullary handler from an N-ary handler and bound parameters (it also supports placeholders if you want). While bind_executor takes a nullary handler and creates a new nullary handler which uses the specified executor.

reddwarf69 commented 6 years ago

I mean, I could do

boost::asio::post(std::bind(handler, ec));

and it would not use the correct executor because of the type erasure in std::bind.

I could try to fix it with

boost::asio::post(boost::asio::bind_executor(boost::asio::get_associated_executor(handler), std::bind(handler, ec)));

and now I would use the correct executor, but still the wrong allocator.

Or I could do

boost::asio::post(boost::beast::bind_handler(std::move(handler), ec));

and everything would be good.

Right? That's what I meant by "better version of bind_executor". Doesn't this make bind_executor obsolete?

vinniefalco commented 6 years ago

Doesn't this make bind_executor obsolete?

LOL... no... how would you bind the result of std::bind to a strand without bind_executor? See: https://github.com/boostorg/beast/blob/93c35524a6125db3e6aeefc8abc826a810dd5d61/example/advanced/server/advanced_server.cpp#L261

vinniefalco commented 6 years ago

By the way, I am starting to think that [networking.ts] should specify that the wrapper returned by std::bind has the same associated allocator and associated executor as the bound function object.

reddwarf69 commented 6 years ago

how would you bind the result of std::bind to a strand without bind_executor

I was going to say with strand::wrap... then I noticed it's deprecated in favour of bind_executor :-p Ignore me.

Still, the rest of my comment was right? Until [networking.ts] takes your suggestion about std::bind, boost::asio::post(boost::beast::bind_handler(std::move(handler), ec)); is the right, simplest, way to fix boost::asio::post(std::bind(handler, ec));?

vinniefalco commented 6 years ago

beast::bind_handler is the right, simplest, way to fix std::bind?

Yes

vinniefalco commented 6 years ago

See: [P1130R0] Networking TS Associations For Call Wrappers http://vinniefalco.github.io/papers/p1133r0.html

reddwarf69 commented 6 years ago

That's really nice. Not just as a proposal, but as an explanation of the issue, showing the problems with each step. It's the kind of thing I would expect from the "ASIO/Networking TS book" the world is waiting for.

ecorm commented 1 year ago

I've run into problems where a handler would not be executed by the bound executor, and it turns out is was std::bind erasing away the associated executor. I'm glad to have found this issue which confirms my hunch that std::bind was the culprit.

@reddwarf69 I'm working on a new release of CppWAMP which is a C++ WAMP implementation using Boost.Asio under the hood. The new release will include router functionality. The API is asynchronous and can accept any completion token supported by Asio. I might add a Websocket transport based on Beast, depending on how well the ngx_tcp_websocket_module nginx module works.