Should I Pass a Shared_Ptr by Reference

Should I pass a shared_ptr by reference?

In controlled circumstances you can pass the shared pointer by constant reference. Be sure that nobody is concurrently deleting the object, though this shouldn't be too hard if you're careful about to whom you give references.

In general, you should pass the shared pointer as a straight copy. This gives it its intended semantics: Every scope that contains a copy of the shared pointer keeps the object alive by virtue of its "share" in the ownership.

The only reason not to always pass by value is that copying a shared pointer comes at a certain price on account of the atomic reference count update; however, this might not be a major concern.

---

Optional digression:

Since the main question has been answered, perhaps it is instructive to consider a few ways in which you should never use a shared pointer. Here is a little thought experiment. Let us define a shared pointer type SF = std::shared_ptr<Foo>. In order to consider references, rather than passing function arguments let us look at the type RSF = std::reference_wrapper<T>. That is, if we have a shared pointer SF p(std::make_shared<Foo>());, then we can make a reference wrapper with value semantics via RSF w = std::ref(p);. So much for the setup.

Now, everybody knows that containers of pointers are minefield. So std::vector<Foo*> will be a nightmare to maintain, and any number of bugs arise from improper lifetime management. What's worse conceptually is that it is never clear who owns the objects whose pointers the container stores. The pointers could even be a mix of pointers to dynamic objects, automatic objects, and garbage. Nobody can tell. So the standard solution is to use std::vector<SF> instead. This is The Right Way to use the shared pointer. On the other hand, what you must never use is std::vector<RSF> -- this is an unmanageable monster that is actually very similar to the original vector of naked pointers! For example, it's not clear whether the object to which you hold a reference is still alive. Taking a reference of the shared pointer has defeated its entire purpose.

For a second example, suppose we have a shared pointer SF p as before. Now we have a function int foo(SF) that we want to run concurrently. The usual std::thread(foo, p) works just fine, since the thread constructor makes a copy of its arguments. However, had we said std::thread(foo, std::ref(p)), we'd be in all sorts of trouble: The shared pointer in the calling scope could expire and destroy the object, and you would be left with a dangling reference and an invalid pointer!

I hope these two admittedly fairly contrived examples shed a bit of light on when you really want your shared pointers to be passed around by copy. In a well-designed program, it should always be clear who is responsible for which resources, and when used right, the shared pointer is a great tool for the job.

Should we pass a shared_ptr by reference or by value?

This question has been discussed and answered by Scott, Andrei and Herb during Ask Us Anything session at C++ and Beyond 2011. Watch from 4:34 on shared_ptr performance and correctness.

Shortly, there is no reason to pass by value, unless the goal is to share ownership of an object (eg. between different data structures, or between different threads).

Unless you can move-optimise it as explained by Scott Meyers in the talk video linked above, but that is related to actual version of C++ you can use.

A major update to this discussion has happened during GoingNative 2012 conference's Interactive Panel: Ask Us Anything! which is worth watching, especially from 22:50.

In C++ threads, should I pass shared_ptr by value or reference?

Typically, threads may outlive the scope where they are created. In such case, any local variable captured by reference may be destroyed while the thread is still running. If this is the case, then you should not capture by reference.

Furthermore, modifying a shared pointer object in one thread and accessing in another without synchronisation results in undefined behaviour. If that is what you're doing, then you should access the pointer using std::atomic_load/atomic_store functions, or simply copy the pointer into each thread. Note that you can capture by copy:

