eranpeer / FakeIt

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

Crash with QSharedPointer #27

Closed misery closed 9 years ago

misery commented 9 years ago

As mentioned in #10 I get a crash in my simple test with Qt's QSharedPointer. AuthModel holds the context as a QSharedPointer, too!

#0 0x685d62 in fakeit::MethodProxyCreator<void>::methodProxy(unsigned int) [..]/external/fakeit.hpp:5815:20
#1 0x685459 in void fakeit::MethodProxyCreator<void>::methodProxyX<0>() [..]/external/fakeit.hpp:5820:20
#2 0x689a89 in fakeit::VirtualTable<ActivationContext>::dtor(int) [..]/external/fakeit.hpp:5615:13
#3 0x7fc23f73049f in QScopedPointerDeleter<ActivationContext>::cleanup(ActivationContext*) [..]/QtCore/qscopedpointer.h:54:9
#4 0x7fc23f730028 in QScopedPointer<ActivationContext, QScopedPointerDeleter<ActivationContext> >::~QScopedPointer() [..]/QtCore/qscopedpointer.h:101:9

The code:

Mock<ActivationContext> context;
When(Dtor(context));

QSharedPointer<AuthModel> model(new AuthModel(&context.get()));
david7482 commented 9 years ago

I also got a similar issue when using mock and smart pointer together.

The code:

class TestInterface
{
public:
    virtual void TestCall() = 0;
    virtual ~TestInterface() {}
};

class Singleton
{
public:
    static std::shared_ptr<Singleton> GetInstance(std::shared_ptr<TestInterface> test)
    {
        static std::shared_ptr<Singleton> singleton(new Singleton(test));
        return singleton;
    }

    void Call() { test->TestCall(); }

private:
    Singleton(std::shared_ptr<TestInterface> test) : test(test) {};

    std::shared_ptr<TestInterface> test;
};

TEST_CASE("Test Init", "[init]")
{
    Mock<TestInterface> mock;
    When(Method(mock, TestCall)).AlwaysReturn();
    Fake(Dtor(mock));
    std::shared_ptr<TestInterface> test(&mock.get());

    std::shared_ptr<Singleton> singleton = Singleton::GetInstance(test);
    singleton->Call();

    Verify(Method(mock, TestCall)).Once();
}

Because the singleton is static, when it tries to delete the mock, it already leaves mock's scope. So, exception is thrown. Do you have any suggestion or existing mechanism to avoid this ?

Thanks

eranpeer commented 9 years ago

Well, it is a bad practice to mix singletons or static data with unit tests. As you mentioned, the static data continues living behind the scope of the test merhod. I would consider refactoring the unit under test so that it would not use a singleton. If you must use static data or singletons please try using the teardown method of your unit testing framework to clean the static state between the test methods.

c0yote commented 9 years ago

I didn't read you two's code too hard, but bear in mind that if you stack allocate your mock and then pass it to a shared pointer without replacing the deleter it will try to destruct twice.

Mock<SomeInterface> mock;
std::shared_ptr<SomeInterface> mockSmrtPtr(&mock.get());
// This will crash when it exits scope.

Probably not the best way to handle it, but when I have to test a function taking a shared_ptr I dodge the issue with a template function like this:

template<typename T>
void blank_deleter(T* ptr) {}

Then I can create my shared_ptr like this so it doesn't try to double destruct.

Mock<SomeInterface> mock;
std::shared_ptr<SomeInterface> mockSmrtPtr(&mock.get(), blank_deleter<SomeInterface>);

You can also run into the pseudo-opposite problem if the shared_ptr persists somewhere else, but the stack allocated mock destructs. Calls against the shared_ptr will be calls against a destructed object.

{
  std::shared_ptr<SomeInterface> temp;
  {
    fakeit::Mock<SomeInterface> mock;
    std::shared_ptr<SomeInterface> mockSmrtPtr(&mock.get(), blank_deleter<SomeInterface>);
    temp = mockSmrtPtr;
  } // Mock destructs here, but shared_ptr is saved to temp.
  temp->foo(1); // Crashes here.
}
eranpeer commented 9 years ago

It looks like this is not a bug but more a misuse of a local mock with global scope. I will close this issue in a week from now if no more comments are made about this. Thanks for submitting this issue.

eranpeer commented 9 years ago

Closing!