You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bb...@apache.org on 2019/08/13 14:54:59 UTC

[mesos] 01/02: Dispatched invocations of resource provider mock default actions.

This is an automated email from the ASF dual-hosted git repository.

bbannier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit d5e67cf9471cc0e197a2e9f2119d63a4e3f63d99
Author: Benjamin Bannier <bb...@apache.org>
AuthorDate: Mon Aug 12 15:52:05 2019 +0200

    Dispatched invocations of resource provider mock default actions.
    
    When invoking member functions of the mock resource provider
    `TestResourceProviderProcess` from default mock actions we need to
    ensure that the mock object remains alive for the duration of the
    invocation.
    
    This patch replaces direct invocations which might run on any thread
    (e.g., also while the mock object is being destructed on another thread)
    which safe `dispatch`es of the default methods.
    
    Review: https://reviews.apache.org/r/71272/
---
 src/tests/api_tests.cpp                       | 11 ++++++--
 src/tests/mesos.hpp                           | 39 ++++++++++++++++++++-------
 src/tests/resource_provider_manager_tests.cpp | 30 +++++++++++++--------
 src/tests/slave_tests.cpp                     | 26 +++++++++++++-----
 4 files changed, 76 insertions(+), 30 deletions(-)

diff --git a/src/tests/api_tests.cpp b/src/tests/api_tests.cpp
index c209967..36fb7b0 100644
--- a/src/tests/api_tests.cpp
+++ b/src/tests/api_tests.cpp
@@ -9192,11 +9192,18 @@ TEST_P(AgentAPITest, MarkResourceProviderGone)
 
   Future<mesos::v1::resource_provider::Event::Subscribed> subscribed;
 
+  auto resourceProviderProcess = resourceProvider.process->self();
   EXPECT_CALL(*resourceProvider.process, subscribed(_))
     .WillOnce(DoAll(
         Invoke(
-            resourceProvider.process.get(),
-            &v1::TestResourceProviderProcess::subscribedDefault),
+            [resourceProviderProcess](
+                const typename mesos::v1::resource_provider::Event::Subscribed&
+                  subscribed) {
+              dispatch(
+                  resourceProviderProcess,
+                  &v1::TestResourceProviderProcess::subscribedDefault,
+                  subscribed);
+            }),
         FutureArg<0>(&subscribed)))
     .WillRepeatedly(Return());
 
diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp
index 73b6e42..1b00881 100644
--- a/src/tests/mesos.hpp
+++ b/src/tests/mesos.hpp
@@ -3039,32 +3039,50 @@ public:
   TestResourceProviderProcess(
       const ResourceProviderInfo& _info,
       const Option<Resources>& _resources = None())
-    : info(_info),
+    : process::ProcessBase(process::ID::generate("test-resource-provider")),
+      info(_info),
       resources(_resources)
   {
-    ON_CALL(*this, connected())
-      .WillByDefault(
-          Invoke(this, &TestResourceProviderProcess::connectedDefault));
+    auto self = this->self();
+
+    ON_CALL(*this, connected()).WillByDefault(Invoke([self]() {
+      dispatch(self, &TestResourceProviderProcess::connectedDefault);
+    }));
     EXPECT_CALL(*this, connected()).WillRepeatedly(DoDefault());
 
     ON_CALL(*this, subscribed(_))
       .WillByDefault(
-          Invoke(this, &TestResourceProviderProcess::subscribedDefault));
+          Invoke([self](const typename Event::Subscribed& subscribed) {
+            dispatch(
+                self,
+                &TestResourceProviderProcess::subscribedDefault,
+                subscribed);
+          }));
     EXPECT_CALL(*this, subscribed(_)).WillRepeatedly(DoDefault());
 
     ON_CALL(*this, applyOperation(_))
       .WillByDefault(
-          Invoke(this, &TestResourceProviderProcess::operationDefault));
+          Invoke([self](const typename Event::ApplyOperation& operation) {
+            dispatch(
+                self,
+                &TestResourceProviderProcess::operationDefault,
+                operation);
+          }));
     EXPECT_CALL(*this, applyOperation(_)).WillRepeatedly(DoDefault());
 
     ON_CALL(*this, publishResources(_))
       .WillByDefault(
-          Invoke(this, &TestResourceProviderProcess::publishDefault));
+          Invoke([self](const typename Event::PublishResources& publish) {
+            dispatch(
+                self,
+                &TestResourceProviderProcess::publishDefault,
+                publish);
+          }));
     EXPECT_CALL(*this, publishResources(_)).WillRepeatedly(DoDefault());
 
-    ON_CALL(*this, teardown())
-      .WillByDefault(
-          Invoke(this, &TestResourceProviderProcess::teardownDefault));
+    ON_CALL(*this, teardown()).WillByDefault(Invoke([self]() {
+      dispatch(self, &TestResourceProviderProcess::teardownDefault);
+    }));
     EXPECT_CALL(*this, teardown()).WillRepeatedly(DoDefault());
   }
 
