eranpeer / FakeIt

C++ mocking made easy. A simple yet very expressive, headers only library for c++ mocking.
MIT License
1.24k stars 170 forks source link

Segfault when using mock in std::unique_ptr #140

Closed r2p2 closed 6 years ago

r2p2 commented 6 years ago

I know that this is a common issue and that it seems to work for some people. But it crashes on my side and I don't know what I do wrong.

class SomeInterface
{
public:
    virtual void foo() = 0;
    virtual ~SomeInterface() = default;
};
int a = 0;
Mock<SomeInterface> mock;
When(Dtor(mock)).Do([&](){a++; });
SomeInterface * i = &(mock.get());
delete i; // a++
delete i; // a++
REQUIRE(a == 2);

I copied this snippet from one of the other issues to create a minimal failing example. While the comment in the issue suggested that the snippet should work, I get a segmentation fault at the first delete call.

gdb backtrace:

#0  0x00000000008a0d6b in fakeit::MethodProxyCreator<void>::methodProxy (this=0xdd0e70, id=0x0) at ../fakeit/fakeit.hpp:5906
#1  0x00000000008e22b7 in fakeit::MethodProxyCreator<void>::methodProxyX<0> (this=0xdd0e70) at ../fakeit/fakeit.hpp:5911
#2  0x00000000008e16ce in fakeit::VirtualTable<____C_A_T_C_H____T_E_S_T____0()::SomeInterface>::dtor(int) (this=0xdd0e70) at ../fakeit/fakeit.hpp:5703
#3  0x00000000008def96 in ::____C_A_T_C_H____T_E_S_T__ () at ../tests/unit/...Test.cc:37

The single header version is in use

 *  Generated: 2017-11-05 20:30:40.182814
phxnsharp commented 6 years ago

This is just a guess, I haven't tried your example, but if you are going to call a method more than once, you probably want .AlwaysDo(), not .Do().

However, you should get a proper exception or test failure (depending on which test framework single-header you are using). So there may be something else going on.

ericlemes commented 6 years ago

I recently fixed a bug related to destructor mocking, therefore unique_ptr. Can you check if this fixes your bug? https://github.com/eranpeer/FakeIt/pull/144

r2p2 commented 6 years ago

I will check it again this weekend.

r2p2 commented 6 years ago

For the record, I might have missed @phxnsharp's hint back in may, so I don't know if it would have solved the issue on my side.

Today I've got the latest fakeit (not master) and catch (2.3.0) single header file and tested the exact same old code as printed above. Now I get the helpful error message Unexpected method invocation: mock.destructor(), which I did not get back in the day as far as I can remember.

After replacing Do with AlwaysDo, the test passes. Looks good to me.

ericlemes commented 6 years ago

Okay. I'll close the issue then.