-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Deadlock in the setup of expectations for the mocked GetCurrentThreadId
function
#3
Comments
Calls to GetCurrentThreadId come from the Option 1: disable using GetCurrentThreadId from #pragma comment( lib, "kernel32" )
// Disable using mutex
#define GTEST_IS_THREADSAFE 0
// trick, due
// error C2065: 'AutoHandle': undeclared identifier
// @issue https://github.com/google/googletest/issues/3586
// @issue https://github.com/google/googletest/issues/4305
#include "..\..\gmock\include\gtest\internal\gtest-port.h"
#undef GTEST_HAS_DEATH_TEST
#include "..\src\gtest\gtest-all.cc"
#include "..\src\gmock\gmock-all.cc"
#include "..\..\src\gmock-win32.cpp"
using namespace testing;
MOCK_STDCALL_FUNC(DWORD, GetCurrentThreadId);
int main(int argc, char* argv[])
{
ON_MODULE_FUNC_CALL(GetCurrentThreadId).WillByDefault(Return(1));
return 0;
} Option 2: disable using GetCurrentThreadId for gmock-win32 // C++11
constexpr bool strings_equal(char const * a, char const * b) {
return *a == *b && (*a == '\0' || strings_equal(a + 1, b + 1));
}
...
#define ON_MODULE_FUNC_CALL(func, ...) \
static_assert(!strings_equal(#func, "GetCurrentThreadId" ), \
"it is prohibited to use the function GetCurrentThreadId"); \
if (!mock_module_##func::oldFn_) \
{ \
mockModule_patchModuleFunc(&func, reinterpret_cast< void* >( \
&mock_module_##func::stub), &mock_module_##func::oldFn_); \
} \
ON_CALL(mock_module_##func::instance(), func(__VA_ARGS__)) |
Yes, good idea, and there are several other (inside GTest) Win32 API functions that may interact with client tests through mocks:
This list may change in the next version of the GTest library. Therefore, it might be advisable to provide some recommendations about avoiding such function mocks in tests.
I agree; that could be also a solution. It is not a simple one, but I will try to implement it by creating a barrier-like restoring to the original function during GMock internal calls. We can achieve this by modifying our stub function (along with other macro definitions) as shown in the following pseudocode: struct mock_module_##func
{
...
static GMOCK_RESULT_(tn, __VA_ARGS__) ct stub(...)
{
const bool invokeOrig = lock > 0;
// Increment lock for the current thread
const LockBarrier barrier{ lock };
// The subsequent reentrant calls will be directed to the original function. Consequently, all
// GMock operations will call real APIs during this active barrier
if (invokeOrig)
return invokeOrig(mock_module_##func::oldFn_, ...);
else
return mock_module_##func::instance().func(...);
}
};
...
// Locks per thread
extern thread_local int lock; We must implement these locks for all GMock functions invoked within our macro definitions and for expectation clauses as well. Additionally, we should introduce a macro to temporarily suppress API mocks during the execution of other GTest macro-defines. This macro can wrap around GTest macro definitions, and we can also implement scoped suppression of API mocks, e.g.: EXPECT_MODULE_FUNC_CALL(GetCurrentThreadId).Times(2);
...
// Used to avoid GetCurrentThreadId mocks during EXPECT_EQ check (because
// we could get over-saturated results or even deadlock)
BYPASS_MOCKS(EXPECT_EQ(tid, 42U));
...
{
const BypasMocks useOrigAPI{ };
// Some other code or GTest ASSERTs that shouldn't be involved in the testing of APIs
...
} |
Yes, after wrapping the WinApi function used in the gmock-win32 core. There are few of them: GetLastError, GetCurrentProcess, WriteProcessMemory, VirtualProtect and GetModuleHandle. It will be easy.
I tried it, it works. There are "sync" call in SetOwnerAndName() and RegisterOwner() too. But... yes!
i gave up on the “expectation clauses” step. All GMock functions... how many are there? And one more task ON_CALL(mock_module_GetCurrentThreadId::instance(), GetCurrentThreadId())
.WillByDefault( testing::Return( 1 ) ); if for example1 in #define ON_MODULE_FUNC_CALL(func, ...) \
...
mock_module_GetCurrentThreadId::invokeOrig = true; \
auto tmp = ON_CALL(mock_module_GetCurrentThreadId::instance(), GetCurrentThreadId()); \
mock_module_GetCurrentThreadId::invokeOrig = false; \
tmp
...
ON_MODULE_FUNC_CALL( GetCurrentThreadId )
.WillByDefault( testing::Return( 1 ) ); get warning about WillByDefault, it can detect temporary object or something. if for example2 in #define ON_MODULE_FUNC_CALL(func, ...) \
...
struct setter { \
~setter() { \
mock_module_GetCurrentThreadId::invokeOrig = false; \
} \
}; \
auto y = [](){ \
mock_module_GetCurrentThreadId::invokeOrig = true; \
setter selfRemove; \
return ON_CALL(mock_module_GetCurrentThreadId::instance(), GetCurrentThreadId()); \
}; y( )
...
ON_MODULE_FUNC_CALL( GetCurrentThreadId )
.WillByDefault( testing::Return( 1 ) ); Even RVO doesnt help, warning about WillByDefault |
I experimented a little bit with wrappers around l-value references returned from #define MOCK_MODULE_FUNC0_(tn, constness, ct, func, ...) \
std::unique_ptr< ::testing::NiceMock< struct mock_module_##func > > func##obj; \
...
template< typename Reference >
struct ref_proxy
{
~ref_proxy() { use_module_mock = true; }
operator Reference() const { return r; }
public:
Reference r;
};
template< typename Reference >
ref_proxy< Reference > makeProxy(Reference&& r)
{
return ref_proxy< Reference >{ std::forward< decltype(r) >(r) };
}
...
extern thread_local volatile bool use_module_mock;
...
#define EXPECT_MODULE_FUNC_CALL(func, ...) \
if (!func##obj) \
{ \
func##obj.reset(new ::testing::NiceMock< struct mock_module_##func >{ }); \
} \
if (!mock_module_##func::oldFn_) \
{ \
mockModule_patchModuleFunc(&func, reinterpret_cast< void* >( \
&mock_module_##func::stub), &mock_module_##func::oldFn_); \
} \
use_module_mock = false; \
static_cast< decltype(EXPECT_CALL(mock_module_##func::instance(), \
func(__VA_ARGS__)))& >(makeProxy(EXPECT_CALL(mock_module_##func::instance(), func(__VA_ARGS__)))) Then we could hold a lock until the clause expression invocation finished (then it will be automatically unlocked): TEST(GetCurrentThreadIdTest, BaseTest)
{
ON_MODULE_FUNC_CALL(GetCurrentThreadId).WillByDefault(Return(42U));
// In this case, the lock will be unlocked immediately after the Times(...) clause was called
EXPECT_MODULE_FUNC_CALL(GetCurrentThreadId).Times(2);
const auto tid1 = GetCurrentThreadId();
const auto tid2 = GetCurrentThreadId();
... I'm not sure yet about the reliability of this proxy-based approach to achieve the desired outcome, but it works in that test code. |
Cool! |
Good point, I forgot about the APIs used in this library, yes, we should also wrap them.
The one thing that's bothering me is this macro: BYPASS_MOCKS(EXPECT_EQ(tid, 42U)); I don't know how we can avoid such wrapping by |
Replacing macro EXPECT_EQ? |
This issue occurred during the instantiation of a static
NiceMock
object, which, in turn, invokes theGetCurrentThreadId
function (referenced intesting::internal::Mutex::Lock
). Environment: GoogleTest 1.14.0, Win10.Minimal sample to reproduce:
The text was updated successfully, but these errors were encountered: