[draft] add support for asio CompletionToken api for client #97
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This implements support for the asio CompletionToken api as discussed in #74.
There are some issues currently with coroutines. The only compiler I can get working with them currently is gcc and it won't compile one of the tests (it crashes with an ICE). There is an example using coroutines, but it throws a
bad_alloc
exception with an unreadable stack trace, I'm not sure if this is an issue with how I wrote the example or asio.Major breaking changes
client::controller::http_connect
no longer accepts a visitor, instead if the filter has a single body type it accepts a completion token for a response with that body, otherwise a completion token with a variant of responses over the possible bodies. This means that code currently using callbacks and filters with more than 1 body (i.e. a custom one, none of the ones packaged with malloy currently use this) need to add a call tostd::visit
client::controller::http_connect
no longer returns astd::future<error_code>
, instead it is contained in the completion token. This means every user currently using a callback token (all of them) needs to add another argument. The reason it originally returned a future was to prevent the user having to write the same boilerplate every time to wait on the result without running off the end ofmain
and without waiting forever on an error. However, the future behavior can be restored by passingasio::use_future
and then adding atry...catch
around the.get()
, which is what I have done for the examplesOther major changes
websocket::connection::[accept, send, read, connect]
now use CompletionToken for the template type, allowing one to pass for exampleasio::use_future
and have the return be a future rather than invoking a callbackNotable internal changes
action_queue
now uses a customaction
polymorphic interface for the stored actions. This is in order to allow non-copyableCompletionToken
s as converting tostd::function
causes a copy (as I discovered). While this does require the explicit allocation of aunique_ptr
I believe it actually results in fewer allocations overall asstd::function
has a lot of hidden ones. This does meanwebsocket::connection
has some additional boilerplate but I consider this a reasonable tradeoff for the api it now providesasio::async_initiate
is now used to enable the CompletionToken api. One gotcha with it is that the type passed to the handler function is not the same as the CompletionToken type, and is not guaranteed to by copyable (although it is always moveable)client::http::connection
has changed a bit,run
has been removed with its arguments being moved to the constructor.start
is now called to start it. I originally tried to remove it completely but due toshared_from_this
it can only be started after the call tomake_shared
is complete. Honestly I am not particularly happy withconnection
in its current form, as it isn't really a standalone class and is essentially just a complex delegate that should be an implementation detail ofcontroller
imo (unlikewebsocket::connection
for example, which is completely usable standalone).Things that could use improvement (IMO)
controller::stop
also use this api? It might be doable but is more complex because of the weird inheritance stuff going on with ituse_future
, there is one currently for websockets but I'm not sure if there is any advantage to more (and the disadvantage is effectively duplicate code with only minor differences)