blob: af5eb482ddb0b52bc06008d480c3f530d263b73e [file] [log] [blame]
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;
}