Boost C++ Libraries: Ticket #11256: future<>::is_ready() == false in continuation function

owner, status changed

viboes — Fri, 01 May 2015 15:46:01 GMT

owner changed from Anthony Williams to viboes
status new → assigned

I will take a look.

A public example able to reproduce the issue will be welcome.

Please, could you tell me which version are you using? develop, master or the released boost1.58?

Which compiler and platform?

Fri, 01 May 2015 22:24:34 GMT

I'll try to put together an example to reproduce; I had very limited time when the issue popped up, and I couldn't get a simple example to fail.

I'll also try and trace calls in future.hpp to get an understanding of what logic leads to the continuation being triggered prematurely.

I'm using released Boost 1.58 patched with fix for future<>::then with executors not compiling; the future.hpp is effectively the one from commit 0bed674 . I'm using 64-bit Ubuntu 14.10 with GCC 4.9.1 .

viboes — Sat, 02 May 2015 08:59:06 GMT

I've reached to reproduce it.

Sat, 02 May 2015 19:10:29 GMT

Below is the simplest code I got to reproduce if anyone else is curious. This version fails on assert, but when you remove .unwrap() it deadlocks instead (in this particular version, boost::lock throws exception saying "resource deadlock avoided", but my other code just deadlocks).

#define BOOST_THREAD_VERSION 4
#define BOOST_THREAD_PROVIDES_EXECUTORS
#include <boost/thread.hpp>
#include <boost/thread/thread_pool.hpp>
#include <cassert>
auto createFuture()
{
    boost::promise<void> promise;
    promise.set_value();
    return promise.get_future();
}
auto stepOne(boost::basic_thread_pool &executor)
{
    auto sendFuture = createFuture();
    auto wrappedFuture = sendFuture.then(executor, [](auto f) mutable {
        return createFuture();
    });
    return wrappedFuture.unwrap();
}
auto stepTwo(boost::basic_thread_pool &executor)
{
    auto future = stepOne(executor);
    return future.then(executor, [](auto f) {
        assert(f.is_ready());
    });
}
int main()
{
    boost::basic_thread_pool executor{1};
    stepTwo(executor).get();
}

Sat, 02 May 2015 19:21:34 GMT

Here's the trace for the boost::lock exception (Boost.Thread from commit e598796eaf, i.e. current develop):

terminate called after throwing an instance of 'boost::exception_detail::clone_impl<boost::exception_detail::error_info_injector<boost::lock_error> >'
  what():  boost unique_lock owns already the mutex: Resource deadlock avoided
Program received signal SIGABRT, Aborted.
0x00007ffff70ffe37 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
56      ../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  0x00007ffff70ffe37 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  0x00007ffff7101528 in __GI_abort () at abort.c:89
#2  0x00007ffff770505d in __gnu_cxx::__verbose_terminate_handler() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#3  0x00007ffff7702ed6 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#4  0x00007ffff7702f21 in std::terminate() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#5  0x00007ffff7703139 in __cxa_throw () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#6  0x0000000000411f65 in boost::throw_exception<boost::lock_error> (e=...) at /usr/include/boost/throw_exception.hpp:69
#7  0x00000000004146e6 in boost::unique_lock<boost::mutex>::lock (this=0x7fffffffe480) at thread/include/boost/thread/lock_types.hpp:343
#8  0x000000000040adcb in boost::detail::make_future_executor_continuation_shared_state<boost::executors::basic_thread_pool, boost::future<boost::future<void> >, void, stepTwo(boost::executors::basic_thread_pool&)::<lambda(auto:2)> >(boost::executors::basic_thread_pool &, boost::unique_lock<boost::mutex> &, <unknown type in /home/kzemek/plgrid/helpers/example, CU 0x0, DIE 0x3d241>, <unknown type in /home/kzemek/plgrid/helpers/example, CU 0x0, DIE 0x3d21b>) (ex=..., lock=...,
    f=<unknown type in /home/kzemek/plgrid/helpers/example, CU 0x0, DIE 0x3d241>, c=<unknown type in /home/kzemek/plgrid/helpers/example, CU 0x0, DIE 0x3d21b>)
    at thread/include/boost/thread/future.hpp:4703
#9  0x000000000040aad0 in boost::future<boost::future<void> >::then<boost::executors::basic_thread_pool, stepTwo(boost::executors::basic_thread_pool&)::<lambda(auto:2)> >(boost::executors::basic_thread_pool &, <unknown type in /home/kzemek/plgrid/helpers/example, CU 0x0, DIE 0x37065>) (this=0x7fffffffe4e0, ex=...,
    func=<unknown type in /home/kzemek/plgrid/helpers/example, CU 0x0, DIE 0x37065>) at thread/include/boost/thread/future.hpp:4874
#10 0x000000000040a872 in stepTwo (executor=...) at example.cpp:30
#11 0x000000000040a901 in main () at example.cpp:36

Sat, 02 May 2015 19:53:02 GMT

This simple change seems to fix the problems: https://github.com/kzemek/thread/commit/f4c65a2f6ebb0f0edbf83272b6d8444101efb39e

This just introduces the same locking logic as with creating a non-nested future continuation (future.hpp:4803).

viboes — Sun, 03 May 2015 12:35:05 GMT

I've committed some changes concerning the missed lock.unlock (https://github.com/boostorg/thread/commit/5205fa71bfc06a8eac22fc2a49357d1db1ef505f).

I've added also your example, but this doesn't solves the is_ready issue. So I have created a new ticket #11263.

IN the test_10964 there are yet some cases that doesn't work.

viboes — Sun, 03 May 2015 14:48:56 GMT

I'm not yet sure but I believe that I find where the bug is. A unwrapped future shouldn't is not a continuation so no need for the adding it self as a continuation at the construction time

  template <class F, class Rp>
  BOOST_THREAD_FUTURE<Rp>
  make_future_unwrap_shared_state(boost::unique_lock<boost::mutex> &lock, BOOST_THREAD_RV_REF(F) f) {
    shared_ptr<future_unwrap_shared_state<F, Rp> >
        h(new future_unwrap_shared_state<F, Rp>(boost::move(f)));
//    lock.lock();
//    h->parent.future_->set_continuation_ptr(h, lock);
//    lock.unlock();
    return BOOST_THREAD_FUTURE<Rp>(h);
  }

In addition the name of the wrapped future was misleading (parent). The code

    virtual void set_continuation_ptr(continuation_ptr_type continuation, boost::unique_lock<boost::mutex>& lock)
    {
      boost::unique_lock<boost::mutex> lk(wrapped.parent.future_->mutex);
      parent.future_->set_continuation_ptr(continuation, lk);
    }

was setting a continuation on the wrapped (and already ready future, not on the parent one).

I've renamed the parent member to wrapped. The code should be

    virtual void set_continuation_ptr(continuation_ptr_type continuation, boost::unique_lock<boost::mutex>& lock)
    {
      boost::unique_lock<boost::mutex> lk(wrapped.parent.future_->mutex);
      wrapped.get().future_->set_continuation_ptr(continuation, lk);
    }

Please, could you try with this change

  template<typename F, typename Rp>
  struct future_unwrap_shared_state: shared_state<Rp>
  {
    F wrapped;
  public:
    explicit future_unwrap_shared_state(BOOST_THREAD_RV_REF(F) f)
    : wrapped(boost::move(f)) {}
    typename F::value_type parent_value(boost::unique_lock<boost::mutex>& ) {
        typename F::value_type r = wrapped.get();
        r.set_exceptional_if_invalid();
        return boost::move(r);
    }
    virtual void wait(boost::unique_lock<boost::mutex>& lk, bool ) { // todo see if rethrow must be used
        parent_value(lk).wait();
    }
    virtual Rp get(boost::unique_lock<boost::mutex>& lk) {
        return parent_value(lk).get();
    }
#if defined BOOST_THREAD_PROVIDES_FUTURE_CONTINUATION
    typedef shared_ptr<shared_state_base> continuation_ptr_type;
    virtual void set_continuation_ptr(continuation_ptr_type continuation, boost::unique_lock<boost::mutex>& lock)
    {
      boost::unique_lock<boost::mutex> lk(wrapped.parent.future_->mutex);
      wrapped.get().future_->set_continuation_ptr(continuation, lk);
    }
#endif
  };
  template <class F, class Rp>
  BOOST_THREAD_FUTURE<Rp>
  make_future_unwrap_shared_state(boost::unique_lock<boost::mutex> &lock, BOOST_THREAD_RV_REF(F) f) {
    shared_ptr<future_unwrap_shared_state<F, Rp> >
        h(new future_unwrap_shared_state<F, Rp>(boost::move(f)));
//    lock.lock();
//    h->parent.future_->set_continuation_ptr(h, lock);
//    lock.unlock();
    return BOOST_THREAD_FUTURE<Rp>(h);
  }
}

viboes — Sun, 03 May 2015 15:21:59 GMT

Sorry, the previous code don't work neither nor compile :(

Sun, 03 May 2015 15:45:34 GMT

I assumed it should be simply wrapped.future_->mutex and changed it as such to avoid compilation error. Using the modified code with the example I posted above results in a deadlock:

