March 22nd, 2024

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

Raymond Chen

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.

Author

Raymond Chen

Raymond has been involved in the evolution of Windows for more than 30 years. In 2003, he began a Web site known as The Old New Thing which has grown in popularity far beyond his wildest imagination, a development which still gives him the heebie-jeebies. The Web site spawned a book, coincidentally also titled The Old New Thing (Addison Wesley 2007). He occasionally appears on the Windows Dev Docs Twitter account to tell stories which convey no useful information.

7 comments

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

Sort by :

Newest

Newest Popular Oldest

Stuart Ballard March 25, 2024 0
Is there a way to use templates to provide a wrapper function that solves this problem for the general case? Something like this:
<code>

Read more
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);
}
```
Read less
- 紅樓鍮 March 26, 2024 · Edited 1
  If is a plain function pointer, you can pattern-match on its signature:
  <code>
  But you'd then also want to handle things like member function pointers, const member function pointers, objects with an unoverloaded (very hard), etc. And it won't work at all for objects with overloaded or templated (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...
  Read more
  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).
  Read less
  - Stuart Ballard March 26, 2024 · Edited 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...
    Read more
    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)?
    
    Read less
  - 紅樓鍮 March 26, 2024 · Edited 0
    Why is it necessary to use a closure here
    
    No, you don't need to use a lambda if you can convert the to without using a tuple. Unfortunately C++ doesn't support syntax like
    <code>
    but it seems like you can write
    <code>
    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 ...
    Read more
    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.
    
    Read less
  - Raymond Chen Author March 26, 2024 0
    
    Another annoying wrinkle is that the parameter packs may not be the same length thanks to default parameters.
Shao Voon Wong March 22, 2024 · Edited 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...
Read more
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.

Read less
BCS March 22, 2024 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.

<code>

And the solution then becomes:

<code>

---

Of particular note, there are IIRC a...
Read more
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.
Read less