| Index: test/gmock-spec-builders_test.cc |
| =================================================================== |
| --- test/gmock-spec-builders_test.cc (revision 402) |
| +++ test/gmock-spec-builders_test.cc (revision 403) |
| @@ -88,13 +88,14 @@ |
| using testing::Ne; |
| using testing::Return; |
| using testing::Sequence; |
| +using testing::SetArgPointee; |
| using testing::internal::ExpectationTester; |
| using testing::internal::FormatFileLocation; |
| -using testing::internal::g_gmock_mutex; |
| using testing::internal::kErrorVerbosity; |
| using testing::internal::kInfoVerbosity; |
| using testing::internal::kWarningVerbosity; |
| using testing::internal::String; |
| +using testing::internal::linked_ptr; |
| using testing::internal::string; |
| |
| #if GTEST_HAS_STREAM_REDIRECTION |
| @@ -157,6 +158,16 @@ |
| GTEST_DISALLOW_COPY_AND_ASSIGN_(MockB); |
| }; |
| |
| +class ReferenceHoldingMock { |
| + public: |
| + ReferenceHoldingMock() {} |
| + |
| + MOCK_METHOD1(AcceptReference, void(linked_ptr<MockA>*)); |
| + |
| + private: |
| + GTEST_DISALLOW_COPY_AND_ASSIGN_(ReferenceHoldingMock); |
| +}; |
| + |
| // Tests that EXPECT_CALL and ON_CALL compile in a presence of macro |
| // redefining a mock method name. This could happen, for example, when |
| // the tested code #includes Win32 API headers which define many APIs |
| @@ -2439,6 +2450,46 @@ |
| EXPECT_EQ(2, b1.DoB(0)); |
| } |
| |
| +TEST(VerifyAndClearTest, |
| + DestroyingChainedMocksDoesNotDeadlockThroughExpectations) { |
| + linked_ptr<MockA> a(new MockA); |
| + ReferenceHoldingMock test_mock; |
| + |
| + // EXPECT_CALL stores a reference to a inside test_mock. |
| + EXPECT_CALL(test_mock, AcceptReference(_)) |
| + .WillRepeatedly(SetArgPointee<0>(a)); |
| + |
| + // Throw away the reference to the mock that we have in a. After this, the |
| + // only reference to it is stored by test_mock. |
| + a.reset(); |
| + |
| + // When test_mock goes out of scope, it destroys the last remaining reference |
| + // to the mock object originally pointed to by a. This will cause the MockA |
| + // destructor to be called from inside the ReferenceHoldingMock destructor. |
| + // The state of all mocks is protected by a single global lock, but there |
| + // should be no deadlock. |
| +} |
| + |
| +TEST(VerifyAndClearTest, |
| + DestroyingChainedMocksDoesNotDeadlockThroughDefaultAction) { |
| + linked_ptr<MockA> a(new MockA); |
| + ReferenceHoldingMock test_mock; |
| + |
| + // ON_CALL stores a reference to a inside test_mock. |
| + ON_CALL(test_mock, AcceptReference(_)) |
| + .WillByDefault(SetArgPointee<0>(a)); |
| + |
| + // Throw away the reference to the mock that we have in a. After this, the |
| + // only reference to it is stored by test_mock. |
| + a.reset(); |
| + |
| + // When test_mock goes out of scope, it destroys the last remaining reference |
| + // to the mock object originally pointed to by a. This will cause the MockA |
| + // destructor to be called from inside the ReferenceHoldingMock destructor. |
| + // The state of all mocks is protected by a single global lock, but there |
| + // should be no deadlock. |
| +} |
| + |
| // Tests that a mock function's action can call a mock function |
| // (either the same function or a different one) either as an explicit |
| // action or as a default action without causing a dead lock. It |
| Index: include/gmock/gmock-spec-builders.h |
| =================================================================== |
| --- include/gmock/gmock-spec-builders.h (revision 402) |
| +++ include/gmock/gmock-spec-builders.h (revision 403) |
| @@ -1475,12 +1475,27 @@ |
| virtual void ClearDefaultActionsLocked() |
| GTEST_EXCLUSIVE_LOCK_REQUIRED_(g_gmock_mutex) { |
| g_gmock_mutex.AssertHeld(); |
| + |
| + // Deleting our default actions may trigger other mock objects to be |
| + // deleted, for example if an action contains a reference counted smart |
| + // pointer to that mock object, and that is the last reference. So if we |
| + // delete our actions within the context of the global mutex we may deadlock |
| + // when this method is called again. Instead, make a copy of the set of |
| + // actions to delete, clear our set within the mutex, and then delete the |
| + // actions outside of the mutex. |
| + UntypedOnCallSpecs specs_to_delete; |
| + untyped_on_call_specs_.swap(specs_to_delete); |
| + |
| + g_gmock_mutex.Unlock(); |
| for (UntypedOnCallSpecs::const_iterator it = |
| - untyped_on_call_specs_.begin(); |
| - it != untyped_on_call_specs_.end(); ++it) { |
| + specs_to_delete.begin(); |
| + it != specs_to_delete.end(); ++it) { |
| delete static_cast<const OnCallSpec<F>*>(*it); |
| } |
| - untyped_on_call_specs_.clear(); |
| + |
| + // Lock the mutex again, since the caller expects it to be locked when we |
| + // return. |
| + g_gmock_mutex.Lock(); |
| } |
| |
| protected: |
| Index: src/gmock-spec-builders.cc |
| =================================================================== |
| --- src/gmock-spec-builders.cc (revision 402) |
| +++ src/gmock-spec-builders.cc (revision 403) |
| @@ -480,7 +480,21 @@ |
| untyped_expectation->line(), ss.str()); |
| } |
| } |
| - untyped_expectations_.clear(); |
| + |
| + // Deleting our expectations may trigger other mock objects to be deleted, for |
| + // example if an action contains a reference counted smart pointer to that |
| + // mock object, and that is the last reference. So if we delete our |
| + // expectations within the context of the global mutex we may deadlock when |
| + // this method is called again. Instead, make a copy of the set of |
| + // expectations to delete, clear our set within the mutex, and then clear the |
| + // copied set outside of it. |
| + UntypedExpectations expectations_to_delete; |
| + untyped_expectations_.swap(expectations_to_delete); |
| + |
| + g_gmock_mutex.Unlock(); |
| + expectations_to_delete.clear(); |
| + g_gmock_mutex.Lock(); |
| + |
| return expectations_met; |
| } |
| |
| |