Why does my thread get a broken string as its initial parameter?

Raymond Chen

March 22nd, 20247 1

A customer passed a string to a newly-created std::thread but found that the std::string received by the thread was already invalid. How can that happen?

Here was how they passed the parameter to the thread:

void OnWidgetChanged(const char* reason)
{
    std::thread backgroundThread(ProcessWidgetChange,
        reason);
    backgroundThread.detach();
}

void ProcessWidgetChange(std::string reason)
{
    // the reason is corrupted!
}

The problem is that the raw const char* pointer is being converted to a std::string too late.

To avoid confusion, let me rename the two reason parameters:

void OnWidgetChanged(const char* rawReason)
{
    std::thread backgroundThread(ProcessWidgetChange,
        rawReason);
    backgroundThread.detach();
}

void ProcessWidgetChange(std::string stringReason)
{
    // the stringReason is corrupted!
}

The parameters to the std::thread constructor are captured by value, and the copies are then passed to the new thread, which executes the desired code by passing those copies to std::invoke. The specification calls out that the capturing happens in the calling thread as part of the constructor.

Here’s a flowchart of the operations that take place:

Original thread  
Capture parameters by value  
Start new thread  
⤳ New thread
  Convert parameters to match signature of target
  Call the target
  Destruct converted parameters

In the above example, it means that the raw const char* is captured and given to the thread. Only when the thread begins execution is the raw const char* converted to a std::string, and by that point, it’s too late.

Original thread  
Copy the rawReason raw pointer  
Start new thread  
⤳ New thread
  Convert: Construct stringReason from rawReason
  Call the target with stringReason
  Destruct: Destruct stringReason and rawReason

The problem is clearer when the conversion is spelled out: The original code has already detached the thread and returned, at which point the caller of OnWidgetChanged is not under any obligation to keep the pointer valid any further. The conversion to a std::string therefore happens too late.

The author of the above code was under the incorrect belief that the operations occurred in a different order:

Original thread  
Convert: Construct stringReason from rawReason  
Capture converted parameters  
Start new thread  
⤳ New thread
  Call the target
  Destruct: Destruct stringReason

If you think about it, it’s not possible to do things the second way. The thread constructor could try to infer the signature of the callable, but the callable might have multiple operator() overloads, so the tricks for deconstructing the function pointer don’t work. And even if they did work, it’s possible that the caller didn’t pass the same number of parameters as the function expects, but expecting it to work because the missing parameters have default values. But there’s no way to deduce the default values (because default values are not part of the function type signature), so the converter doesn’t know what values to use for the missing parameters.

The solution is to convert the rawReason raw pointer to a std::string while the rawReason is still valid, pass that converted value to the std::thread constructor.

void OnWidgetChanged(const char* rawReason)
{
    std::thread backgroundThread(ProcessWidgetChange,
        std::string(rawReason));
    backgroundThread.detach();
}

void ProcessWidgetChange(std::string stringReason)
{
    // use the stringReason
}

In the original customer version, the issue wasn’t about an anticipated conversion from a raw pointer to a std::string, but rather an anticipated conversion of a raw COM pointer to a smart one:

void OnWidgetChanged(IWidget* widget)
{
    // Oops, captures raw pointer, not ComPtr.
    std::thread backgroundThread(ProcessWidgetChange,
        widget);
    backgroundThread.detach();
}

void ProcessWidgetChange(ComPtr<IWidget> widget)
{
    widget->Refresh();
}

The solution, as before, is to force the conversion early:

void OnWidgetChanged(IWidget* widget)
{
    std::thread backgroundThread(&ProcessWidgetChange,
        ComPtr<IWidget>(widget));
    backgroundThread.detach();
}

Bonus chatter: It would have been nice to use Class Template Argument Deduction (CTAD) to simplify this to

void OnWidgetChanged(IWidget* widget)
{
    std::thread backgroundThread(&ProcessWidgetChange,
        ComPtr(widget));
    backgroundThread.detach();
}

Unfortunately, as we saw earlier, WRL’s ComPtr doesn’t support CTAD.

Raymond Chen

Read next

7 comments