auto f = [status]() {

---

Furthermore, the shared pointer provides no extra thread safety to accessing the pointed object beyond keeping the ownership alive and ensuring it gets deleted exactly once. If the pointed type is not atomic, then modifying it in one thread and accessing in another without synchronisation results in undefined behaviour. If that is what you're doing, you need to use mutexes or something similar. Or copy the pointed object itself into each thread.

Regarding the edited question: Your examples apply to this last case. Both of them have undefined behaviour. You need synchronisation.

Passing a shared pointer by reference or by value as parameter to a class

Pass it by value then move it into the member:

class Foo {
public:
    Foo(std::shared_ptr<Boo> boo) 
        : m_Boo(std::move(boo)) {}
private:
    std::shared_ptr<Boo> m_Boo;
};

This will be the most efficient in all cases - if the caller has a rvalue-reference then there wont be a single add-ref, if the caller has a value there'll be a single add-ref.

If you pass by const& you force an add-ref even in cases where its unnecessary. If you pass by value and then set without a std::move you may get 2 add-refs.

Edit: This is a good pattern to use if you've got a class where a move is significantly cheaper than a copy, and you have a function call which will always copy the instance passed in - as in this case. You force the copy to happen at the function call boundary by taking it by value, and then if the caller has a rvalue reference the copy need never happen at all - it will be moved instead.

Passing const shared_ptrT& versus just shared_ptrT as parameter

The advantage of passing the shared_ptr by const& is that the reference count doesn't have to be increased and then decreased. Because these operations have to be thread-safe, they can be expensive.

You are quite right that there is a risk that you can have a chain of passes by reference that later invalidates the head of the chain. This happened to me once in a real-world project with real-world consequences. One function found a shared_ptr in a container and passed a reference to it down a call stack. A function deep in the call stack removed the object from the container, causing all the references to suddenly refer to an object that no longer existed.

So when you pass something by reference, the caller must ensure it survives for the life of the function call. Don't use a pass by reference if this is an issue.

(I'm assuming you have a use case where there's some specific reason to pass by shared_ptr rather than by reference. The most common such reason would be that the function called may need to extend the life of the object.)

Update: Some more details on the bug for those interested: This program had objects that were shared and implemented internal thread safety. They were held in containers and it was common for functions to extend their lifetimes.

This particular type of object could live in two containers. One when it was active and one when it was inactive. Some operations worked on active objects, some on inactive objects. The error case occurred when a command was received on an inactive object that made it active while the only shared_ptr to the object was held by the container of inactive objects.

The inactive object was located in its container. A reference to the shared_ptr in the container was passed, by reference, to the command handler. Through a chain of references, this shared_ptr ultimately got to the code that realized this was an inactive object that had to be made active. The object was removed from the inactive container (which destroyed the inactive container's shared_ptr) and added to the active container (which added another reference to the shared_ptr passed to the "add" routine).

At this point, it was possible that the only shared_ptr to the object that existed was the one in the inactive container. Every other function in the call stack just had a reference to it. When the object was removed from the inactive container, the object could be destroyed and all those references were to a shared_ptr that that no longer existed.

It took about a month to untangle this.

Why do we need to pass const shared pointer as reference?

But can't we just do this const FramePtr frame?

Sure, we could, but then the copy constructor would be invoked, which is for larger types (anything larger than the built-in types) normally more expensive than passing by reference. This is nothing specific to shared_ptr. It should be generally your default to pass any objects by const reference if you don't need a copy and don't want to change them. Only built-in types like int, float, or char should be passed-by-value.

More interesting is why func1 uses a copy. Most probable case is that he needs a copy anyway, because he wants to keep a reference in the class. I couldn't find the exact file you're refering to in the github repository you've posted. If it's still unclear please past the function body of func1 into the question.

Edit: Ah, I see. Looks like the reason he passes-by-value here, has more to do with thread-safety. Didn't read the whole but otherwise the shared_ptr might be deleted by the owning thread if he passed by const reference.

Here for examle func needs pass-by-value cause otherwise the pointer could be deleted by the main thread. Probably something like this but more complicated:

#include <chrono>
#include <iostream>
#include <memory>
#include <thread>

using namespace std::chrono_literals;

struct S {
    S() {}
};

void
//This signature would be false
//func(std::shared_ptr<S> const& s)
func(std::shared_ptr<S> s)
{
    std::cout << s.use_count() << '\n';
    std::this_thread::sleep_for(2s);
    //use_count would be 0 here if we pass by reference
    std::cout << s.use_count() << '\n';
}

int
main(int argc, char**) {
    std::shared_ptr<S> s{std::make_shared<S>()};

    std::thread t{func, std::ref(s)};

    std::this_thread::sleep_for(1s);

    s.reset();

    t.join();

    return 0;
}

When to pass by pointer or shared_ptr

I must use pointers in order to retain polymorphic behaviours as I am storing the passed object into a vector for later use

If you store the pointed object in vector, then you don't retain polymorphic behaviours.

Should I ... use shared_ptr::get to get the actual pointer, pass that pointer, re-wrap it using shared_ptr::reset, and then add it to the vector?

No. A shared pointer may not take ownership of the pointer that is already owned by another shared pointer. This would have undefined behaviour.

(because I should only pass smart pointers if I'm transferring ownership)

If you intend to store a shared pointer to the object, then you are transferring (sharing) the ownership. If that is your intention, then pass a const reference to the shared pointer, as described in the linked answer.

If you don't intend to share the ownership, then storing a shared pointer is not what you should do. You may want to store a reference wrapper, bare pointer, or a weak pointer instead. How you should pass the reference to the function, will depend on what you choose to do with it.

---

Related Topics