mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <>
Subject Re: Review Request 57358: Implemented discard behavior in process::Queue.
Date Wed, 15 Mar 2017 18:01:28 GMT

This is an automatically generated e-mail. To reply, visit:

(Updated March 15, 2017, 11:01 a.m.)

Review request for mesos, Benjamin Mahler and Jie Yu.


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

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;
  .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. */ });


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.

// 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 





make libprocess-tests

3rdparty/libprocess/src/tests/libprocess-tests --gtest_filter="Scheme/HTTPTest.Endpoints/0"
--gtest_repeat="`ulimit -n`"

make check


Joseph Wu

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