[2/2] mesos git commit: Made resource provider driver start explicit.

[bb...@apache.org]

Made resource provider driver start explicit.

The driver for HTTP connections of resource providers is implemented
as an actor which takes callbacks. It previously started listening for
and handling of events on creation. We typically store drivers as
member of resource providers.

This setup is in general problematic since it e.g., becomes impossible
to write safe callbacks using internal resource provider state; it was
e.g., impossible to write a safe callback using the member driver to
send calls. This is due to a race between initialization of the member
driver and a newly created driver starting to handle events and
triggering callbacks making use of a not yet initialized member driver
variable. As a concrete example, with a 'MockResourceProvider' holding
a 'std::unique_ptr<Driver>' member tests would regularly hit this
race, even though the time window between creating a temporary to
assign to the member and the assignment appeared to be small.

This patch introduces an explicit 'start' method to the driver which
is to be used to explicitly start processing of events by the driver.

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


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

Branch: refs/heads/master
Commit: aaef8563f0c1c149758975103c7066b7e0e81705
Parents: 66dbf99
Author: Benjamin Bannier <be...@mesosphere.io>
Authored: Fri Dec 15 17:55:03 2017 +0100
Committer: Benjamin Bannier <bb...@apache.org>
Committed: Fri Dec 15 17:55:03 2017 +0100

----------------------------------------------------------------------
 include/mesos/v1/resource_provider.hpp     |  2 ++
 src/resource_provider/driver.cpp           |  6 ++++++
 src/resource_provider/http_connection.hpp  | 12 ++++++------
 src/resource_provider/storage/provider.cpp |  2 ++
 src/tests/mesos.hpp                        |  2 ++
 5 files changed, 18 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/aaef8563/include/mesos/v1/resource_provider.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/v1/resource_provider.hpp b/include/mesos/v1/resource_provider.hpp
index 006889a..787c619 100644
--- a/include/mesos/v1/resource_provider.hpp
+++ b/include/mesos/v1/resource_provider.hpp
@@ -84,6 +84,8 @@ public:
 
   ~Driver();
 
+  void start() const;
+
   Driver(const Driver& other) = delete;
   Driver& operator=(const Driver& other) = delete;
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/aaef8563/src/resource_provider/driver.cpp
----------------------------------------------------------------------
diff --git a/src/resource_provider/driver.cpp b/src/resource_provider/driver.cpp
index 62c4ca1..70b7e32 100644
--- a/src/resource_provider/driver.cpp
+++ b/src/resource_provider/driver.cpp
@@ -86,6 +86,12 @@ Future<Nothing> Driver::send(const Call& call)
   return dispatch(process.get(), &DriverProcess::send, call);
 }
 
+
+void Driver::start() const
+{
+  return dispatch(process.get(), &DriverProcess::start);
+}
+
 } // namespace resource_provider {
 } // namespace v1 {
 } // namespace mesos {

http://git-wip-us.apache.org/repos/asf/mesos/blob/aaef8563/src/resource_provider/http_connection.hpp
----------------------------------------------------------------------
diff --git a/src/resource_provider/http_connection.hpp b/src/resource_provider/http_connection.hpp
index 207120a..3d5088d 100644
--- a/src/resource_provider/http_connection.hpp
+++ b/src/resource_provider/http_connection.hpp
@@ -162,18 +162,18 @@ public:
               lambda::_1));
   }
 
+  void start()
+  {
+    detection = detector->detect(None())
+      .onAny(defer(self(), &Self::detected, lambda::_1));
+  }
+
 protected:
   // Because we're deriving from a templated base class, we have
   // to explicitly bring these hidden base class names into scope.
   using process::Process<HttpConnectionProcess<Call, Event>>::self;
   typedef HttpConnectionProcess<Call, Event> Self;
 
-  void initialize() override
-  {
-    detection = detector->detect(None())
-      .onAny(defer(self(), &Self::detected, lambda::_1));
-  }
-
   void finalize() override
   {
     disconnect();

http://git-wip-us.apache.org/repos/asf/mesos/blob/aaef8563/src/resource_provider/storage/provider.cpp
----------------------------------------------------------------------
diff --git a/src/resource_provider/storage/provider.cpp b/src/resource_provider/storage/provider.cpp
index 17acf1b..5e7cb5e 100644
--- a/src/resource_provider/storage/provider.cpp
+++ b/src/resource_provider/storage/provider.cpp
@@ -647,6 +647,8 @@ Future<Nothing> StorageLocalResourceProviderProcess::recover()
           }),
           None())); // TODO(nfnt): Add authentication as part of MESOS-7854.
 
+      driver->start();
+
       return Nothing();
     }));
 }

http://git-wip-us.apache.org/repos/asf/mesos/blob/aaef8563/src/tests/mesos.hpp
----------------------------------------------------------------------
diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp
index 6207d62..41f47cf 100644
--- a/src/tests/mesos.hpp
+++ b/src/tests/mesos.hpp
@@ -2967,6 +2967,8 @@ public:
             this,
             lambda::_1),
         credential));
+
+    driver->start();
   }
 
   void connectedDefault()