@@ -3087,6 +3105,7 @@ public:
   {
     while (!events.empty()) {
       Event event = events.front();
+
       events.pop();
 
       switch (event.type()) {
diff --git a/src/tests/resource_provider_manager_tests.cpp b/src/tests/resource_provider_manager_tests.cpp
index 2792000..bcf6a03 100644
--- a/src/tests/resource_provider_manager_tests.cpp
+++ b/src/tests/resource_provider_manager_tests.cpp
@@ -1192,13 +1192,15 @@ TEST_P_TEMP_DISABLED_ON_WINDOWS(
   // We terminate the resource provider once we have confirmed that it
   // got disconnected. This avoids it to in turn resubscribe racing
   // with the newly created resource provider.
+  auto resourceProviderProcess = resourceProvider->process->self();
   Future<Nothing> disconnected;
   EXPECT_CALL(*resourceProvider->process, disconnected())
     .WillOnce(DoAll(
-      Invoke(
-        resourceProvider->process.get(),
-        &v1::TestResourceProviderProcess::stop),
-      FutureSatisfy(&disconnected)))
+        Invoke([resourceProviderProcess]() {
+          dispatch(
+              resourceProviderProcess, &v1::TestResourceProviderProcess::stop);
+        }),
+        FutureSatisfy(&disconnected)))
     .WillRepeatedly(Return()); // Ignore spurious calls concurrent with `stop`.
 
   // The agent failover.
@@ -1270,12 +1272,15 @@ TEST_P(ResourceProviderManagerHttpApiTest, ResubscribeUnknownID)
 
   // We explicitly terminate the resource provider after the expected
   // disconnect to prevent it from resubscribing indefinitely.
+  auto resourceProviderProcess = resourceProvider.process->self();
   Future<Nothing> disconnected;
   EXPECT_CALL(*resourceProvider.process, disconnected())
     .WillOnce(DoAll(
-      Invoke(
-        resourceProvider.process.get(), &v1::TestResourceProviderProcess::stop),
-      FutureSatisfy(&disconnected)));
+        Invoke([resourceProviderProcess]() {
+          dispatch(
+              resourceProviderProcess, &v1::TestResourceProviderProcess::stop);
+        }),
+        FutureSatisfy(&disconnected)));
 
   // Start and register a resource provider.
   Owned<EndpointDetector> endpointDetector(
@@ -1423,13 +1428,16 @@ TEST_F(ResourceProviderManagerHttpApiTest, ResourceProviderSubscribeDisconnect)
   // We terminate the first resource provider once we have confirmed
   // that it got disconnected. This avoids it to in turn resubscribe
   // racing with the other resource provider.
+  auto resourceProviderProcess1 = resourceProvider1.process->self();
   Future<Nothing> disconnected1;
   EXPECT_CALL(*resourceProvider1.process, disconnected())
     .WillOnce(DoAll(
-      Invoke(
-        resourceProvider1.process.get(),
-        &v1::TestResourceProviderProcess::stop),
-      FutureSatisfy(&disconnected1)))
+        Invoke([resourceProviderProcess1]() {
+          dispatch(
+              resourceProviderProcess1,
+              &v1::TestResourceProviderProcess::stop);
+        }),
+        FutureSatisfy(&disconnected1)))
     .WillRepeatedly(Return()); // Ignore spurious calls concurrent with `stop`.
 
   Future<Event::Subscribed> subscribed2;
diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
index 7c6e1d9..1a92602 100644
--- a/src/tests/slave_tests.cpp
+++ b/src/tests/slave_tests.cpp
@@ -109,6 +109,8 @@ using mesos::internal::protobuf::createLabel;
 using mesos::master::detector::MasterDetector;
 using mesos::master::detector::StandaloneMasterDetector;
 
+using mesos::v1::resource_provider::Event;
+
 using mesos::slave::ContainerConfig;
 using mesos::slave::ContainerTermination;
 
@@ -10682,15 +10684,25 @@ TEST_F(SlaveTest, ResourceProviderPublishAll)
 
     // Two PUBLISH_RESOURCES events will be received: one for launching the
     // executor, and the other for launching the task.
+    auto resourceProviderProcess = resourceProvider.process->self();
     EXPECT_CALL(*resourceProvider.process, publishResources(_))
-      .WillOnce(Invoke(
-          resourceProvider.process.get(),
-          &v1::TestResourceProviderProcess::publishDefault))
+      .WillOnce(
+          Invoke([resourceProviderProcess](
+                     const typename Event::PublishResources& publish) {
+            dispatch(
+                resourceProviderProcess,
+                &v1::TestResourceProviderProcess::publishDefault,
+                publish);
+          }))
       .WillOnce(DoAll(
-          FutureArg<0>(&publish),
-          Invoke(
-              resourceProvider.process.get(),
-              &v1::TestResourceProviderProcess::publishDefault)));
+          Invoke([resourceProviderProcess](
+                     const typename Event::PublishResources& publish) {
+            dispatch(
+                resourceProviderProcess,
+                &v1::TestResourceProviderProcess::publishDefault,
+                publish);
+          }),
+          FutureArg<0>(&publish)));
 
     Future<TaskStatus> taskStarting;
     Future<TaskStatus> taskRunning;