[thread] await hangs when using boost::promise/future
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? Thanks.
2015-01-17 21:45 GMT+08:00 TONGARI J <tongari95@gmail.com>:
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?
Forgot to mention that it's compiled with Boost 1.57, BOOST_THREAD_VERSION=4, x64
Thanks.
On Sat, Jan 17, 2015 at 8:48 AM, TONGARI J <tongari95@gmail.com> wrote:
2015-01-17 21:45 GMT+08:00 TONGARI J <tongari95@gmail.com>:
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?
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. 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. 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. The continuation then tries to acquire the mutex (has_exception), and blocks waiting for the set_value() to clear. set_value() is in turn waiting for the continuation thread to join because nothing else has a handle to the thread. 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. Lee
Le 17/01/15 15:36, Lee Clagett a écrit :
On Sat, Jan 17, 2015 at 8:48 AM, TONGARI J <tongari95@gmail.com <mailto:tongari95@gmail.com>> wrote:
2015-01-17 21:45 GMT+08:00 TONGARI J <tongari95@gmail.com <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 <class T, class Promise> 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.
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?
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_lock<boost::mutex>& ) { this->thr_ = thread(&future_async_continuation_shared_state::run, 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 :
On Sat, Jan 17, 2015 at 8:48 AM, TONGARI J <tongari95@gmail.com <mailto:tongari95@gmail.com>> wrote:
2015-01-17 21:45 GMT+08:00 TONGARI J <tongari95@gmail.com <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 <class T, class Promise> 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. After the analysis below I should say, it should block, and currently doesn't blocks :(
Vicente
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.
Hrr, I think that I see what you mean. The current implementation makes the shared state to block on the destructor, not the future itself, is that correct?
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_lock<boost::mutex>& ) { this->thr_ = thread(&future_async_continuation_shared_state::run, 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 :(
On Sat, Jan 17, 2015 at 1:55 PM, Vicente J. Botet Escriba < 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 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 Sun, Jan 18, 2015 at 1:14 AM, Lee Clagett <forum@leeclagett.com> wrote:
On Sat, Jan 17, 2015 at 1:55 PM, Vicente J. Botet Escriba < 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 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.
Sorry I think I misspoke slightly here. In the patch I have listed, the
thread join will occur when the shared_continuation_state is destructed. This is owned by the shared_state of the original promise/future and the continuation future, which is even more confusing than I described. So in most cases it will block in the destructor of the continuation future, or a setter in the original promise. Hopefully I got it right this time. Lee
Le 18/01/15 07:14, Lee Clagett a écrit :
On Sat, Jan 17, 2015 at 1:55 PM, Vicente J. Botet Escriba <vicente.botet@wanadoo.fr <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 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
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? Thanks in advance, 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.
Le 25/01/15 13:07, TONGARI J a écrit :
2015-01-25 16:43 GMT+08:00 Vicente J. Botet Escriba <vicente.botet@wanadoo.fr <mailto: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.
Let me know if you get something working. Vicente
participants (3)
-
Lee Clagett -
TONGARI J -
Vicente J. Botet Escriba