Again code review is probably the only way to find this bug
-
Struggled a week to find what was wrong. Then thoroughly analyzed the code and figured the problem in an instant. The problem: On some rare occasions in customers machines deadlock occurred.
const int WM_ASK_THREAD_TO_DO_TASK = WM_USER + 100;
void ThreadMessageLoop()
{
while(GetMessage( &msg, NULL, 0, 0 ) > 0)
{
if (msg.message == WM_ASK_THREAD_TO_DO_TASK)
{
DoTask();
SetEvent((HANDLE)msg.wparam);
}TranslateMessage(&msg); DispatchMessage(&msg); }
}
void AskThreadToDoTask()
{
HANDLE hTaskComplete = CreateEvent(NULL, FALSE, FALSE, NULL);PostThreadMessage(dwThreadId, WM\_ASK\_THREAD\_TO\_DO\_TASK, (WPARAM)hTaskComplete, 0); WaitForSingleObject(hTaskComplete, INFINITE); CloseHandle(hTaskComplete);
}
Hints:
- DoTask does not throw any exception.
- There might be other windows on the thread executing tasks.
Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. -Brian Kernighan
-
Struggled a week to find what was wrong. Then thoroughly analyzed the code and figured the problem in an instant. The problem: On some rare occasions in customers machines deadlock occurred.
const int WM_ASK_THREAD_TO_DO_TASK = WM_USER + 100;
void ThreadMessageLoop()
{
while(GetMessage( &msg, NULL, 0, 0 ) > 0)
{
if (msg.message == WM_ASK_THREAD_TO_DO_TASK)
{
DoTask();
SetEvent((HANDLE)msg.wparam);
}TranslateMessage(&msg); DispatchMessage(&msg); }
}
void AskThreadToDoTask()
{
HANDLE hTaskComplete = CreateEvent(NULL, FALSE, FALSE, NULL);PostThreadMessage(dwThreadId, WM\_ASK\_THREAD\_TO\_DO\_TASK, (WPARAM)hTaskComplete, 0); WaitForSingleObject(hTaskComplete, INFINITE); CloseHandle(hTaskComplete);
}
Hints:
- DoTask does not throw any exception.
- There might be other windows on the thread executing tasks.
Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. -Brian Kernighan
- WM_USER should not be used on Win32, WM_APP should be used instead - Window messages are of type UINT, not int - Who posts the WM_QUIT message? What happens if the thread terminates before the WM_ASK_THREAD_TO_DO_TASK gets processed? WaitForMultipleObjects ([hThread, hTaskComplete]) would solve that problem. - Where and when is the thread message queue created? PostThreadMessage might fail. - Why is the TranslateMessage and DisptachMessage not in an else branch? Otherwise the whole code is pointless, why tell another thread to execute a function and then wait for the thread? Does the function have to be executed on the UI thread?