-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57358/
-----------------------------------------------------------
(Updated March 15, 2017, 11:01 a.m.)
Review request for mesos, Benjamin Mahler and Jie Yu.
Changes
-------
Changed approach. This commit now modifies `process::Queue` and adds logic to
handle discarded futures in the Queue itself.
Summary (updated)
-----------------
Implemented discard behavior in process::Queue.
Bugs: MESOS-6919
https://issues.apache.org/jira/browse/MESOS-6919
Repository: mesos
Description (updated)
-------
This commit deals with a specific case where the SocketImpl class
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.
This commit fixes the self-reference by implementing discard
handlers in the libprocess Queue class. When a waiting `Queue::get`
is discarded, the Promise backing the `Queue::get` is lazily dropped
and cleaned up. Data on the queue will then skip these discarded
Promises and satisfy the next pending waiter.
Diffs (updated)
-----
3rdparty/libprocess/include/process/queue.hpp
ab08e30df742412f22a06202526f8b55350ed435
3rdparty/libprocess/src/process.cpp f6ee24e2db43d63d91222549efee85421bbf9bf3
3rdparty/libprocess/src/tests/queue_tests.cpp
95b738133fa50641f8f9b83014837d2808e0e4ff
Diff: https://reviews.apache.org/r/57358/diff/3/
Changes: https://reviews.apache.org/r/57358/diff/2-3/
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`"
make check
Thanks,
Joseph Wu
