Avoid memory leaks by design

There are many tools and techniques out there to help detect memory leaks. For instance, you can monitor performance counters for unexpected memory growth. Once a leak is suspected, UMDH might be able to help you pinpoint the culprit. However, when it comes to memory management, “an ounce of prevention is worth a pound of cure.” Treat memory leaks not as code defects which require superficial fixes but as the design issues that they are.

In modern C++ (at least since, say, C++11), the situation can be summed up thusly:

• Memory leaks are a design issue.
• In the vast majority of cases, the specific design issue is unclear object lifetime arising from the use of raw pointers.
• The solution is either eliminating the pointers or switching to smart pointers.

I admit that this terse synopsis is rather unsatisfying. So rather than leaving it that, let’s look at some specific examples. Note that the sample code all assumes this “Common.h” header file:

#pragma once

#include <memory>
#include <iostream>
#include <string>
#include <atomic>
#include <sstream>

// Helps track leaked allocations
struct Counter { static std::atomic<int> N; };
std::atomic<int> Counter::N(0);

// Base class to disallow copy by assignment or construction.
struct DenyCopy
{
    DenyCopy() { }
    ~DenyCopy() { }
    DenyCopy(DenyCopy const &) = delete;
    DenyCopy & operator = (DenyCopy const &) = delete;
};

// The sample class which we'll be allocating/freeing. We trace on
// construction/destruction, as well as dynamic [de]allocation
// (via overloaded new/delete operators).
template<typename TCounter>
class MyClass_T : private DenyCopy
{
public:
    MyClass_T(std::wstring const & name)
        : name_(name)
    {
        ++TCounter::N;
        std::wcout << L"MyClass:" << name_ << std::endl;
    }

    ~MyClass_T()
    {
        std::wcout << L"~MyClass:" << name_ << std::endl;
        --TCounter::N;
    }

    void * operator new(size_t size)
    {
        std::wcout << L"(Dynamic allocation)" << endl;
        return ::operator new(size);
    }

    void operator delete(void * ptr)
    {
        ::operator delete(ptr);
        std::wcout << L"(Dynamic deallocation)" << endl;
    }

    std::wstring const & get_Name() const { return name_; }

private:
    std::wstring name_;
};

// RAII wrapper to trace on function enter/leave.
class FuncTrace : private DenyCopy
{
public:
    FuncTrace(wchar_t const * name)
        : name_(name)
    {
        std::wcout << L"{ Enter " << name_ << std::endl;
    }

    ~FuncTrace()
    {
        std::wcout << L"} Leave " << name_ << std::endl;
    }

private:
    wchar_t const * name_;
};

// Some helpful typedefs
typedef MyClass_T<Counter> MyClass;
typedef std::unique_ptr<MyClass> MyClassUPtr;
typedef std::shared_ptr<MyClass> MyClassSPtr;

Elimination of pointers

Pointers typically come into play when using dynamic memory, such as when objects are allocated in the “free store” by calling new. Often you simply don’t need dynamic memory which means you can eliminate the associated pointers. Stack allocation is sufficient when you are dealing with a small object whose lifetime is no greater than the local scope. Consider this example:

void QuestionableDynamicAllocation()
{
    FuncTrace t(__FUNCTIONW__);
    MyClass * p = new MyClass(L"probably-should-stack-allocate");
    wcout << L"Using " << p->get_Name() << L"..." << endl;
    // ... do more things ...
    delete p;
}

void BetterStackAllocation()
{
    FuncTrace t(__FUNCTIONW__);
    MyClass c(L"on-the-stack");
    wcout << L"Using " << c.get_Name() << L"..." << endl;
    // ... do more things ...
    // yay, no need for delete!
}

int wmain(int argc, wchar_t ** argv)
{
    QuestionableDynamicAllocation();
    BetterStackAllocation();
    wcout << L"Final count: " << Counter::N << endl;
    return 0;
}

The output:

{ Enter QuestionableDynamicAllocation
(Dynamic allocation)
MyClass:probably-should-stack-allocate
Using probably-should-stack-allocate...
~MyClass:probably-should-stack-allocate
(Dynamic deallocation)
} Leave QuestionableDynamicAllocation
{ Enter BetterStackAllocation
MyClass:on-the-stack
Using on-the-stack...
~MyClass:on-the-stack
} Leave BetterStackAllocation
Final count: 0

Stack allocation is clearly sufficient in this case given that the object is expected to be destructed before the function exits. Using the stack here also gives us the bonus of exception safety — delete may not be called if any of the intervening code throws an exception.

Scoped smart pointer

Okay, so let’s say we really do need dynamic allocation — perhaps we have limited stack space. In that case, unique_ptr can be used to safely wrap new and delete:

void ScopedUse(bool leaveEarly)
{
    FuncTrace t(__FUNCTIONW__);
    MyClassUPtr p = make_unique<MyClass>(L"this-scope-only");
    wcout << L"I'm doing something with " << p->get_Name() << endl;
    if (leaveEarly)
    {
        wcout << L"Leaving now!" << endl;
        return;
    }

    wcout << L"Doing more with " << p->get_Name() << endl;
}

int wmain(int argc, wchar_t ** argv)
{
    ScopedUse(false);
    ScopedUse(true);
    wcout << L"Final count: " << Counter::N << endl;
    return 0;
}

The output:

