You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by al...@apache.org on 2017/11/13 20:53:12 UTC

[2/4] mesos git commit: Refactored executor test driver to avoid using uninitialized object.

Refactored executor test driver to avoid using uninitialized object.

The shared pointer to MockHTTPExecutor is initialized after the library,
while the library may start using it right after its initialization via
the `events` callback. This change removes the shared pointer altogether
and hence prevents possible segfaults.

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


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

Branch: refs/heads/master
Commit: f221d8ebb1846e43d8c319bdcaf2694b1601f55b
Parents: 75f1274
Author: Alexander Rukletsov <ru...@gmail.com>
Authored: Mon Nov 13 21:52:22 2017 +0100
Committer: Alexander Rukletsov <al...@apache.org>
Committed: Mon Nov 13 21:52:22 2017 +0100

----------------------------------------------------------------------
 src/tests/mesos.hpp | 95 ++++++++++++++++++++----------------------------
 1 file changed, 40 insertions(+), 55 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/f221d8eb/src/tests/mesos.hpp
----------------------------------------------------------------------
diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp
index 30c8108..495c5a0 100644
--- a/src/tests/mesos.hpp
+++ b/src/tests/mesos.hpp
@@ -2483,36 +2483,41 @@ public:
   MOCK_METHOD2_T(acknowledged,
                  void(Mesos*, const typename Event::Acknowledged&));
 
-  void event(Mesos* mesos, const Event& event)
+  void events(Mesos* mesos, std::queue<Event> events)
   {
-    switch (event.type()) {
-      case Event::SUBSCRIBED:
-        subscribed(mesos, event.subscribed());
-        break;
-      case Event::LAUNCH:
-        launch(mesos, event.launch());
-        break;
-      case Event::LAUNCH_GROUP:
-        launchGroup(mesos, event.launch_group());
-        break;
-      case Event::KILL:
-        kill(mesos, event.kill());
-        break;
-      case Event::ACKNOWLEDGED:
-        acknowledged(mesos, event.acknowledged());
-        break;
-      case Event::MESSAGE:
-        message(mesos, event.message());
-        break;
-      case Event::SHUTDOWN:
-        shutdown(mesos);
-        break;
-      case Event::ERROR:
-        error(mesos, event.error());
-        break;
-      case Event::UNKNOWN:
-        LOG(FATAL) << "Received unexpected UNKNOWN event";
-        break;
+    while (!events.empty()) {
+      Event event = std::move(events.front());
+      events.pop();
+
+      switch (event.type()) {
+        case Event::SUBSCRIBED:
+          subscribed(mesos, event.subscribed());
+          break;
+        case Event::LAUNCH:
+          launch(mesos, event.launch());
+          break;
+        case Event::LAUNCH_GROUP:
+          launchGroup(mesos, event.launch_group());
+          break;
+        case Event::KILL:
+          kill(mesos, event.kill());
+          break;
+        case Event::ACKNOWLEDGED:
+          acknowledged(mesos, event.acknowledged());
+          break;
+        case Event::MESSAGE:
+          message(mesos, event.message());
+          break;
+        case Event::SHUTDOWN:
+          shutdown(mesos);
+          break;
+        case Event::ERROR:
+          error(mesos, event.error());
+          break;
+        case Event::UNKNOWN:
+          LOG(FATAL) << "Received unexpected UNKNOWN event";
+          break;
+      }
     }
   }
 };
@@ -2526,39 +2531,19 @@ class TestMesos : public Mesos
 public:
   TestMesos(
       ContentType contentType,
-      const std::shared_ptr<MockHTTPExecutor<Mesos, Event>>& _executor)
+      const std::shared_ptr<MockHTTPExecutor<Mesos, Event>>& executor)
     : Mesos(
           contentType,
           lambda::bind(&MockHTTPExecutor<Mesos, Event>::connected,
-                       _executor,
+                       executor,
                        this),
           lambda::bind(&MockHTTPExecutor<Mesos, Event>::disconnected,
-                       _executor,
+                       executor,
                        this),
-          lambda::bind(&TestMesos<Mesos, Event>::events,
+          lambda::bind(&MockHTTPExecutor<Mesos, Event>::events,
+                       executor,
                        this,
-                       lambda::_1)),
-      executor(_executor) {}
-
-protected:
-  void events(std::queue<Event> events)
-  {
-    while (!events.empty()) {
-      Event event = std::move(events.front());
-      events.pop();
-      executor->event(this, event);
-    }
-  }
-
-private:
-  // TODO(bmahler): This is a shared pointer because the `Mesos`
-  // library copies the pointer into callbacks that can execute
-  // after `Mesos` is destructed. We can avoid this by ensuring
-  // that `~Mesos()` blocks until deferred callbacks are cleared
-  // (merely grabbing the `process::Mutex` lock is sufficient).
-  // The `Mesos` library can also provide a `Future<Nothing> stop()`
-  // to allow callers to wait for all events to be flushed.
-  std::shared_ptr<MockHTTPExecutor<Mesos, Event>> executor;
+                       lambda::_1)) {}
 };
 
 } // namespace executor {