winapi

Async Find File thread not working with GUI in Win32 C++ application

I have a Find File operation which takes place in a separate thread to avoid locking up the GUI, and is started with CreateThread. As it runs it needs to (1) update the GUI Status Bar in the main application and (2) be interruptible from a toolbar button.

Neither one of these is working: (1) the PostMessage which posts to the GUI StatusBar window is ignored, (2) GUI toolbar clicks are also ignored and the application is still locked up.

HANDLE hSearchThread;

void MainWindow_Search() {
    // ...
    // Construct Search Criteria object (3 variables: Name/CaseSensitive/StartPath)
    APP_SEARCH_CRITERIA_SPEC* searchCriteria = new APP_SEARCH_CRITERIA_SPEC({name,caseSensitive,startPath});

    // Start Thread
    hSearchThread = CreateThread(NULL/*default security attrs*/, 
                                        0/*default stack size*/, 
                                        App_Thread_Search, 
                                        searchCriteria, 
                                        0/*default creation flags*/, NULL);

    // ... Verified hSearchThread is successfully created. Start waiting
    WaitForSingleObject(hSearchThread, INFINITE);

    // Close thread handle and delete Search Criteria object
    CloseHandle(hSearchThread);
    if (searchCriteria != NULL) {
        delete searchCriteria;
        searchCriteria = NULL; // also manually set to NULL
    }
}

Actual Find-File Thread:

DWORD WINAPI App_Thread_Search(LPVOID lParam) {

    APP_SEARCH_CRITERIA_SPEC* searchCriteria = (APP_SEARCH_CRITERIA_SPEC*)lParam;

    std::queue<std::wstring> paths;
    // Put specified Start Path on the queue
    paths.push(searchCriteria->startPath);

    std::vector<FILE_OR_DIR_ITEM> pathContents;
    std::wstring currentPath;
    std::wstring statusBarText;

    // Main Loop
    while (!paths.empty() && 
           appSearchInProgress/* Global var that can be set to FALSE from outside to stop */) {
       // Get first path
       currentPath = paths.front();
       // Update Status Bar (NOT WORKING, hWndStatusBar is valid)
       statusBarText = L"Searching folder: " + currentPath;
       PostMessage(hWndStatusBar, SB_SETTEXT, 1, (LPARAM)statusBarText.c_str());

       // Pop first path
       paths.pop();
       
       // Get path contents. If directories found, push them on the queue
       pathContents = GetPathContents(...); // utility function
       for (FILE_OR_DIR_ITEM itemFound : pathContents) {
           if (itemFound.isDirectory) {
              paths.push(itemFound.path + L"\\" + itemFound.name);
           }
       }

       // ...etc: If matching substring, add to Search Results
    }
    // return 0 at the end
    return 0;
}

From the Main Window Proc, toolbar button to terminate (WM_NOTIFY), not reached while the search is running, only reachable after it stops. I thought having a separate thread was supposed to keep the main GUI functional.

// Process messages for Main Window
LRESULT CALLBACK WndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam) {
    //..
    case WM_NOTIFY:
    {

       LPNMMOUSE lpnmMouse = (LPNMMOUSE)lParam;
       NMHDR nmHdr = lpnmMouse->hdr;

       HWND hWndFrom = nmHdr.hwndFrom;
       UINT code = nmHdr.code;  

       //...
       if (hWndFrom == hWndToolbar && code == NM_CLICK) {
          //...
          if (lpnmMouse->dwItemSpec == IDM_STOP_SEARCH) {
            // Stop thread
            TerminateThread(hSearchThread, 0);
            // Set global var to FALSE
            appSearchInProgress = false;
        }
    }
}
Solution

  • You are blocking your main thread while the search thread is running. WaitForSingleObject() will not exit back to the caller until the search thread has fully terminated. That means your main thread is not able to run an active message loop to process any UI messages. That is why your UI is unresponsive until the search is finished.

    If you need to perform a blocking wait while still servicing the UI, then use MsgWaitForMultipleObjects() instead, and run your own secondary message loop whenever messages are available.

    And, absolutely NEVER use TerminateThread() to stop a worker thread under normal conditions! You already have a way to signal your search thread to stop, so just let it stop on its own time.

    For example:

    std::atomic_bool appSearchInProgress{false};

void MainWindow_Search() {
    //...

    APP_SEARCH_CRITERIA_SPEC searchCriteria(
        {name,caseSensitive,startPath}
    );

    appSearchInProgress.store(true);
    HANDLE hSearchThread = CreateThread(NULL/*default security attrs*/, 
                                        0/*default stack size*/, 
                                        App_Thread_Search, 
                                        &searchCriteria, 
                                        0/*default creation flags*/, NULL);

    if (!hSearchThread) {
        // error handling...
        appSearchInProgress.store(false);
        return;
    }

    do {
        DWORD res = MsgWaitForMultipleObjects(1, &hSearchThread, FALSE, INFINITE, QS_ALLINPUT);

        if (res == WAIT_OBJECT_0) {
            // thread is finished!
            break;
        }

        if (res == WAIT_FAILED) {
            // error handling...
            appSearchInProgress.store(false);
            if (WaitForSingleObject(hSearchThread, 30000) == WAIT_TIMEOUT) {
                TerminateThread(hSearchThread, 0);
            }
            break;
        }

        // pump the message queue...
        MSG msg;
        while (PeekMessage(&msg, NULL, 0, 0, PM_REMOVE)) {
            TranslateMessage(&msg);
            DispatchMessage(&msg);
        }
    }
    while (true);

    CloseHandle(hSearchThread);

    // use search results as needed...
}

// Process messages for Main Window
LRESULT CALLBACK WndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam) {
    //...
    case WM_NOTIFY:
    {
        //...
        if (lpnmMouse->dwItemSpec == IDM_STOP_SEARCH) {
            // Stop thread by setting global var to FALSE
            appSearchInProgress.store(false);
        }
        break;
    }
    //...
}

    However, there is really no point in creating a worker thread just to immediately wait on its completion. You may as well just run your search logic directly in the main thread, and pump the message queue in between search iterations.

    For example:

    bool appSearchInProgress = false;

