Why is beginthreadex thread argument variable not updating in parent thread

Why is beginthreadex thread argument variable not updating in parent thread



I have a thread which creates a hidden window for the purpose of receiving WinAPI messages based on power state. I need to get the HWND of the created window from the thread so that I can throw a WM_QUIT message to close the window and end the thread gracefully:


HWND


WM_QUIT



Main:


HWND hiddenWindowHandle = NULL;
HANDLE PowerWindowThreadHandle = (HANDLE)_beginthreadex(0, 0, &windowsPowerThread, (void*)&hiddenWindowHandle, 0, 0);



Thread:


unsigned int __stdcall windowsPowerThread(void* data)
{
HWND hiddenWindowHandle = createHiddenWindow();
HWND hwHandle = *(HWND*)data;
hwHandle = hiddenWindowHandle;
...



The problem is that the hiddenWindowHandle is not being updated with the generated HWND.


hiddenWindowHandle


HWND



I have verified in the thread that it's being created, and I've verified that I'm not trying to access the handle before the thread creates it.



What am I missing here?






How exactly do you make sure that the reading thread waits until the value is available and perform the necessary synchronization? If the first thing the other thread does is create that window, why not create the window in the main thread and pass the handle as argument to the thread function!? That'd eliminate all need for synchronization…

– Michael Kenzel
Sep 16 '18 at 23:31






Because I print the validity of the HWND in the thread, and in main have a loop I break via a cin after which I print the value as it's seen from main. The thread shows a valid handle, I press enter, main doesn't. The solution you mention of course would work, but at the moment I want to know why this solution (which I've seen in multiple answers and forums) doesn't work.

– Connor Spangler
Sep 16 '18 at 23:32



HWND


cin






Basic coding bug, you are only updating the local variable. Use HWND* phwnd = (HWND*)data; *phwnd = hiddenWindowHandle; And make sure that the the other hiddenWindowHandle variable (pick a different name!) is not a local variable, its address needs to stay valid while the thread is running.

– Hans Passant
Sep 17 '18 at 0:13







You are trying to implement the wrong solution (see Why is there a special PostQuitMessage function?). A better alternative would be to use a (named) event, signaled from the UI thread when it's time to shut down that thread, and change your thread's message loop to use MsgWaitForMultipleObjects. Added bonus: You don't have to communicate any information between threads, or expose an implementation detail (the window).

– IInspectable
Sep 17 '18 at 7:37







My suggestion accomplishes exactly what you are trying to accomplish. Without the need to communicate any information. And it doesn't invert responsibilities (like the code you are trying to implement). With your proposed solution, the window suddenly becomes responsible for cleaning up the thread that owns it. If you do not want to confuse readers of your code, follow that model: Windows are owned by threads. If you want a thread to terminate, signal an event, and have the thread cleanup the resources it owns.

– IInspectable
Sep 17 '18 at 9:54




1 Answer
1



Your code lacks the necessary synchronization. What you have here is a data race. Thus, what you get is strictly undefined behavior. What will most likely happen is that the compiler simply does not re-fetch the value of hiddenWindowHandle from memory in every iteration of the loop, since it can simply assume that the value does not change. One possible solution would be to make hiddenWindowHandle an std::atomic and have the main thread perform a busy wait until the value changes from NULL. Alternatively, you could put all access to the shared variable into a critical section locked by a mutex, or use a condition variable to wait for the value to be available.


hiddenWindowHandle


hiddenWindowHandle


std::atomic


NULL



So if I understand your code correctly, the thread that creates the window receives a pointer to the result variable in the form of a void* and then tries to communicate the result like so:


void*


unsigned int __stdcall windowsPowerThread(void* data)


HWND hwHandle = *(HWND*)data;
hwHandle = hiddenWindowHandle;




There's two problems here. First of all, data doesn't point to an HWND, it points to an std::atomic<HWND> now, so you already have undefined behavior there. The main problem, and probably the explanation why your original code didn't just happen to work anyways, despite the data race, is that you create a new local HWND called hwHandle. This local variable is initialized with the value of whatever data points to. You then assign your result to that local variable, but never to the actual result variable.


data


HWND


std::atomic<HWND>


HWND


hwHandle


data



What you want to do is more something along the lines of


unsigned int __stdcall windowsPowerThread(void* data)


HWND hiddenWindowHandle = createHiddenWindow(…);
*static_cast<std::atomic<HWND>*>(data) = hiddenWindowHandle;




You may also want to consider using std::thread instead of the raw CRT functions.


std::thread






Comments are not for extended discussion; this conversation has been moved to chat.

– Samuel Liew
Sep 17 '18 at 14:16



Thanks for contributing an answer to Stack Overflow!



But avoid



To learn more, see our tips on writing great answers.



Required, but never shown



Required, but never shown




By clicking "Post Your Answer", you agree to our terms of service, privacy policy and cookie policy

Popular posts from this blog

𛂒𛀶,𛀽𛀑𛂀𛃧𛂓𛀙𛃆𛃑𛃷𛂟𛁡𛀢𛀟𛁤𛂽𛁕𛁪𛂟𛂯,𛁞𛂧𛀴𛁄𛁠𛁼𛂿𛀤 𛂘,𛁺𛂾𛃭𛃭𛃵𛀺,𛂣𛃍𛂖𛃶 𛀸𛃀𛂖𛁶𛁏𛁚 𛂢𛂞 𛁰𛂆𛀔,𛁸𛀽𛁓𛃋𛂇𛃧𛀧𛃣𛂐𛃇,𛂂𛃻𛃲𛁬𛃞𛀧𛃃𛀅 𛂭𛁠𛁡𛃇𛀷𛃓𛁥,𛁙𛁘𛁞𛃸𛁸𛃣𛁜,𛂛,𛃿,𛁯𛂘𛂌𛃛𛁱𛃌𛂈𛂇 𛁊𛃲,𛀕𛃴𛀜 𛀶𛂆𛀶𛃟𛂉𛀣,𛂐𛁞𛁾 𛁷𛂑𛁳𛂯𛀬𛃅,𛃶𛁼

Edmonton

Crossroads (UK TV series)