Le 17/01/15 15:36, Lee Clagett a écrit :

On Sat, Jan 17, 2015 at 8:48 AM, TONGARI J mailto:tongari95@gmail.com> wrote:

2015-01-17 21:45 GMT+08:00 TONGARI J mailto:tongari95@gmail.com>:

Hi,

The sample code is here: https://gist.github.com/jamboree/3b8716ad1922749e77ae

The boost::future adaption code is adapted from N4286.

About this code template void await_suspend( future<T> & t, std::experimental::resumable_handle<Promise> rh) { t.then([=](auto&& result) mutable { if (result.has_exception()) rh.promise().set_exception(result.get_exception_ptr()); rh(); }); } this will not suspend, as the result of t.then is ignored, and so it block on the destructor.

Tested with VS2015 preview. If std::promise is passed, the code works fine. Any clue why it hangs if boost::promise is passed?

Forgot to mention that it's compiled with Boost 1.57, BOOST_THREAD_VERSION=4, x64

Thanks.

I found two bugs in this portion of boost::future last week, and I have been meaning to post to trac and offer assistance in fixing.

Please report Boost.Thread bugs as soon as possible. I'm really interested.

The first bug is in the .then() continuation - it is supposed to block in this situation (according to the docs), just like boost/std::async does if the future object is ignored. Agreed. Instead, it continues unblocked.

The second bug is a consequence of ignoring the returned future. The call to .set_value() acquires the mutex, and invokes the continuation on a separate thread. Right. The code is void launch_continuation(boost::unique_lockboost::mutex& ) { this->thr_ = thread(&future_async_continuation_shared_state::run,

Hrr, I think that I see what you mean. The current implementation makes the shared state to block on the destructor, not the future is self, is that correct? this); } Do you mean that I need to unlock before creating the thread? I could do it, but thread ensures that the function will not be called synchronously.

The continuation then tries to acquire the mutex (has_exception), and blocks waiting for the set_value() to clear. The continuation is executed in another thread and only when it is ready (either there is a set_value or a set_exception). So the continuation shouldn't block. set_value() is in turn waiting for the continuation thread to join because nothing else has a handle to the thread. I don't understand how did you got this conclusion. Please could you clarify? I can understand that the as the blocking is on the shared future, the promise destructor will block in this case.

The best way to get around this is to _not_ ignore the returned future from the .then call. I also have patch that fixes the second bug, but fixing the first bug will require enough changes that will likely result in an obsolete patch.

Is the patch for Boost.Thread? Thanks for your comments. I will try to fix this the blocking issue asap but I suspect that this couldn't go into the next Boost version :( Best, Vicente

Le 17/01/15 16:36, Vicente J. Botet Escriba a écrit :

Le 17/01/15 15:36, Lee Clagett a écrit : The continuation then tries to acquire the mutex (has_exception), and blocks waiting for the set_value() to clear.

The continuation is executed in another thread and only when it is ready (either there is a set_value or a set_exception). So the continuation shouldn't block.

set_value() is in turn waiting for the continuation thread to join because nothing else has a handle to the thread.

I don't understand how did you got this conclusion. Please could you clarify? I can understand that the as the blocking is on the shared future, the promise destructor will block in this case.

