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?
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
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