[2/2] mesos git commit: Fixed a subscriber FD leak when running tests.

[an...@apache.org]

Fixed a subscriber FD leak when running tests.

This FD leak would only surface when running tests. We hold on to
a reference of the \`Connection\` object in the client so that it is
not destroyed while the connection is active. When running tests,
the IP:Port of libprocess remain the same which means the objects
keep on accumulating. In a real world cluster, we remove the
subscriber upon noticing a \_disconnection\_ i.e. this means the
socket has already been already closed upstream by Libprocess on
the server side.

Review: https://reviews.apache.org/r/49844/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/44e10cb2
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/44e10cb2
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/44e10cb2

Branch: refs/heads/master
Commit: 44e10cb2a8665c13c3d98184f8c27a608309020f
Parents: 6ac4b23
Author: Anand Mazumdar <an...@apache.org>
Authored: Sun Jul 17 14:28:05 2016 -0700
Committer: Anand Mazumdar <an...@apache.org>
Committed: Sun Jul 17 14:28:05 2016 -0700

----------------------------------------------------------------------
 src/master/master.cpp | 20 ++++++++++----------
 src/master/master.hpp | 15 ++++++++++++++-
 2 files changed, 24 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/44e10cb2/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 61eaa4a..370fd87 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -7573,8 +7573,8 @@ void Master::Subscribers::send(const mesos::master::Event& event)
   VLOG(1) << "Notifying all active subscribers about " << event.type() << " "
           << "event";
 
-  foreachvalue (Subscriber subscriber, subscribed) {
-    subscriber.http.send<mesos::master::Event, v1::master::Event>(event);
+  foreachvalue (const Owned<Subscriber>& subscriber, subscribed) {
+    subscriber->http.send<mesos::master::Event, v1::master::Event>(event);
   }
 }
 
@@ -7592,18 +7592,18 @@ void Master::exited(const UUID& id)
 
 void Master::subscribe(HttpConnection http)
 {
-  Subscribers::Subscriber subscriber{http};
-
-  subscribers.subscribed.put(http.streamId, subscriber);
-
-  LOG(INFO) << "Added subscriber: " << subscriber.http.streamId << " to the "
+  LOG(INFO) << "Added subscriber: " << http.streamId << " to the "
             << "list of active subscribers";
 
-  subscriber.http.closed()
+  http.closed()
     .onAny(defer(self(),
-           [this, subscriber](const Future<Nothing>&) {
-             exited(subscriber.http.streamId);
+           [this, http](const Future<Nothing>&) {
+             exited(http.streamId);
            }));
+
+  subscribers.subscribed.put(
+      http.streamId,
+      Owned<Subscribers::Subscriber>(new Subscribers::Subscriber{http}));
 }
 
 } // namespace master {

http://git-wip-us.apache.org/repos/asf/mesos/blob/44e10cb2/src/master/master.hpp
----------------------------------------------------------------------
diff --git a/src/master/master.hpp b/src/master/master.hpp
index ac998b1..bade8af 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -1754,6 +1754,19 @@ private:
       Subscriber(const HttpConnection& _http)
         : http(_http) {}
 
+      // Not copyable, not assignable.
+      Subscriber(const Subscriber&) = delete;
+      Subscriber& operator=(const Subscriber&) = delete;
+
+      ~Subscriber()
+      {
+        // TODO(anand): Refactor `HttpConnection` to being a RAII class instead.
+        // It is possible that a caller might accidently invoke `close()` after
+        // passing ownership to the `Subscriber` object. See MESOS-5843 for more
+        // details.
+        http.close();
+      }
+
       HttpConnection http;
     };
 
@@ -1762,7 +1775,7 @@ private:
 
     // Active subscribers to the 'api/vX' endpoint keyed by the stream
     // identifier.
-    hashmap<UUID, Subscriber> subscribed;
+    hashmap<UUID, process::Owned<Subscriber>> subscribed;
   } subscribers;
 
   hashmap<OfferID, Offer*> offers;