shared_ptr overuse in C++

One of the common misunderstandings in modern C++ (which I admittedly fell for too) is that you should only use smart pointers, and never raw pointers. I've seen code bases where no raw pointers existed, and shared_ptrs were passed to every function that needed access to them.

‍

As said, that's a misunderstanding. You should almost never use new and delete in modern C++, but it's still very valid to pass raw pointers or references around and store them.

‍

The pitfall can be divided in two:

‍

• Passing smart pointers to functions that don't have to deal with ownership
• Improper use of shared_ptr in non-owning objects

‍

Let's look at each of these two individually.

‍

Passing smart pointers to functions that don't have to deal with ownership

‍

Consider the following example:

‍

void print_person_name(std::shared_ptr<Person> person)
{
    std::cout << person->name() << '\n';
}

int main()
{
    auto person = std::make_shared<Person>("Pete");
    print_person_name(person);
}

‍

‍

Here print_person_ name is a function that does not deal with ownership in any way. Yet, because the caller has a person dynamically managed by a shared_ptr, it was decided to make print_person_name also accept a shared_ptr.

‍

This is bad for several reasons:

‍

• Smart pointers are about ownership. If a function accepts a smart pointer, it expresses to the caller that it will take ownership of the object managed by that smart pointer. In this case, there's nothing related to ownership going on in print_person_name.
• A function should not care about how an object is managed at the call-site. Now, print_person_name cannot be reused from places where a Person was created by other means than a shared_ ptr.

‍

The solution is simple. Make print_person_name accept a const reference to a Person, instead.

‍

There are a few basic guidelines:

‍

• Prefer passing by reference to const
• If the function should be able to handle non-existence of the object, pass it by pointer to const instead
• If the function needs write access, pass by reference or pointer to non-const

‍

Here's an example:

‍

// Prefer passing by reference to const
void print_person_name(const Person& person)
{
    std::cout << person.name() << '\n';
}

// Can the function handle possible absence of the value, then pass by raw pointer to const
void print_person_name(const Person* person)
{
    if (person)
        std::cout << person->name() << '\n';
}

// Is the function modifying the object, then pass by reference to non-const
void increment_person_age(Person& person)
{
    person.setAge(person.age() + 1);
}

// Or by pointer to non-const
void increment_person_age(Person* person)
{
    if (person)
        person->setAge(person.age() + 1);
}

int main()
{
    auto personPtr = std::make_shared<Person>("Pete");
    print_person_name(*personPtr); // Dereference smart pointer so function accepting reference to const is called
    
    auto stackPerson = Person("Pete");
    print_person_name(stackPerson); // Function accepting const reference is called
    
    personPtr.reset();
    print_person_name(personPtr.get()); // As ptr could be nullptr, call function that handles nullptr, by passing raw pointer
    
    increase_person_age(stackPerson); // Calls function accepting reference to non-const
    increase_person_age(personPtr.get()); // Calls function accepting raw pointer to non-const
}

‍

‍

So basically, just like you have been doing with C++98/03.

‍

See this chapter in the C++ Core Guidelines for more information.

‍

Improper use of shared_ptr in non-owning objects

‍

The second part of this pitfall is a bit more complex and a bit less obvious. A common pitfall with raw pointers is to have dangling pointers. So when storing pointers to objects for later use it's very tempting to always use a shared_ptr. Although that might appear as a wise choise, it's a pitfall, for the following reasons:

‍

• It makes the lifetime of objects dependent on the lifetime of objects that are not the owner, and thus their lifetime will possibly be extended. Instead, they should be dependent on the lifetime of the objects that are the owner. Doing this wrong, can lead to hard to track lifetime-related bugs and shared_ptr cycles.
• Over-usage of shared_ptr can lead to performance degradation, as copying a shared_ptr is not free. Mainly because of atomic reference counting.

‍

Storing a pointer to an object is not the same as owning the object it refers to. Similar to with functions, any object that's not the (shared) owner of another object, should not have a smart pointer to it.

‍

Let's say you have a class Application and UserProfile, which both have a pointer to an object of type DatabaseSession. If your intention is to let the lifetime of DatabaseSession be dependent of the lifetime of Application ánd UserProfile, then using a shared_ptr is a valid approach. Like below.

‍

class DatabaseSession {};

class Application
{
public:
    explicit Application(std::shared_ptr<DatabaseSession> dbs) : dbs_(std::move(dbs)) {}
    
private:
    std::shared_ptr<DatabaseSession> dbs_;
};


class UserProfile
{
public:
    explicit UserProfile(std::shared_ptr<DatabaseSession> dbs) : dbs_(std::move(dbs)) {}
    
private:
    std::shared_ptr<DatabaseSession> dbs_;
};

‍

‍

But it is by design that you choose to let the lifetime of DatabaseSession be dependent on the lifetime of both Application and UserProfile.

‍

If you expect Application to own DatabaseSession, and Application to outlive UserProfile, then UserProfile should be non-owning. And that intent can and should be made clear with having a raw pointer to DatabaseSession inside UserProfile, and a unique_ptr to DatabaseSession inside Application (or a value type instead of pointer type entirely). Like below.

‍

class DatabaseSession {};

class Application
{
public:
    explicit Application(std::unique_ptr<DatabaseSession> dbs) : dbs_(std::move(dbs)) {}
    
private:
    std::unique_ptr<DatabaseSession> dbs_;
};


class UserProfile
{
public:
    explicit UserProfile(DatabaseSession* dbs) : dbs_(dbs) {}
    
private:
    DatabaseSession* dbs_;
};

‍

‍

This seems counter-intuitive as you could still end up with having a dangling pointer. But that's a bug. And that bug should not be solved by introducing a shared_ptr, and extending the lifetime of DatabaseSession to the lifetime of UserProfile. The bug should be solved by making sure UserProfile doesn't outlive Application, or by making sure UserProfile is notified that Application is gone and the pointer to DatabaseSession should be set to nullptr. It's very tempting to use shared_ptr in situations like these and just let the lifetime be managed by whatever object lives longest, but as written, that's a sign of bad design. shared_ptr should only be used in situations where you indeed have shared ownership by design, not by accident.

‍

It could be beneficial to having a weak_ptr in UserProfile to DatabaseSession, but that forces Application to suddenly have a shared_ptr to DatabaseSession, while the intention was to let Application be the sole owner of DatabaseSession. And a shared_ptr implies that ownership is shared.

‍

Long story short, using shared_ptr to avoid dangling pointers and undeterministicly extend the lifetime of objects is a pitfall of smart pointers, not an advantage.

‍

Conclusion

‍

While shared_ptr simplifies memory management, over usage has several disadvantages, mainly being unclear ownership, unintended lifetime extension of objects, memory leaks caused by shared_ptr cycles, and performance degradation.

‍

Prefer passing (const) references to functions that don't deal with ownership, and prefer storing (const) pointers in objects that should reference other objects, without being shared owner.