Pass non-dynamic memory allocation to CreateThread() function

Question

I pass a address of USB_INSTACE_DATA variable to function CreateThread() like this. Is it safe to use, or I must use dynamic memory allocation for USB_INSTACE_DATA data?

DWORD WINAPI MyThreadFunction(LPVOID lpParam);
void GetUSBInfo(PDEV_BROADCAST_DEVICEINTERFACE pDevInf, WPARAM wParam) {

    USB_INSTACE_DATA data{ pDevInf, wParam };
    DWORD   dwThreadId;
    auto hThreadArray = CreateThread(
        NULL,                   // default security attributes
        0,                      // use default stack size  
        MyThreadFunction,       // thread function name
        &data,                  // argument to thread function 
        0,                      // use default creation flags 
        &dwThreadId);           // returns the thread identifier
}

[Don't post images of text](https://meta.stackoverflow.com/questions/285551/why-should-i-not-upload-images-of-code-data-errors-when-asking-a-question). Copy-paste text *as text*. — Some programmer dude, Sep 24 '22 at 10:39
Please show a [mcve]. The code shown doesn't compile, which means that **your** code is different. That difference hides the crucial detail whether `data` has static storage duration or automatic storage duration. — IInspectable, Sep 24 '22 at 11:01
Unless you have very specific reasons to only use Windows-specific functions, use the C++ standard [`std::thread`](https://en.cppreference.com/w/cpp/thread/thread). That will make it *very* easy to pass object by value instead of using pointers. — Some programmer dude, Sep 24 '22 at 15:28
Also, if you create a thread, either you need to save it so you can *join* it later, or you need to detach it if it should run totally independent (but be careful when exiting your program, if you want to the thread to keep on working). — Some programmer dude, Sep 24 '22 at 15:29
@Someprogrammerdude That second comment reads: *"Don't use `std::thread`. It's bad. Riddled with crazy gotchas. It's so bad even that we had to supplant it with another type, `std::jthread`"*. Yes, that sounds about right. — IInspectable, Sep 24 '22 at 15:58

score 2 · Accepted Answer · answered Sep 24 '22 at 11:11

This isn't safe. USB_INSTACE_DATA data{ pDevInf, wParam }; has automatic storage duration. Once the enclosing scope ends (i.e. when the function returns), the memory is cleaned up.

The thread launched through CreateThread, however, holds a pointer &data beyond the function's scope. This is an instance of the classical use-after-free bug.

The solution is to make data live at least as long as any client holding a reference/pointer to it. You can use memory with static storage duration, or manual memory management (such as heap allocations). Mind you, this doesn't automagically solve any other issues inherent to concurrent programs such as dealing with data races. That's an issue you will have to address separately.

Pass non-dynamic memory allocation to CreateThread() function

1 Answers1