This line here [ https://github.com/boostorg/thread/blob/master/include/boost/thread/future.h... ]. this_continuation_ptr is the last thing that references the continuation

On Sat, Jan 17, 2015 at 1:55 PM, Vicente J. Botet Escriba < vicente.botet@wanadoo.fr> wrote: thread, so it tries to join it during destruction, while the mutex is held up the call stack. If the continuation thread calls a function on the future it receives, it then must wait for the same mutex which will never be released.

The best way to get around this is to _not_ ignore the returned future from the .then call. I also have patch that fixes the second bug, but fixing the first bug will require enough changes that will likely result in an obsolete patch.

Is the patch for Boost.Thread?

Thanks for your comments. I will try to fix this the blocking issue asap but I suspect that this couldn't go into the next Boost version :(

I have a patch here [

https://github.com/vtnerd/thread/compare/boostorg:develop...ContinuationFix ] that addresses the deadlock issue, and should allow the code by the OP to work exactly as desired. However, the patch does not work as the docs indicate. Instead of blocking on the destructor of the continuation future, it will block in the destructor of the original future, OR the setter of the original promise (which could be its destructor in broken_promise case), whichever occurs last. Fixing the .then() blocking issue is a little tricky because the destructor of the continuation future could need to wait for a thread that won't be launched until a value is set in the original promise. Is blocking in the destructor of the continuation future necessary? A callback signals when the original future has been set just the same. The negative is the thread joining is a little more confusing to describe, and deferred execution will not work correctly if the continuation future is ignored (the test case in the patch above fails when the continuation launch policy is deferred). Lee

On Sat, Jan 17, 2015 at 1:55 PM, Vicente J. Botet Escriba mailto:vicente.botet@wanadoo.fr> wrote:

Le 17/01/15 16:36, Vicente J. Botet Escriba a écrit :

...

Le 17/01/15 15:36, Lee Clagett a écrit : The continuation then tries to acquire the mutex (has_exception), and blocks waiting for the set_value() to clear. The continuation is executed in another thread and only when it is ready (either there is a set_value or a set_exception). So the continuation shouldn't block.

...

set_value() is in turn waiting for the continuation thread to join because nothing else has a handle to the thread.

I don't understand how did you got this conclusion. Please could you clarify? I can understand that the as the blocking is on the shared future, the promise destructor will block in this case.

This line here [ https://github.com/boostorg/thread/blob/master/include/boost/thread/future.h... ]. this_continuation_ptr is the last thing that references the continuation thread, so it tries to join it during destruction, while the mutex is held up the call stack. If the continuation thread calls a function on the future it receives, it then must wait for the same mutex which will never be released. The mutex that is held is the mutex of the other future, not the one of

Le 18/01/15 07:14, Lee Clagett a écrit : the continuation future. The fact that it joins is a consequence that the user has not stored the future (as expects, but that with the current interface I can not force).

...

...

The best way to get around this is to _not_ ignore the returned future from the .then call. I also have patch that fixes the second bug, but fixing the first bug will require enough changes that will likely result in an obsolete patch.

Is the patch for Boost.Thread?

Thanks for your comments. I will try to fix this the blocking issue asap but I suspect that this couldn't go into the next Boost version :(

I have a patch here [ https://github.com/vtnerd/thread/compare/boostorg:develop...ContinuationFix ] that addresses the deadlock issue, and should allow the code by the OP to work exactly as desired. However, the patch does not work as the docs indicate. Instead of blocking on the destructor of the continuation future, it will block in the destructor of the original future, OR the setter of the original promise (which could be its destructor in broken_promise case), whichever occurs last.

I will take a look, but I want to go towards the semantic of the standard. The original future could be destructed before the continuation future, and in fact this is usual the case.

Fixing the .then() blocking issue is a little tricky because the destructor of the continuation future could need to wait for a thread that won't be launched until a value is set in the original promise. I'm working on a fix (on a fix/blocking_future branch - the fix is not yet visible). For the time been I have reached to fix async. future::then() works also, but I have some yet unidentified issues with shared_future::then()

Is blocking in the destructor of the continuation future necessary? A callback signals when the original future has been set just the same. The negative is the thread joining is a little more confusing to describe, and deferred execution will not work correctly if the continuation future is ignored (the test case in the patch above fails when the continuation launch policy is deferred). As far as we have a specific thread associated to it, I prefer to have some one that takes car of joining it. I will see in which cases we can call directly the continuation without using a new thread.

The experimental::future::then() has not this case as as async has been removed. That means that there are no more blocking futures), but as soon as you introduce async, the continuation is executed using the same mechanism. Best, Vicente

2015-01-25 16:43 GMT+08:00 Vicente J. Botet Escriba < vicente.botet@wanadoo.fr>:

Le 17/01/15 14:45, TONGARI J a écrit :

Hi,

...

The sample code is here: https://gist.github.com/jamboree/3b8716ad1922749e77ae

The boost::future adaption code is adapted from N4286.

Tested with VS2015 preview. If std::promise is passed, the code works fine. Any clue why it hangs if boost::promise is passed?

Hi,

I've been working on a possible fix on this branch https://github.com/boostorg/thread/tree/fix/blocking_future.

I have found a lot of issues on the current implementation and most of them are fixed now (weel at least I think so), however I have yet some issues with shared_future::then.

Please, could you give a try to this branch?

Doesn't seem to change the behavior. That said, I'm not sure what the correct adaption code for boost::future would be, as you guys pointed out, the code extracted from N4286 isn't quite correct. And I also found other bugs in their await implementation, but I'm not sure it's relevant to this problem... Anyway, thanks for looking into this, though I was just playing with the proposed await, not really for practical work.