How to avoid clang-tidy warning when passing shared_ptr to thread function?

Question

I have C++ code that looks (simplified) like this:

#include <unistd.h>

#include <iostream>
#include <memory>
#include <string>
#include <thread>

std::thread the_thread;

void my_thread_func(const std::shared_ptr<std::string>& string_ptr) {
    std::cout << "at thread begin: " << *string_ptr << std::endl;
    sleep(1);
    std::cout << "at thread finish: " << *string_ptr << std::endl;
}

void start_thread() {
    auto local_string_ptr = std::make_shared<std::string>("foobar");
    the_thread = std::thread(&my_thread_func, local_string_ptr);
}

int main(int, char**) {
    start_thread();
    sleep(2);
    the_thread.join();
}

(on Compiler Explorer: https://godbolt.org/z/9GYTf7orz)

When I run clang-tidy --checks=performance-unnecessary-value-param for this code, it warns:

<source>:10:50: warning: the parameter 'string_ptr' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
   10 | void my_thread_func(std::shared_ptr<std::string> string_ptr) {

But my understanding is that passing string_ptr as const reference to my_thread_func() would be incorrect, since then the thread would have a reference to an object that is local to start_thread() and that is destroyed while the thread is running.

My questions:

• would it be correct (and recommended) to use void my_thread_func(const std::shared_ptr<std::string>& string_ptr), or would that be an error?
• is the clang-tidy warning correct, or is it a false positive?
• is my code generally incorrect/inelegant, thereby causing this problem this problem in the first place? I.e. what would be the best way to write this?

My experience is that clang-tidy is usually correct :-) that's why I'm confused about the warning here.

Answer 1

The clang-tidy warning assumes a single-threaded context: an ordinary call to a function

int get(std::shared_ptr<int> p,std::binary_semaphore &s) {
  s.acquire();
  return *p;
}

might very well need the std::shared_ptr parameter (object, not reference) to keep the int alive long enough to return it. (The caller might have obtained a reference to the std::shared_ptr from some table rather than constructing it for the call, and whatever signals the semaphore might drop the only other reference immediately after doing so.)

Separately, even if the function called for a thread has a reference parameter, it will not be a reference to an argument to the std::thread constructor: each such argument is copied into a temporary T whose lifetime is that of the thread, and the reference refers to T. (As mentioned in the comments, you can use std::ref to produce a std::reference_wrapper that can convert back into a reference to the original argument.) That means that you might not need a std::shared_ptr here at all: you can just construct the object as an argument, and it will be moved into T.