Thread 2 (Thread 0x7ffff699c700 (LWP 172)):
#0  __lll_lock_wait () at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:135
#1  0x00007ffff6cad56a in _L_lock_913 () from /lib/x86_64-linux-gnu/libpthread.so.0
#2  0x00007ffff6cad390 in __GI___pthread_mutex_lock (mutex=0x64a6b0) at ../nptl/pthread_mutex_lock.c:79
#3  0x000000000040de34 in pthread_mutex_lock (m=0x64a6b0) at thread/include/boost/thread/pthread/mutex.hpp:62
#4  boost::mutex::lock (this=0x64a6b0) at thread/include/boost/thread/pthread/mutex.hpp:116
#5  0x0000000000414a01 in boost::unique_lock<boost::mutex>::lock (this=0x7ffff699bc60) at thread/include/boost/thread/lock_types.hpp:346
#6  0x00000000004128ed in boost::unique_lock<boost::mutex>::unique_lock (this=0x7ffff699bc60, m_=...) at thread/include/boost/thread/lock_types.hpp:124
#7  0x000000000042b983 in boost::detail::shared_state<boost::future<void> >::mark_finished_with_result(boost::future<void>&&) (this=0x64a670, result_=<unknown type in /home/kzemek/plgrid/helpers/example, CU 0x0, DIE 0x1b312>)
    at thread/include/boost/thread/future.hpp:551
#8  0x000000000040c9c3 in boost::detail::future_executor_continuation_shared_state<boost::executors::basic_thread_pool, boost::future<void>, boost::future<void>, stepOne(boost::executors::basic_thread_pool&)::<lambda(auto:1)> >::run(boost::shared_ptr<boost::detail::shared_state_base>) (that_=...) at thread/include/boost/thread/future.hpp:4303
#9  0x000000000040c82a in boost::detail::run_it<boost::detail::future_executor_continuation_shared_state<boost::executors::basic_thread_pool, boost::future<void>, boost::future<void>, stepOne(boost::executors::basic_thread_pool&)::<lambda(auto:1)> > >::operator()(void) (this=0x64a838) at thread/include/boost/thread/future.hpp:4274
#10 0x000000000040c79c in boost::detail::nullary_function<void()>::impl_type<boost::detail::run_it<boost::detail::future_executor_continuation_shared_state<boost::executors::basic_thread_pool, boost::future<void>, boost::future<void>, stepOne(boost::executors::basic_thread_pool&)::<lambda(auto:1)> > > >::call(void) (this=0x64a830) at thread/include/boost/thread/detail/nullary_function.hpp:52
#11 0x000000000040ec67 in boost::detail::nullary_function<void ()>::operator()() (this=0x7ffff699bdd0) at thread/include/boost/thread/detail/nullary_function.hpp:125
#12 0x00000000004119ed in boost::executors::basic_thread_pool::worker_thread (this=0x7fffffffe530) at thread/include/boost/thread/executors/basic_thread_pool.hpp:91
#13 0x000000000042ada9 in boost::detail::invoke<void (boost::executors::basic_thread_pool::*)(), boost::executors::basic_thread_pool<>*>(void (boost::executors::basic_thread_pool::*&&)(), boost::executors::basic_thread_pool<>*&&, (boost::executors::basic_thread_pool<>*&&)...) (f=<unknown type in /home/kzemek/plgrid/helpers/example, CU 0x0, DIE 0x4cba2>, a0=<unknown type in /home/kzemek/plgrid/helpers/example, CU 0x0, DIE 0x4cba7>)
    at thread/include/boost/thread/detail/invoke.hpp:77
