[bug] deadlock in the setup of expectations for the mocked GetCurrentThreadId function

[smalti]

Sample where the fix was tested:

#include <gmock/gmock.h>
#include <gmock-win32.h>

MOCK_STDCALL_FUNC(DWORD, GetCurrentThreadId);

TEST(GetCurrentThreadIdTest, BaseTest)
{
    ON_MODULE_FUNC_CALL(GetCurrentThreadId).WillByDefault(testing::Return(42U));
    EXPECT_MODULE_FUNC_CALL(GetCurrentThreadId).Times(2);

    const auto tid1 = ::GetCurrentThreadId();
    const auto tid2 = ::GetCurrentThreadId();

    BYPASS_MOCKS(EXPECT_EQ(tid1, 42U));
    BYPASS_MOCKS(EXPECT_EQ(tid2, 42U));

    RESTORE_MODULE_FUNC(GetCurrentThreadId);
    VERIFY_AND_CLEAR_MODULE_FUNC_EXPECTATIONS(GetCurrentThreadId);
}

int main(int argc, char* argv[])
{
    testing::InitGoogleTest(&argc, argv);
    return RUN_ALL_TESTS();
}

Env:

• GTest: googletest-1.13.0
• Toolchains: MSVC 2015 v140; MSVC 2022 v143 /std:c++17
• Platform: x86

---

Not solved problems:

• We need to avoid optimizations for lock, see avoid_opt test code
• The RESTORE_MODULE_FUNC call is required before expectations checks
• Current implementation made only for MOCK_MODULE_FUNC0_

---

To close the issue, the merging of this PR is also required:
#7

[Alex0vSky]

Yeah, optimization
got

const auto tid1 = ::GetCurrentThreadId();
call        ebx  

even in Debug configuration, without any optimization.
Give me some time

[Alex0vSky]

Done merging to current state.
For continue moving.

[Alex0vSky]

Conflict resolved here #11

[Alex0vSky]

Found my error in patchModuleFunc_##m impl
Fix in #12

[Alex0vSky]

We need to avoid optimizations for lock, see avoid_opt test code

I didn't find any errors with proxy_base::~proxy_base.
Naked, without avoid_opt, lock is successfully decremented every time.
Tried MSVC 2019 and clang
Maybe special building arguments? I have default/gray ones for release building.

The RESTORE_MODULE_FUNC call is required before expectations checks

It might be a good idea to restore all hooks when exiting from current test.

Current implementation made only for MOCK_MODULE_FUNC0_

Temporary, convenient.

[smalti]

I didn't find any errors with proxy_base::~proxy_base.
Tried MSVC 2019 and clang.

My test setup (where I need to use the ugly avoid_opt) was:

• MSVC 2015 Update 3, v140 toolchain, Release | x86 configuration
  Optimization settings: default settings were applied, which included: /O2 /Oi /GL for compilation and /OPT:REF /OPT:ICF for the linker.

Then I retested it on a newer setup, and it works without avoid_opt:

• MSVC 2022, v143 toolchain, Release | x86 configuration, /std:c++14, /std:c++17, /std:c++20, /std:c++latest (Preview - Features from the Latest C++ Working Draft)

So, the problem exists only on my old setup: MSVC 2015 Update 3, v140, and I think that we can skip it (since it works properly on MSVC 2019, clang, MSVC 2022 /std:c++14, /std:c++17, /std:c++20).

As it turns out, the issue is related to my older configuration MSVC 2015 Update 3 with the v140 toolchain. Given that the problem is not present when using MSVC 2019, Clang, or MSVC 2022 with the specified C++ standards (/std:c++14, /std:c++17, /std:c++20), I think we can skip (what do you think ?) that problem for older setup (maybe we should recheck MSVS 2017).

---

So then, my next TODO list will be:

• Remove the avoid_opt workaround
• Apply the implementation from MOCK_MODULE_FUNC0_ to other macro-defines

[smalti]

It might be a good idea to restore all hooks when exiting from current test.

I agree, and we need to call VERIFY_AND_CLEAR_MODULE_FUNC_EXPECTATIONS after tests since the NiceMock object is a static object.

[Alex0vSky]

/O2 /Oi /GL for compilation and /OPT:REF /OPT:ICF for the linker.
.../std:c++14, /std:c++17, /std:c++20)
I think that we can skip it (since it works properly on MSVC 2019, clang, MSVC 2022 /std:c++14, /std:c++17, /std:c++20)

Msvc2019(v142) and its clang.
/O2 /Oi /GL for compilation, /OPT:REF /OPT:ICF for the linker.

• /std:c++14 passed
• /std:c++17 passed
• /std:c++20 error
  Need declare mockModule_patchModuleFunc
  before using it in patchModuleFunc_ function.
  After rewriting
• /std:c++20 passed
• /std:c++latest passed

We can skip it, I agree.

[Alex0vSky]

C++20 resolverd in #14

[Alex0vSky]

Moved to here

[Alex0vSky]

Moved from here