Discussion is closed. Login to edit/delete existing comments.

  • BCS 0

    Arguably, the root issue is passing `ProcessWidgetChange` as a function pointer. A case can be made that in C++ the name of a function should only ever be treated as an overload set and the only valid operation on an overload set is to call it.

    If the code had been written like the following, it would be a bit more clear what the problem is.

    void OnWidgetChanged(const char* reason)
{
    std::thread backgroundThread([reason] {
      ProcessWidgetChange(reason);
    });
    backgroundThread.detach();
}

    And the solution then becomes:

    void OnWidgetChanged(const char* reason)
{
    std::thread backgroundThread([reason=std::string{reason}] {  // force conversion here
      ProcessWidgetChange(reason);
    });
    backgroundThread.detach();
}

    —

    Of particular note, there are IIRC a few standard defined identifiers where this is not just a good idea, but required; they are not even defined to be functions despite acting as one syntactically.

  • Shao Voon Wong 0

    I also encountered this problem. I have documented it at https://www.codeproject.com/Articles/5377342/VCplusplus-30-Multithreading-Mistakes-on-Windows

    Note: for the original problem of COM ptr. COM ptr needs to be marshalled to another thread. It did not prompt any error because the worker thread did not call CoInitialize() to initialize the COM apartment.

    EDIT2: In the COM case, it can be solved without using COM smart pointer. Just call AddRef() on the raw interface pointer in the main thread to increment reference count to keep it alive and Release() to decrement it in the worker thread.

  • Stuart Ballard 0

    Is there a way to use templates to provide a wrapper function that solves this problem for the general case? Something like this:

    template <typename F, typename... Args>
std::thread make_thread(F f, Args... args)
{
  // Some kind of template assert using something like is_same_t<> between
  // TArgs and the parameter types of F to force the template to resolve to
  // exactly those types, so that conversions happen in the call to
  // _this_ function

  // I originally thought that sizeof(f(...args)) would be enough, but
  // I think that was wrong because that's still legal with the unconverted
  // argument types.

  return std::thread(f, ...args);
}
    • 紅樓鍮 1

      If F is a plain function pointer, you can pattern-match on its signature: 

      template <typename R, typename... Params, typename... Args>
std::thread make_thread(R (*f)(Params...), Args &&... args) {
  using decay_tuple = std::tuple<std::decay_t<Params>...>;
  return {[f, tp = decay_tuple(std::forward<Args>(args)...)] mutable {
    std::apply(f, std::move(tp));
  }};
}

      But you’d then also want to handle things like member function pointers, const member function pointers, objects with an unoverloaded operator() (very hard), etc. And it won’t work at all for objects with overloaded or templated operator()(s).

      Edit: Removed “(very hard)” as it can be implemented in terms of the member function pointer overload.

      PS: The implementation above never works with functions taking non-const lvalue references (std::thread‘s constructor won’t allow you to simply pass an lvalue at the position of a non-const lvalue reference parameter either; it requires you to express your (dangerous) intent by using std::ref. The implementation above can also be made to work with std::ref with some metaprogramming dance).

      • Stuart Ballard 0

        That’s fascinating! I’m not a C++ programmer myself but I’ve read enough of this blog to have an appreciation of how powerful and how intricate it can get. The way you’re pattern matching on the function’s signature is super clever and I wish I’d thought of it. I had the vague idea that it might be possible to template the Params types and Args types separately, but after poring over C++ specs for things like is_invocable_t for a while I couldn’t see how to do it.

        Why is it necessary to use a closure here, by the way? Is there not a way to avoid that by doing something like

        Params... converted_args = std::something<Params...>(args...);
std::thread new_thread(f, converted_args);
return new_thread;

        Edit: Is R allowed to resolve to void, or would a separate overload be needed if we want to accept void-returning functions?
        Edit 2: Does the std::thread constructor work with the special cases you mentioned (member functions, overloaded operator() etc)?

        • 紅樓鍮 0

          Why is it necessary to use a closure here

          No, you don’t need to use a lambda if you can convert the Args to Params without using a tuple. Unfortunately C++ doesn’t support syntax like

          Params... converted_args = something(args)...;

          but it seems like you can write

          std::thread(f, something<Params>(args)...)

          I used a tuple because I totally forgot you can zip-unpack two different parameter packs (of the same length) directly, and I used a lambda because then I needed to use std::apply and couldn’t directly invoke f.

          • Raymond ChenMicrosoft employee 0

            Another annoying wrinkle is that the parameter packs may not be the same length thanks to default parameters.

Feedback usabilla icon