#14 0x0000000000429815 in boost::detail::thread_data<void (boost::executors::basic_thread_pool::*)(), boost::executors::basic_thread_pool*>::run2<1ul> (this=0x64a0f0) at thread/include/boost/thread/detail/thread.hpp:75
#15 0x00000000004278c6 in boost::detail::thread_data<void (boost::executors::basic_thread_pool::*)(), boost::executors::basic_thread_pool*>::run (this=0x64a0f0) at thread/include/boost/thread/detail/thread.hpp:81
#16 0x00007ffff7bc7d8a in ?? () from /usr/lib/x86_64-linux-gnu/libboost_thread.so.1.58.0
#17 0x00007ffff6cab0a5 in start_thread (arg=0x7ffff699c700) at pthread_create.c:309
#18 0x00007ffff71c2cfd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111
Thread 1 (Thread 0x7ffff7fe6780 (LWP 168)):
#0  __lll_lock_wait () at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:135
#1  0x00007ffff6cad56a in _L_lock_913 () from /lib/x86_64-linux-gnu/libpthread.so.0
#2  0x00007ffff6cad390 in __GI___pthread_mutex_lock (mutex=0x64a6b0) at ../nptl/pthread_mutex_lock.c:79
#3  0x000000000040de34 in pthread_mutex_lock (m=0x64a6b0) at thread/include/boost/thread/pthread/mutex.hpp:62
#4  boost::mutex::lock (this=0x64a6b0) at thread/include/boost/thread/pthread/mutex.hpp:116
#5  0x0000000000414a01 in boost::unique_lock<boost::mutex>::lock (this=0x7fffffffe2e0) at thread/include/boost/thread/lock_types.hpp:346
#6  0x00000000004128ed in boost::unique_lock<boost::mutex>::unique_lock (this=0x7fffffffe2e0, m_=...) at thread/include/boost/thread/lock_types.hpp:124
#7  0x000000000042993c in boost::future<boost::future<void> >::get (this=0x64a998) at thread/include/boost/thread/future.hpp:1868
#8  0x0000000000427ac9 in boost::detail::future_unwrap_shared_state<boost::future<boost::future<void> >, void>::set_continuation_ptr (this=0x64a890, continuation=..., lock=...) at thread/include/boost/thread/future.hpp:5068
#9  0x000000000040ae5c in boost::detail::make_future_executor_continuation_shared_state<boost::executors::basic_thread_pool, boost::future<void>, void, stepTwo(boost::executors::basic_thread_pool&)::<lambda(auto:2)> >(boost::executors::basic_thread_pool &, boost::unique_lock<boost::mutex> &, <unknown type in /home/kzemek/plgrid/helpers/example, CU 0x0, DIE 0x3dc9c>, <unknown type in /home/kzemek/plgrid/helpers/example, CU 0x0, DIE 0x3dc76>) (ex=..., lock=...,
    f=<unknown type in /home/kzemek/plgrid/helpers/example, CU 0x0, DIE 0x3dc9c>, c=<unknown type in /home/kzemek/plgrid/helpers/example, CU 0x0, DIE 0x3dc76>) at thread/include/boost/thread/future.hpp:4704
#10 0x000000000040ab0c in boost::future<void>::then<boost::executors::basic_thread_pool, stepTwo(boost::executors::basic_thread_pool&)::<lambda(auto:2)> >(boost::executors::basic_thread_pool &, <unknown type in /home/kzemek/plgrid/helpers/example, CU 0x0, DIE 0x379fd>) (this=0x7fffffffe4e0, ex=..., func=<unknown type in /home/kzemek/plgrid/helpers/example, CU 0x0, DIE 0x379fd>) at thread/include/boost/thread/future.hpp:4804
#11 0x000000000040a8a2 in stepTwo (executor=...) at example.cpp:30
#12 0x000000000040a931 in main () at example.cpp:36

viboes — Sun, 03 May 2015 18:49:42 GMT

The following design seems to work well except for the example future_unwrap.cpp.

The unwrap shared state adds a continuation on the wrapped shared state. There is no redefinition for wait or get neither the set_continuation_ptr. Please could you try it?

  template<typename F, typename Rp>
  struct future_unwrap_shared_state: shared_state<Rp>
  {
    F wrapped;
    typename F::value_type unwrapped;
  public:
    explicit future_unwrap_shared_state(BOOST_THREAD_RV_REF(F) f)
    : wrapped(boost::move(f)) {
    }
    void launch_continuation(boost::unique_lock<boost::mutex>& lk, shared_ptr<shared_state_base> that)
    {
      if (! unwrapped.valid() )
      {
        unwrapped = wrapped.get();
        if (unwrapped.valid())
        {
          boost::unique_lock<boost::mutex> lk2(unwrapped.future_->mutex);
          unwrapped.future_->set_continuation_ptr(this->shared_from_this(), lk2);
        } else {
          this->mark_exceptional_finish_internal(boost::copy_exception(future_uninitialized()), lk);
        }
      } else {
        this->mark_finished_with_result_internal(unwrapped.get(), lk);
      }
    }
  };
  template<typename F>
  struct future_unwrap_shared_state<F,void>: shared_state<void>
  {
    F wrapped;
    typename F::value_type unwrapped;
  public:
    explicit future_unwrap_shared_state(BOOST_THREAD_RV_REF(F) f)
    : wrapped(boost::move(f)) {
    }
    void launch_continuation(boost::unique_lock<boost::mutex>& lk, shared_ptr<shared_state_base> that)
    {
      if (! unwrapped.valid() )
      {
        unwrapped = wrapped.get();
        if (unwrapped.valid())
        {
          lk.unlock();
          boost::unique_lock<boost::mutex> lk2(unwrapped.future_->mutex);
          unwrapped.future_->set_continuation_ptr(this->shared_from_this(), lk2);
        } else {
          this->mark_exceptional_finish_internal(boost::copy_exception(future_uninitialized()), lk);
        }
      } else {
        unwrapped.wait();
        this->mark_finished_with_result_internal(lk);
      }
    }
  };
  template <class F, class Rp>
  BOOST_THREAD_FUTURE<Rp>
  make_future_unwrap_shared_state(boost::unique_lock<boost::mutex> &lock, BOOST_THREAD_RV_REF(F) f) {
    shared_ptr<future_unwrap_shared_state<F, Rp> >
        h(new future_unwrap_shared_state<F, Rp>(boost::move(f)));
    lock.lock();
    h->wrapped.future_->set_continuation_ptr(h, lock);
    lock.unlock();
    return BOOST_THREAD_FUTURE<Rp>(h);
  }

