Naios / continuable

C++14 asynchronous allocation aware futures (supporting then, exception handling, coroutines and connections)
https://naios.github.io/continuable/
MIT License
815 stars 44 forks source link

Stopping a continuable in a failure handler makes wait() hang forever #46

Closed rkonklewski-am2m closed 2 years ago

rkonklewski-am2m commented 2 years ago

First of all, thanks for this great library @Naios! I really appreciate all the work you've put into it!

In some parts of my project I need to resolve a continuation chain synchronously and get its result. The most obvious way of doing that is applying the cti::transforms::wait() transform. While it seems to work most of the time, I discovered two scenarios, in which it would hang the wait forever (see steps to reproduce for code samples): 1) When a failure handler returns an empty_result (i.e. stop) 2) When a failure handler returns nothing (void)

The first one requires intent, so it's less dangerous. The second scenario, unfortunately, makes it easy to shoot oneself in the foot by mistake.


Commit Hash

Latest

Expected Behavior

Waiting on a stopped continuable does not hang. Either returning an empty result or throwing an exception would be infinitely better.

Actual Behavior

Waiting on a stopped continuable hangs forever. A workaround is to use timed waits (wait_for or wait_until).

Steps to Reproduce

1) Failure handler returns an empty_result

auto eptr = std::make_exception_ptr(std::exception("test"));
auto continuable = cti::make_exceptional_continuable<void>(eptr).fail([]() { return cti::stop(); });
std::move(continuable).apply(cti::transforms::wait()); // waits forever

2) Failure handler returns void

auto eptr = std::make_exception_ptr(std::exception("test"));
auto continuable = cti::make_exceptional_continuable<void>(eptr).fail([]() { /*do nothing*/ });
std::move(continuable).apply(cti::transforms::wait()); // waits forever

Your Environment

Naios commented 2 years ago

I'm glad that you like the library @rkonklewski-am2m.

I will have a look at this problem, probably it won't be hard to reproduce it with the test cases you provided.

This might take some time though, as working on this library is not my highest priority at the moment, so I would also highly appreciate a pull request.

yaoyuan1216 commented 2 years ago

I found the reason is a failed void continuable would not invoke following callbacks (shown below), as a result the ready in wait_relaxed would never be changed to true. template <typename... Args, typename T> void invoke_void_no_except(identity<exception_arg_t, Args...>, T&& /*callable*/) noexcept { // Don't invoke the next failure handler when being in an exception handler }

Naios commented 2 years ago

I fixed this issue in https://github.com/Naios/continuable/commit/b51be39e7126c397cec9d13fefdfa3cbbe8291f0, thank you for your report.

rkonklewski-am2m commented 1 year ago

Thank you! In the end I opted to use timed waits, so this issue became a low priority. However, there are a lot of places where a simple wait would be much more convenient. I'm surely going to utilize this fix in the future.