mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Review Request 57358: Fixed potential for circular dependencies in Future continuations.
Date Tue, 07 Mar 2017 02:50:45 GMT

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

Review request for mesos, Benjamin Mahler, Jie Yu, and Michael Park.


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


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