void MainWindow_Search() {
    //...
    appSearchInProgress = true;
    while (!paths.empty() && appSearchInProgress) {
        //...

        // pump the message queue...
        MSG msg;
        while (PeekMessage(&msg, NULL, 0, 0, PM_REMOVE)) {
            TranslateMessage(&msg);
            DispatchMessage(&msg);
        }
    }
}

    But, if you really want to use a worker thread, then it should be handled truly asynchronous. Just let it run in the background, and DON'T wait on it (except maybe if the app needs to close while the search is in progress). After starting the search thread, return back to the caller immediately so it can go back to the main message loop. Have the search thread notify your main thread when it is finished running.

    If you don't want the user to interact with certain UI elements while the search is running, then simply disable those elements until the worker thread is finished.

    For example:

    HANDLE hSearchThread = NULL;
std::atomic_bool appSearchInProgress{false};

static const UINT WM_SEARCH_FINISHED = WM_APP + 1;

void MainWindow_Search_Async() {
    //...

    auto searchCriteria = std::make_unique<APP_SEARCH_CRITERIA_SPEC>(
        {name,caseSensitive,startPath}
    );

    appSearchInProgress.store(true);
    hSearchThread = CreateThread(NULL/*default security attrs*/, 
                                        0/*default stack size*/, 
                                        App_Thread_Search, 
                                        searchCriteria.get(), 
                                        0/*default creation flags*/, NULL);

    if (!hSearchThread) {
        // error handling...
        appSearchInProgress.store(false);
        return;
    }

    // release ownership since CreateThread() captured it...
    searchCriteria.release();

    // disable UI elements as needed...
}

DWORD WINAPI App_Thread_Search(LPVOID lParam) {

    // take ownership since CreateThread() captured it...
    std::unique_ptr<APP_SEARCH_CRITERIA_SPEC> searchCriteria{
        static_cast<APP_SEARCH_CRITERIA_SPEC*>(lParam)
    };

    //...

    PostMessage(hWndMainWnd, WM_SEARCH_FINISHED, 0, 0);
    return 0;
}

// Process messages for Main Window
LRESULT CALLBACK WndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam) {
    //...
    case WM_NOTIFY:
    {
        //...
        if (lpnmMouse->dwItemSpec == IDM_STOP_SEARCH) {
            // Stop thread by setting global var to FALSE
            appSearchInProgress.store(false);
        }
        break;
    }

    case WM_SEARCH_FINISHED:
    {
        appSearchInProgress.store(false);
        CloseHandle(hSearchThread);
        hSearchThread = NULL;

        // use search results as needed...
        // re-enable UI elements as needed...

        return 0;
    }
    //...
}

    Also, on a side note:

    When sending a string pointer across thread boundaries, you must ensure the string data remains alive in memory until after the message has been processed (just like you have to do with your APP_SEARCH_CRITERIA_SPEC object). However, since you are not doing that, you are risking sending invalid string pointers to your StatusBar. PostMessage() is asynchronous, it returns to the caller immediately, thus you may potentially modify/free the posted string before the StatusBar ever sees the pointer that you gave it.

    You need to either: