1

As in title I want to add/remove items to a class derived from the WTL CListViewCtrl class from worker threads, but always get "Unhandled exception thrown: read access violation."

I tried Win32 API PostMessage and SendMessage but once the worker thread touches the HWND of CListViewCtrl I get the same exception.

// CListCtrl member function, calling from worker thread
HWND GetHwnd()
{
    return hwndListCtrl;       // exception here
}

I tried this SafeQueue but once worker thread touches the mutex or queue then exception again.

// SafeQueue is member variable in CListViewCtrl, created in GUI thread
SafeQueue<T> m_SafeQueue;
. . .
// member function in SafeQueue class, calling from worker thread
void enqueue(T t)
{
    std::lock_guard<std::mutex> lock(m);  // exception here
    q->push(t);
}

I tried to create the mutex and queue with new and HeapAlloc/LocalAlloc but same exception again.

I tried Win32 API CreateMutex but no luck, same exception when accessing mutex handle from worker thread.

It works fine when I add items from the GUI thread.

Only way it works from worker threads if I declare HWND or mutex and queue as static/global but I would avoid this since I want to use more than one instance from this listcontrol and I prefer any more elegant way than global variable.

I want to make this class reusable since I want to use it many times with a few modifications (more columns, different colors).

I appreciate any help and idea how I can make this work.

Environment: VS2015 Community, WTL/C++ and Win10 Pro 64bit

I found the problem that causes access violation exception: I declared ThreadProc callback function as static member function in CListViewCtrl class.

// DO NOT USE
// in CListViewCtrl
**static** DWORD WINAPI ThreadProc(LPVOID lp)
{
. . .
}

LRESULT OnStartWorkerThread(WORD /*wNotifyCode*/, WORD /*wID*/, HWND . ..)
{
    DWORD dw;
    ::CreateThread(NULL, 0, this->ThreadProc, NULL, 0, &dw);
}

A working solution:

class CListViewCtrl ...
{
    // thread-safe queue to store listctrl items to be added later in GUI thread
    SafeQueue<CListCtrlItem<nCols> > m_SafeQueue;  

    // thread ID of the thread in which listctrl was created, saved in OnCreate
    DWORD m_dwGuiTid;

    // . . .

Check if SafeAddItem function called from GUI or any other threads

    BOOL InvokeRequired()
    {
        if (m_GuiTid == ::GetCurrentThreadId())
            return false;

        return true;
    }

    // ...

SafeAddItem member function can be called from GUI and worker threads

    void SafeAddItem(CListCtrlItem<nCols> item)
    {
        if (!InvokeRequired())
        {
            // we are in GUI thread so just add listctrl item "normal" way
            AddItem(item);
            return;
        }

     // we are in other thread so enqueue listctrl item and post a message to GUI           
        m_SafeQueue.Enqueue(item);
        ::PostMessage(m_hWnd, WM_ADD_ITEM, 0, 0);
     }
    // . . .

Message handler of PostMessage, we are in GUI thread

    LRESULT OnAddItem(UINT /*uMsg*/, WPARAM /*wParam*/, LPARAM lParam, BOOL& bHandled)
    {
        CListCtrlItem<nCols> item;
        while (!m_SafeQueue.Empty())
        {
            item = m_SafeQueue.Dequeue();
            // we are in GUI thread so we can add list ctrl items normal way
            AddItem(item);
        }
        return 1;
    }
    // ...
}

And now we can add listctrl items from any threads this way. I pass this pointer to ThreadProc in _beginthreadex

m_ListCtrl.SafeAddItem(item);
Community
  • 1
  • 1
mrherkar
  • 81
  • 1
  • 7

2 Answers2

1

The question appears to be not really about UI updates from worker thread, but about proper use of worker threads per se.

