mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 57358: WIP: Fixed FD leak in SSL server socket cleanup.
Date Mon, 13 Mar 2017 22:34:09 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57358/
-----------------------------------------------------------

(Updated March 13, 2017, 3:34 p.m.)


Review request for mesos, Benjamin Mahler and Jie Yu.


Summary (updated)
-----------------

WIP: Fixed FD leak in SSL server socket cleanup.


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message