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 <[email protected]> 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;
