boost::future::then lambda called before future is ready.

[Jeffrey.Gramowski@…]

When chaining .then together with anther one and the previous .then returns a boost::future. The lambda will get called before the future is ready. I think what is happening is that the lambda is being called when the outer hidden future is ready. I think that the callback shouldn't be called until the returned future is ready.

Here is the sample code which demonstrates the problem.

// BoostFutureTest.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"

#include <future>
// boost version 1.60.0
// has the following set.
// #define BOOST_THREAD_VERSION 4
// #define BOOST_THREAD_PROVIDES_EXECUTORS

#include <boost\thread\future.hpp>


int _tmain(int argc, _TCHAR* argv[])
{
    int value = 0;
    int tmpValue = 0;
    boost::promise<void> promise1;
    boost::promise<void> promise2;

    auto future1 = promise1.get_future();

    auto waitFuture = future1.then([&tmpValue, &promise2](boost::future<void> future){
        assert(future.is_ready());  // this works correctly and is ready.

        std::async(std::launch::async, [&promise2, &tmpValue](){
            std::this_thread::sleep_for(std::chrono::seconds(1));
            tmpValue = 1;
            promise2.set_value();
        });

        return promise2.get_future();
    }).then([&value, &tmpValue](boost::future<void> future){
        // this lambda is being called before the future is ready.
        assert(future.is_ready());  // this doesn't work correctly and is not ready.

        // if we call future.get() this will block until the promise2 has been set be I don't want
        // the then lambda to block. I want the lambda called when the future is ready.

        value = tmpValue;
    });


    promise1.set_value();
    waitFuture.wait();

    std::cout << "value = " << value << std::endl; // should print 1 but prints 0
    return 0;
}

[viboes]

Hi, The following shouldn't compile

    auto waitFuture = future1.then([&tmpValue, &promise2](boost::future<void> future){
        assert(future.is_ready());  // this works correctly and is ready.

Do you mean something else instead of future.is_ready?

[viboes]

You should store the returned future otherwise it will block in

        std::async(std::launch::async, [&promise2, &tmpValue](){
            std::this_thread::sleep_for(std::chrono::seconds(1));
            tmpValue = 1;
            promise2.set_value();
        });

[viboes]

Replying to viboes:

Hi, The following shouldn't compile

    auto waitFuture = future1.then([&tmpValue, &promise2](boost::future<void> future){
        assert(future.is_ready());  // this works correctly and is ready.

Do you mean something else instead of future.is_ready?

Forget this I didn't saw the future parameter.

[viboes]

Have you uncommented

//#define BOOST_THREAD_VERSION 4

[viboes]

I believe the problem is due to the fact that the continuation returns a future<void>. You will need to unwrap the returned future<future<void>>

    }).unwrap().then([&value, &tmpValue](boost::future<void> future){

However, I would like that your code doesn't compile as the continuation should had a boost::future<future<void>> parameter.

When I change the continuation as follows I get the expected error.

    }).then([&value, &tmpValue](boost::future<void>& future){

Note that the future is passed by reference.

This mean that the call to the continuation is getting the embedded future. I will check this.

[Jeffrey.Gramowski@…]

Replying to viboes:

Have you uncommented

//#define BOOST_THREAD_VERSION 4

Yes this is has been set in the header files for my boost version. I just wanted to document that it is set.

[viboes]

Have you seen my comment 7?

[Jeffrey.Gramowski@…]

Replying to viboes:

Have you seen my comment 7?

I tried the unwrap function and it works. But I still think that there is a problem. So either my code shouldn't compile and force the developer to use the unwrap function. Or if boost is going to do the "auto" unwrap functionality than it needs to do it in a way so that it is equivalent to calling the unwrap function.

For now I guess I will call the unwrap function but I still think that something needs to be done here. The current implementation can introduce very hard to find bugs in a system dealing with threads. This is how I ran into this problem. I was trying to track down a deadlock in some multi-threaded code which wasn't suppose to block and I tracked it down to this condition. Note I am also using the executor functionality so that I can control the threads and our threads are not allowed to block.

[viboes]

I agree that there is a problem. But is is not that the unwrap must be done implicitly.

Boost::future::then was defined before the C++ experimental std::experimental::future::then included in the TS and the original proposal behaved like Boost.Thread. Changing this will be a breaking change and people don't like that.

The problem is that the future<future<void>> is implicitly convertible to future<void>, which must be explicit. I will see what happens if I define the constructor explicit.

[viboes]

​https://github.com/boostorg/thread/commit/fc748dbddf33baf2afd1cf33920a6c0da8511f77

[viboes]

​https://github.com/boostorg/thread/commit/58c6b384cc80ae2af598ab48d16522e71b31ca09