Feature request: cancelling/aborting pending request in async requests

[abmantis]

If a request is not complete but its timeout value has not been reach (or is not set), there is no way to cancel it. While this is ok for synchronous requests (using cpr::Get), async requests should provide some way of canceling/terminating the request.

[RuleOfThrees]

Hello! I'd like to take a look at this, if it's ok!

[COM8]

Sure, you are more then welcome to take a look at this. Let me know in case you need any help.

[leviliangtw]

I have also looked into this topic and studied a little about it. In my view, it's time to refactor and combine async and multiperform together.

Here are some facts that I conclude:

• Currently, there is no simple approach to directly interrupt and cancel an async thread.
• I have managed to use std::atomic_bool as an interrupt signal for an async thread and it succeeded; however, it seems not easy to cancel curl_easy_perform.
• There are two approaches to canceling a curl request:
  • Customized callback functions like CURLOPT_XFERINFOFUNCTION
  • Utilizing curl_multi_remove_handle by wrapping curl_easy_perform into curl_multi_perform

What are your opinions and ideas?

References:

• How to stop a std::async task | Studio Freya
• c++ - Is there a way to cancel/detach a future in C++11? - Stack Overflow
• c++ - Is it Possible to terminate the std::async thread? - Stack Overflow
• c - Example how to cancel a libCurl request - Stack Overflow
• CURLOPT_XFERINFOFUNCTION
• curl_multi_remove_handle

[RuleOfThrees]

Sorry to take so long to give feedback, unfortunately real life intervened. I have done some research on the matter, and experimented a bit. The conclusions so far are as follows: The prospective cancellation function for an async request will have to be able to handle the following situations:

1. The Task related to the AsyncResponse that the API user has gotten, is in the queue. In this case, the Task has to be identified and removed from the queue. This may require making the AsyncResponse its own class instead of being an alias for a future.
2. The Task, and its associated request, is being processed by a thread in the GlobalThreadPool. In this case, there are two options, as @leviliangtw remarked:
   • Use the CURLOPT_XFERINFOFUNCTION or similar callbacks. This could deliver the desired result, but it would be using a feature for its side-effects, which may or may not be perceived as bad practice.
   • Make async use the multiperform API instead of easy. The advantage of this, is that the multiperform API has a standard way of cancelling requests. The disadvantage is that initializing a multiperform session for every single request would cause avoidable overhead to users who wouldn't use the async cancellation feature. A possible solution would be to replace the GlobalThreadPool by a multiperform session. This could solve all problems, but may negatively impact performance.

My conclusion so far is that there are two possible approaches. First would be

to refactor and combine async and multiperform together.

as said above.

An alternative would be to leave async as it is, and try to expand the cpr API by offering MultiGetAsync and the assorted functions, where what would be returned would be a vector of AsyncResponses which would be progressively populated by a thread polling the multiperform session in the background.

Opinions?

[COM8]

@RuleOfThrees thanks for looking into this as well. I will give you some feedback on your approach until next week.

[COM8]

In my eyes introducing a MultiGetAsync interface is the best in this case. With that we do not destroy existing workflows/implementations or the behaviour of existing applications.

I also strongly suggest creating a separate cpr::AsyncResponse class. This makes it then later far easier for cancelling tasks by simply calling for example myAsyncResponse.cancel();.

In my eyes we should avoid using multiperform for async calls. It would be far better to use CURLOPT_XFERINFOFUNCTION, although like mentioned by the docs:

While data is being transferred it will be called frequently, and during slow periods like when nothing is being transferred it can slow down to about one call per second.

It's not perfect, but as long as we can guarantee, that we can cancel the request in a finite amount of time after myAsyncResponse.cancel(); has been called, it should be fine. This needs to be tested for really slow connections!

[RuleOfThrees]

Thanks! Now I think I have a good understanding of how it's expected to be done. I'll get on to it!