----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57358/#review168837 -----------------------------------------------------------
Do you still need this? - Benjamin Mahler On March 9, 2017, 8:53 p.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57358/ > ----------------------------------------------------------- > > (Updated March 9, 2017, 8:53 p.m.) > > > Review request for mesos, Benjamin Mahler and Jie Yu. > > > Bugs: MESOS-6919 > https://issues.apache.org/jira/browse/MESOS-6919 > > > Repository: mesos > > > Description > ------- > > 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 > ----- > > 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/ > > > 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 > >
