-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57358/
-----------------------------------------------------------
(Updated March 7, 2017, 10:09 a.m.)
Review request for mesos, Benjamin Mahler, Jie Yu, and Michael Park.
Changes
-------
Need to rethink approach, as this change "fixes" the leak, but ends up changing
the semantics of `Future` and thereby fails a bunch of regression tests.
Summary (updated)
-----------------
WIP: Fixed potential for circular dependencies in Future continuations.
Bugs: MESOS-6919
https://issues.apache.org/jira/browse/MESOS-6919
Repository: mesos
Description
-------
This commit deals with a specific case where an object passes a
self-reference (shared_ptr) into a Future continuation and then
discards the original Future.
The behavior of `Future::discard` is that the `onDiscard` event
is only chained to the future immediately following the discarded
future. i.e.
```
Future<A> top;
top
.onDiscard([]() { /* This gets executed. */ })
.then([](const A&) { return Future<B>(); })
.onDiscard([]() { /* This also gets executed. */ })
.then([](const B&) { return Future<C>(); })
.onDiscard([]() { /* But not this. */ });
top.discard();
```
When a Future is discarded, we only clear the `onDiscard` callbacks,
which means that all other continuations will continue to reside in
memory. In the case of the `Socket` class, the Libevent SSL socket
implementation had several levels of continuations:
```
// Each item in this queue is backed by a Promise<>::Future.
Queue<Future<std::shared_ptr<SocketImpl>>> accept_queue;
// LibeventSSLSocketImpl::accept.
accept_queue.get()
.then(...)
// Socket::accept. This continuation holds a self-reference
// to the `shared_ptr<SocketImpl>`.
.then([self](...) {...})
```
Because the second continuation is never discarded nor called, we
end up with a dangling pointer. For the `Socket` class, this leads
to an FD leak.
Diffs
-----
3rdparty/libprocess/include/process/future.hpp
ec7ef2251947f5f42a202af0d6cb0858338e7755
Diff: https://reviews.apache.org/r/57358/diff/1/
Testing
-------
cmake .. -DENABLE_LIBEVENT=1 -DENABLE_SSL=1
make libprocess-tests
3rdparty/libprocess/src/tests/libprocess-tests
--gtest_filter="Scheme/HTTPTest.Endpoints/0" --gtest_repeat="`ulimit -n`"
Thanks,
Joseph Wu
