-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57358/
-----------------------------------------------------------
(Updated March 7, 2017, 12:18 p.m.)
Review request for mesos, Benjamin Mahler, Jie Yu, and Michael Park.
Changes
-------
Different approach, which doesn't break tests, but slightly modifies the
semantics of `Socket::accept`. The assumption is that, when you discard the
future from `Socket::accept`, you are preparing to clean up the socket and
thereby do not care if the socket gives you inconsistent results.
Summary (updated)
-----------------
Fixed FD leak in SSL server socket cleanup.
Bugs: MESOS-6919
https://issues.apache.org/jira/browse/MESOS-6919
Repository: mesos
Description (updated)
-------
Fixed FD leak in SSL server socket cleanup.
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 extends the future returned by the Libevent SSL Socket
to deallocate some extra memory upon being discarded. This will
remove the extra reference to the SocketImpl class and thereby
resolve the issue of a dangling pointer.
Diffs (updated)
-----
3rdparty/libprocess/src/libevent_ssl_socket.hpp
9493a50243340a1c48ab1c67f6e61cc081ef24ce
3rdparty/libprocess/src/libevent_ssl_socket.cpp
7d493301bd5c0f24bf89e0b213f07ffe7801508b
Diff: https://reviews.apache.org/r/57358/diff/2/
Changes: https://reviews.apache.org/r/57358/diff/1-2/
Testing (updated)
-------
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
