Opened 6 years ago

Closed 6 years ago

#12220 closed Bugs (fixed)

Memory leak in future::then()

Reported by: valentin.milea@… Owned by: viboes
Milestone: Boost 1.62.0 Component: thread
Version: Boost 1.61.0 Severity: Regression
Keywords: memory leak future then Cc:

Description

Since Boost 1.60, the continuation destructor is never called.

I've reproduced the leak using Boost 1.61 with the attached program in following configurations:

  • GCC 5.3.0 with BOOST_THREAD_VERSION=2, BOOST_THREAD_PROVIDES_FUTURE, BOOST_THREAD_PROVIDES_FUTURE_CONTINUATION
  • MSVC 14.0 with BOOST_THREAD_VERSION=2, BOOST_THREAD_PROVIDES_FUTURE, BOOST_THREAD_PROVIDES_FUTURE_CONTINUATION
  • MSVC 14.0 with BOOST_THREAD_VERSION=4

Interestingly, the leak disappears with GCC 5.3.0 when BOOST_THREAD_VERSION=4.

Also, there is no leak in any configuration for deferred launch policy.

Attachments (1)

future_then_leak.cpp (472 bytes ) - added by valentin.milea@… 6 years ago.

Download all attachments as: .zip

Change History (16)

by valentin.milea@…, 6 years ago

Attachment: future_then_leak.cpp added

comment:1 by Bloody Rookie, 6 years ago

I can confirm that for the mentioned Boost versions there is a problem with the lifetime of a future continuation. In Boost 1.60 the flag BOOST_THREAD_FUTURE_BLOCKING was introduced. For future continuations with async launch policy the flag is used in the future.hpp code in the following way:

#ifdef BOOST_THREAD_FUTURE_BLOCKING
             this->thr_ = thread(&future_async_shared_state::run, static_shared_from_this(this), boost::forward<Fp>(f));
#else
            thread(&future_async_shared_state::run, static_shared_from_this(this), boost::forward<Fp>(f)).detach();
#endif

If the flag is defined the thread is attached to the future_async_shared_state via the thr_ member variable. The thread holds a reference to the shared state in its thread_info member (note the static_shared_from_this(this) parameter). This leads to the situation that even after the thread finished its work making the future ready, the shared state still holds on to the thread and the thread structure still holds a reference to the shared state. This makes the future immortal and thus the resources are never released.

A workaround is to undefine the flag. This has the side effect that the destructor of the future won't block which sometimes is undesirable (and which was the reason the flag was introduced). Using deferred launch policy doesn't lead to the memory leak since no new thread is launched for the execution of the continuation.

Please respond to this ticket, a memory leak makes the continuations unusable.

comment:2 by viboes, 6 years ago

Owner: changed from Anthony Williams to viboes
Status: newassigned

comment:3 by viboes, 6 years ago

Sorry for been late. I will take a look at asap.

comment:4 by viboes, 6 years ago

Why do you say that" The thread holds a reference to the shared state in its thread_info member (note the static_shared_from_this(this) parameter)."? static_shared_from_this(this) is used as parameter of the run function and the parameter is released when the function ends, isn't it?

comment:5 by viboes, 6 years ago

See also #11951

comment:6 by BloodyRookie, 6 years ago

"static_shared_from_this(this) is used as parameter of the run function and the parameter is released when the function ends, isn't it?"

Where is the parameter released?

Suppose I have code like this

{
   auto f = boost::make_ready_future<int>(41).then([](boost::future<int>) { return 42; });
   f.get(); // wait for the future to complete
}

Then at some point

this->thr_ = thread(&future_async_shared_state::run, static_shared_from_this(this), boost::forward<Fp>(f));

Is called. The thread ctor calls

thread_info(make_thread_info(boost::bind(boost::type<void>(),f,a1)))

a1 is

boost::shared_ptr<boost::detail::future_async_continuation_shared_state<boost::future<int>,int,int <lambda>(boost::future<int>)>>

The result of boost::bind is stored in the thread_data member (actually the pointer thread_data_ptr keeps it alive)

After the future completes the situation has not changed, nothing is released imo. In the debugger I can see the following:

http://imgur.com/a/7u5ii

There are 2 strong references on the shared state, one is from the variable f but the other one is from the creation of the thread which is attached to the future. The dtor of the future is never called.

If I comment out the BOOST_THREAD_FUTURE_BLOCKING flag definition then at the same break point there is only 1 strong reference and the dtor of the future is called.

comment:7 by viboes, 6 years ago

I should use a weak_ptr instead but for the time been I don't reach to make it working. Any help much appreciated.

comment:8 by viboes, 6 years ago

See also #11951

comment:9 by viboes, 6 years ago

I have been trying to see how to fix these issues (See also #11951) and I don't know how to fix them if the future must be blocking.

As the Concurrent TS doesn't blocks in this cases, I believe that even if it is a breaking change I will create a new Boost.Thread version 5 that doesn't defines BOOST_THREAD_FUTURE_BLOCKING it by default.

What others think?

P.S. This couldn't be provided before Boost 1.63.

comment:10 by viboes, 6 years ago

I was thinking about this and instead of joining the thread I can just wait. IN this way I don't need to store the thread and the cycle will be broken.

comment:11 by viboes, 6 years ago

I have committed

https://github.com/boostorg/thread/commit/5450e98c6bd2515388de1d3d97431cf5374319e7

Please, could you tell me if this fixes the issue?

comment:12 by viboes, 6 years ago

Milestone: To Be DeterminedBoost 1.62.0

comment:13 by BloodyRookie, 6 years ago

I applied the patch and now the dtor is getting called indeed. Are there any disadvantages to the BOOST_THREAD_FUTURE_BLOCKING approach?

comment:14 by viboes, 6 years ago

Thanks for testing.

BOOST_THREAD_FUTURE_BLOCKING used the thread to join it. BOOST_THREAD_ASYNC_FUTURE_WAITS uses the future itself to wait. This avoid the shared_ptr cycle.

comment:15 by viboes, 6 years ago

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.