{ Enter ScopedUse
(Dynamic allocation)
MyClass:this-scope-only
I'm doing something with this-scope-only
Doing more with this-scope-only
~MyClass:this-scope-only
(Dynamic deallocation)
} Leave ScopedUse
{ Enter ScopedUse
(Dynamic allocation)
MyClass:this-scope-only
I'm doing something with this-scope-only
Leaving now!
~MyClass:this-scope-only
(Dynamic deallocation)
} Leave ScopedUse
Final count: 0

The object is guaranteed to be destroyed in any normal function exit condition, exceptional or successful, even in the early return case demonstrated above — a typical source of leaks in code using raw pointers.

Safe transfer of ownership

Sometimes we need to create and prepare a dynamically allocated object but ultimately pass it off to some other place, after which the callee is responsible for its lifetime. Again, unique_ptr comes to our rescue here with its implementation of move semantics. This allows us to safely transfer ownership so that we avoid double deletion or no deletion:

void NewOwner(MyClassUPtr p)
{
    FuncTrace t(__FUNCTIONW__);
    wcout << L"I now own " << p->get_Name() << endl;
}

void CreateAndTransfer()
{
    FuncTrace t(__FUNCTIONW__);
    MyClassUPtr p = make_unique<MyClass>(L"to-be-moved");
    // ... do some other things here ...
    // Now give the object away
    NewOwner(move(p));
    if (!p)
    {
        wcout << L"I can't use this instance anymore." << endl;
    }
}

int wmain(int argc, wchar_t ** argv)
{
    CreateAndTransfer();
    wcout << L"Final count: " << Counter::N << endl;
    return 0;
}

Note that we have to use move for the code to compile — you cannot otherwise pass a unique_ptr by value as this would lead to unclear ownership. The output:

{ Enter CreateAndTransfer
(Dynamic allocation)
MyClass:to-be-moved
{ Enter NewOwner
I now own to-be-moved
} Leave NewOwner
~MyClass:to-be-moved
(Dynamic deallocation)
I can't use this instance anymore.
} Leave CreateAndTransfer
Final count: 0

Safe reassignment

Consider this very bad code which implements a retry loop and some macros for “safe” cleanup:

#define BEGIN_RETRY_LOOP(maxAttempt) \
{ int retryMaxAttempt = (maxAttempt); int retryAttempt = 0; for (; retryAttempt < retryMaxAttempt; ++retryAttempt)

#define END_RETRY_LOOP \
if (retryAttempt == retryMaxAttempt) goto Cleanup; }

#define FREE(ptr) { if (ptr) { delete ptr; ptr = nullptr; } }

bool Validate(MyClass * p)
{
    return p->get_Name() == wstring(L"Attempt-1");
}

void LeakyReassignment()
{
    FuncTrace t(__FUNCTIONW__);
    MyClass * p;
    BEGIN_RETRY_LOOP(5)
    {
        wstringstream wss;
        wss << L"Attempt-" << retryAttempt;
        p = new MyClass(wss.str());
        bool isValid = Validate(p);
        if (isValid)
        {
            break;
        }
    }
    END_RETRY_LOOP

    // ...
    wcout << L"I have a valid pointer " << p->get_Name() << endl;
    // ... more stuff here ...
Cleanup:
    FREE(p);
}

int wmain(int argc, wchar_t ** argv)
{
    LeakyReassignment();
    wcout << L"Final count: " << Counter::N << endl;
    return 0;
}

The output shows the obvious leak here:

{ Enter LeakyReassignment
(Dynamic allocation)
MyClass:Attempt-0
(Dynamic allocation)
MyClass:Attempt-1
I have a valid pointer Attempt-1
~MyClass:Attempt-1
(Dynamic deallocation)
} Leave LeakyReassignment
Final count: 1

Since the object was declared “invalid” on the first attempt, a new object was allocated on the second attempt, overwriting the original pointer. That first object is never released. While this could be “fixed” by adding a FREE(p) inside an else clause, we should instead stay very far away from the defective “if (failed) goto cleanup” pattern and use real C++. Let’s look at a better implementation that uses unique_ptr and ditches the ugly macros:

void CorrectReassignment()
{
    FuncTrace t(__FUNCTIONW__);
    MyClassUPtr p;
    for (int retryAttempt = 0; retryAttempt < 5; ++retryAttempt)
    {
        wstringstream wss;
        wss << L"Attempt-" << retryAttempt;
        p = make_unique<MyClass>(wss.str());
        bool isValid = Validate(p.get());
        if (isValid)
        {
            break;
        }
    }

    // ...
    wcout << L"I have a valid pointer " << p->get_Name() << endl;
    // ... more stuff here ...
}

int wmain(int argc, wchar_t ** argv)
{
    CorrectReassignment();
    wcout << L"Final count: " << Counter::N << endl;
    return 0;
}

The output shows that unique_ptr just works and properly destroys the object when being assigned a new value:

{ Enter CorrectReassignment
(Dynamic allocation)
MyClass:Attempt-0
(Dynamic allocation)
MyClass:Attempt-1
~MyClass:Attempt-0
(Dynamic deallocation)
I have a valid pointer Attempt-1
~MyClass:Attempt-1
(Dynamic deallocation)
} Leave CorrectReassignment
Final count: 0

That just about covers the basics. In the next post, I’ll show some more complex memory management situations for which the above solutions won’t suffice.