Google mock with `unique_ptr` and `std::move` object giving memory leak issue

Question

This question is a follow-up on Google mock with unique_ptr object giving memory leak issue. My class under test takes owenership of the unique_ptr on its contructor:

class Foo {
  public:
    virtual void doFoo() { std::cout << "Foo" << std::endl; }
};

class Bar {
  public:
    // So I can inject a mock Foo for testing
    Bar(std::unique_ptr<Foo> foo_)
        : foo_(std::move(foo_))
    {}

    Bar()
        : foo_(std::make_unique<Foo>())
    {}

    void doBar() { foo_->doFoo(); }

  private:
    std::unique_ptr<Foo> foo_;
};

class MockFoo : public Foo {
    MOCK_METHOD(void, doFoo, (), (override));
};

class FooTest : public ::testing::Test {
  public:
    void SetUp() override
    {
        mockFoo = std::make_unique<NiceMock<MockFoo>>();
    }

    std::unique_ptr<NiceMock<MockFoo>> mockFoo;
    NiceMock<MockFoo>* mockFooPtr;
};

TEST_F(FooTest, foo)
{
    using ::testing::NiceMock;

    // This will fail on purpose
    EXPECT_CALL(*mockFoo, doFoo()).Times(2);

    mockFooPtr = mockFoo.get();

    Bar bar(std::move(mockFoo));
    bar.doBar();

    EXPECT_TRUE(testing::Mock::VerifyAndClearExpectations(mockFooPtr));
}

On running that test, I get

ERROR: this mock object (used in test FooTest.foo) should be deleted but never is. Its address is @0x5ccb272fba60.
3: ERROR: 1 leaked mock object found at program exit. Expectations on a mock object are verified when the object is destructed. Leaking a mock means that its expectations aren't verified, which is usually a test bug. If you really intend to leak a mock, you can suppress this error using testing::Mock::AllowLeak(mock_object), or you may use a fake or stub instead of a mock.

So VerifyAndClearExpectations isn't working. The problem is the ownership transfer. If Bar is updated to get a raw pointer there's no mock leak error:

class Bar {
  public:
    Bar(Foo* foo_)
        : foo_(foo_)
    {}

    void doBar() { foo_->doFoo(); }

  private:
    Foo* foo_;
};

TEST_F(FooTest, foo)
{
    using ::testing::NiceMock;


    // This will fail on purpose
    EXPECT_CALL(*mockFoo, doFoo()).Times(1);

    mockFooPtr = mockFoo.get();

    // Bar bar(std::move(mockFoo));
    Bar bar(mockFoo.get());
    bar.doBar();
}

I have to use std::move because that's what production code expects.

Based on the comments below, I tried updating the test to:

TEST_F(FooTest, foo)
{
    // This will fail on purpose
    EXPECT_CALL(*mockFoo, doFoo()).Times(2);

    mockFooPtr = mockFoo.get();
    {
        Bar bar(std::move(mockFoo));
        bar.doBar();
        EXPECT_TRUE(testing::Mock::VerifyAndClearExpectations(mockFooPtr));
    }
}

I still get a mock leak error.

Thanks!

Answer 1

Polymorphic objects should have a virtual destructor. If you do not, then ownership via a base class pointer leads to undefined behavior upon destruction because only the base class destructor will run. In particular, with your code, destroying a std::unique_ptr<Foo> does not invoke ~MockFoo() even when the managed object is really a MockFoo.

To get the desired behavior, add a virtual destructor to the base class:

class Foo {
  public:
    virtual ~Foo() = default; // <-- Add this line

    virtual void doFoo() { std::cout << "Foo" << std::endl; }
};

See When to use virtual destructors?

Note: It is not necessary to edit derived classes, since this one line causes the destructors of derived classes to also be virtual. However, it can be useful (as a level of defensive programming, and to make unit tests more robust) to add derived class destructors annotated override, as in ~MockFoo() override = default;, since this would cause a compilation error if the base class lacks a virtual destructor.