There is sufficient amount of comments about dangers of doing UI updates: they are all about potential deadlock problem. Most of the updates involve sending a message, which is a blocking API call. While you do the update from worker thread and the calling thread is blocked, any attempt from the handler in the UI to synchronize or otherwise collaboratively work with the worker may result in a deadlock. The only way around this is to prepare update in the worker thread and signal the UI thread (including by posting a message instead of sending it, in terms of SendMessage, PostMessage API) to take over and complete the updates from UI thread.

Back to original problem: you seem to be having a problem with a static thread procedure. The fourth argument in the CreateThread call is:

lpParameter [in, optional]

A pointer to a variable to be passed to the thread.

You have it NULL and you are typically to use it to pass this value to your thread procedure callback. This way you can pass execution back from static function to your class instance:

DWORD CFoo::ThreadProc()
{
    // ThreadProc with proper "this" initialization
    // HWND h = GetHwnd()...
}
DWORD WINAPI ThreadProc(LPVOID pvParameter)
{
    return ((CFoo*) pvParameter)->ThreadProc();
}
LRESULT CFoo::OnStartWorkerThread(WORD /*wNotifyCode*/, WORD /*wID*/, HWND ...)
{
    DWORD dw;
    ::CreateThread(NULL, 0, this->ThreadProc, (LPVOID) this, 0, &dw);
}

Also note that you are not supposed to use CreateThread directly: you have _beginthreadex and AtlCreateThread (related question).

Community
  • 1
  • 1
Roman R.
  • 68,205
  • 6
  • 94
  • 158
  • Thank you for your suggestions. I know that SendMessage is a blocking call, so I wanted to use PostMessage instead, but since this was a very strange problem I tried every solutions to get it work. I updated my question with a working solution. I use that SafeQueue to temporary store listctrl items as it would be more complicated to send pointers across thread in PostMessage wParam or lParam. – mrherkar Feb 17 '17 at 04:28
0

In Windows you should never directly modify a GUI control via a worker thread. In the .NET world if we want to update a control via a worker thread we have to do a platform invoke on a Delegate which basically performs a context switch.

You have a similar problem in WIN32.

There is an excellent article on this subject I will call your attention to. It also discusses various safe workarounds: https://www.codeproject.com/Articles/552/Using-Worker-Threads

Worker threads and the GUI II: Don't touch the GUI

"That's right. A worker thread must not touch a GUI object. This means that you should not query the state of a control, add something to a list box, set the state of a control, etc.

Why?

Because you can get into a serious deadlock situation. A classic example was posted on one of the discussion boards, and it described something that had happened to me last year. The situation is this: you start a thread, and then decide to wait for the thread to complete. Meanwhile, the thread does something apparently innocuous, such as add something to a list box, or, in the example that was posted, calls FindWindow. In both cases, the process came to a screeching halt, as all threads deadlocked."

Dave S
  • 973
  • 9
  • 17
  • thank you for your reply. I know that I shouldn't access GUI objects from other thread, but I couldn't access even the mutex handle which is important for sync between threads. And accessing data members from other thread shouldn't cause access violation exception just only strange behaviour. I had a feeling this is strange problem and it looks I found the problem: I declared working thread callback function as static member function in CListViewCtrl. And static member function can access only static data member. – mrherkar Feb 16 '17 at 09:35
  • Thanks for the clarification. From your discription - "I want to add/remove items to a class derived from the WTL CListViewCtrl class from worker threads ... I tried Win32 API PostMessage and SendMessage but once the worker thread touches the HWND of CListViewCtrl I get the same exception ... It works fine when I add items from the GUI thread." It sounded like you were trying toaccess the ListView directly from a worker thread. – Dave S Feb 16 '17 at 09:55
  • Sorry if it was misunderstanding. First I tried to use PostMessage but its first parameter is the target window HWND but I couldn't read/get this HWND from the worker thread as it caused access violation exception. That sounded very strange for me as this shouldn't cause exception. Now it works as it should with a non-member non-static ThreadProc function. Thank you – mrherkar Feb 16 '17 at 10:05
  • Thanks for explaining your solution. – Dave S Feb 16 '17 at 10:30