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

user_1287554 0
Viewed 781

This page on Thread Safety by Microsoft says shared_ptr should be used even if there are multiple copies sharing the same object.

So does this mean that both of the following are acceptable? I've tried both and they appear to work fine.

EDIT: The actual business objective is to get string updates from the long running thread to the main thread. I figured I should use shared_ptr since string is not thread safe. Don't care about ownership honestly.

Option 1 (Passing reference):

auto status = std::make_shared<std::string>();
auto f = [&status]() {
    ...
  *status = "current status";
    ...
};

std::thread t{f};

while(true) {
  std::cout << *status << std::endl;
  std::this_thread::sleep_for(1000ms);
  if (*status == "completed") break;
}

t.join();

Option 2 (Making a copy):

auto status = std::make_shared<std::string>();
auto f = [](std::shared_ptr<std::string> s) {
    ...
  *s= "current status";
    ...
};

std::thread t{f, status};

while(true) {
  std::cout << *status << std::endl;
  std::this_thread::sleep_for(1000ms);
  if (*status == "completed") break;
}

t.join();

EDIT2: So apparently both these approaches are wrong for what I'm trying to achieve. I need to use std::mutex (cppreference) and not muck around with shared_ptr. See second half of this answer.

c++
4 Answers

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.

user_20793030

It is weird to accept shared_ptr by reference as you lose the whole point of using shared_ptr in the first place. You may just use a raw pointer instead.

There are cases when accepting by reference of shared_ptr is legitimate but if you give a reference of it to a thread then it will cause UB once that instance of the shared_ptr is destroyed and the thread still uses the shared_ptr.

Primary purpose of shared_ptr is to manage lifetime of the object. If you pass a reference of it to a thread then you throw away the whole purpose and advantages of the shared_ptr.

user_108036710

if you use a reference you can't detach the thread.

for example, this program will be crashed:

#include <thread>
#include <chrono>
#include <string>
#include <iostream>
void f1()
{
  auto status = std::make_shared<std::string>();
  auto f = [&status]() 
  {
     std::this_thread::sleep_for(std::chrono::seconds(1)); 
    *status = "current status";
  };
  std::thread t{f};
  t.detach();
}
int main() { 
  f1();
  std::string status="other status";//use the frame
  std::this_thread::sleep_for(std::chrono::seconds(1)); 
  std::cout<<status<<std::endl; //check the frame
}
user_28521650

When you pass a reference to status the lambda, it means that you would need make sure yourself that the lambda does not outlive the status variable.

Imagine that we would want to move the thread creation to a separate function:

std::thread
spawn_thread( std::shared_ptr< std::string > status )
{
    auto f = [&status]( ) {
        // Adding a sleep to make sure this gets executed after we exit spawn_thread function.
        std::this_thread::sleep_for( std::chrono::milliseconds( 500 ) );
        *status = "current status";
    };

    std::thread t{f};
    return t;
}

int
main( )
{
    auto status = std::make_shared< std::string >( );
    auto thread = spawn_thread( status );

    while ( true )
    {
        std::cout << *status << std::endl;
        std::this_thread::sleep_for( std::chrono::milliseconds( 1000 ) );
        if ( *status == "completed" )
        {
            break;
        }
    }

    thread.join( );
}

Running this code will most likely result in a crash, since variable status (not the shared data behind it) gets out of scope before we access it within the lambda.

Of course we could pass the reference to status to the spawn_thread function, but then we propagate the problem further - now the caller of spawn_thread needs to make sure that this variable outlives the thread.

std::shared_ptr is designed for the cases when you do not want to manually control the lifetime of the object passed around, but in order for it to work you need to pass it by value so that the internal mechanism keeps count of the number of shared_ptr instances.

Keep in mind that while passing around and copying shared_ptr is thread safe, concurrent reads and writes of the value stored inside it is not.

user_125011370
Related