viboes — Sun, 03 May 2015 18:59:28 GMT

Found. A missing unlock here

  template<typename F, typename Rp>
  struct future_unwrap_shared_state: shared_state<Rp>
  {
    F wrapped;
    typename F::value_type unwrapped;
  public:
    explicit future_unwrap_shared_state(BOOST_THREAD_RV_REF(F) f)
    : wrapped(boost::move(f)) {
    }
    void launch_continuation(boost::unique_lock<boost::mutex>& lk, shared_ptr<shared_state_base> that)
    {
      if (! unwrapped.valid() )
      {
        unwrapped = wrapped.get();
        if (unwrapped.valid())
        {

Sun, 03 May 2015 20:22:29 GMT

This does seem to fix assert(f.is_ready()) problem, but unwrapped = wrapped.get(); throws if wrapped has been fulfilled with an exception.

viboes — Sun, 03 May 2015 20:34:08 GMT

This is normal.

viboes — Sun, 03 May 2015 20:34:15 GMT

https://github.com/boostorg/thread/commit/b7b2a463cfebdaeb697fbb667858c5fd35998f14

viboes — Sun, 03 May 2015 20:36:11 GMT

Oh, I see what you mean. In the implementation I need to take care of this exception. And propagate it. I will do it.

Sun, 03 May 2015 20:39:55 GMT

Yeah, sorry for not being clearer. :)

viboes — Sun, 03 May 2015 20:53:26 GMT

Not tested yet

  template<typename F, typename Rp>
  struct future_unwrap_shared_state: shared_state<Rp>
  {
    F wrapped;
    typename F::value_type unwrapped;
  public:
    explicit future_unwrap_shared_state(BOOST_THREAD_RV_REF(F) f)
    : wrapped(boost::move(f)) {
    }
    void launch_continuation(boost::unique_lock<boost::mutex>& lk, shared_ptr<shared_state_base> that)
    {
      if (! unwrapped.valid() )
      {
        if (wrapped.has_exception()) {
          this->mark_exceptional_finish_internal(wrapped.get_exception_ptr(), lk);
        } else {
          unwrapped = wrapped.get();
          if (unwrapped.valid())
          {
            lk.unlock();
            boost::unique_lock<boost::mutex> lk2(unwrapped.future_->mutex);
            unwrapped.future_->set_continuation_ptr(this->shared_from_this(), lk2);
          } else {
            this->mark_exceptional_finish_internal(boost::copy_exception(future_uninitialized()), lk);
          }
        }
      } else {
        if (unwrapped.has_exception()) {
          this->mark_exceptional_finish_internal(unwrapped.get_exception_ptr(), lk);
        } else {
          this->mark_finished_with_result_internal(unwrapped.get(), lk);
        }
      }
    }
  };

viboes — Sun, 03 May 2015 20:54:14 GMT

https://github.com/boostorg/thread/commit/71d9a0a12010583a59548dc32bb1cb84f16a5d56

milestone changed

viboes — Sun, 03 May 2015 20:55:34 GMT

milestone To Be Determined → Boost 1.59.0

Sun, 03 May 2015 21:19:34 GMT

It does work in my tests. Thanks for the great work! :)

milestone changed

viboes — Thu, 13 Aug 2015 06:59:18 GMT

milestone Boost 1.59.0 → Boost 1.60.0

status changed; resolution set

viboes — Wed, 02 Sep 2015 18:10:16 GMT

status assigned → closed
resolution → fixed

https://github.com/boostorg/thread/commit/a3497e1ffceb14318ff09e339a53293926351057