views:

32

answers:

3

I have a C++/MFC application I need to restructure. The app used to process most of the data on the main thread, therefore blocking the input, and now I want to change it so, that all GUI updates are done through PostMessage.

Unfortunately, I can't seem to find a good source on how to achieve this goal.

Right now I'm thinking of creating a priority queue, protected with critical section, a worker thread (while(true)) that processes this queue, and PostMessage mechanism that sends pointers to data to main thread.

What scares me with this approach is that PostMessage is not guaranteed to arrive at the main thread at all, so, if I understand correctly, there is a chance of memory leak.

The second problem is that another app can send a custom message to my application, and my application might try to dereference the WPARAM or LPARAM as a pointer thereby causing AV.

Does anyone know what are the best practices for such tasks?

The data can be HTML content for web control, or other content for listboxes, dropdowns, etc.

+2  A: 

Use two queues, one for work requests going to the worker thread and one for results going back to the main thread. You can use PostMessage to wake up the main thread and tell it to check the queue, but you won't need any parameters in the message.

Mark Ransom
A: 

I solved a similar problem some time ago. I made a singleton queue to hold the data (or actions) that need to flow from background threads to the UI (main) thread. The queue is protected by a critical section. Background thread would place its data in the queue and post a message. The message does not hold any data, it acts as a simple wake-up call "hey, main thread, look at the queue, there's work for you".

This way, you don't risk leaking any memory or other resources; the queue can be safely destroyed with all the data it contains.

Alex Emelianov
Terrible. You serialize both threads, thereby negating every benefit you gain from using multiple cores while increasing the complexity a great deal.
John Dibling
No, you don't have to serialize the threads. The key point is to hold the queue lock only when you extract the command instance from it, and then release the lock so that your background threads can continue adding to it without waiting. Background threads hold the lock only as long as needed to put a new member in the queue.
Alex Emelianov
@Alex: That's pretty much the definition of serialization. The two threads can't both access the queue at the same time.
John Dibling
John: queue access is fast and atomic. You can swap pointers in a matter of several CPU cycles. The key task is to separate processing on data that's happening in different threads. In our case, no thread is waiting on another thread to finish processing data. Once again, the lock is there only to protect the integrity of queue's data structures.
Alex Emelianov
+2  A: 

Your messages will get there. I'm not sure why you think PostMessage isn't guaranteed to work -- it is. (EDIT: Assuming PostMessage() returns TRUE! Check your return codes!)

You want to avoid using a queue to communicate data between the threads. Any queue that is accessed by both threads will need to be protected. Adding hard locks on both sides will serialize your application.

Instead, create a data structure on the heap using new that contains your data, then tell the other thread "I;ve got data for you, and here it is." The recieving thread then takes ownership of that data pointer, and is responsible for deleteing it. Doing it this way, there are no hard locks.

Now the only trick is figuring out the "tell the other thread" part, but that's easy too.

If you're sending data from the worker thread to the main thread, just use PostMessage():

worker_thread_proc()
{
// ..

  // Create the data object you're going to pass to the MT
  MyData* data = new MyData;
  data->some_value_ = "foo";

  // Pass it:
  PostMessage(main_wnd, WM_YOU_HAVE_DATA, reinterpret_cast<WPARAM>(data), 0);
}

...the main thread processes this, then deletes the data:

MainWnd::OnYouHaveData(WPARAM wp, LPARAM)
{
  std::auto_ptr<MyData> data(reinterpret_cast<MyData*>(wp));
  my_widget->set_text(data->some_value_); // you get the idea
}

If you're worried about external apps's custom messages bumping in to yours, you can get Windows to give you a unique message ID using RegisterWindowsMessage() -- your only challenge here is picking the right name for your message.

If your sending data from the main thread to the worker thread, you can do the same as above, except instead of using PostMessage() to send the data over the wall, you can use either QueueUserAPC() (making sure your worker thead is in an alertable wait state -- read the remarks in the linked docs) or PostThreadMessage().

EDIT:

Per your comments in the OP, now I understand why you're concerned about PostMessage() not working.

Yes, there is a hard limit to the Windows message queue size. By default, there can be only 4,000 messages in the queue. (registry settings can adjust this up to a max of 10,000).

If the queue is full, any call toPostMessage() will fail with an error code. When you check GetLastError() (I don't remember which error code it returns right now) it will be clear that the message queue is full.

Not to sound like a mother hen, but you really need to check your return values from API calls. But beyond that, if you are running in the the message queue ceiling, I'd say your application is broken anyway. When the queue is full, your application won't be able to breathe. The screen won't paint, any processing you do will be stale, and all kinds of bad things happen. If this is the situation you're in, you may need to look at why.

John Dibling
Isn't it possible that WM_QUIT arrives short before worker posts WMU_NEWDATAPOINTER which is then left dangling? EDIT: Oh, probably not if I do the shutdown during the WM_CLOSE.
Madman
Sure, but in that case you can go in to the queue and delete all the pointers.
John Dibling
Moreover, when the application shuts down, Windows will reclaim the memory. You need to be careful with other resources of course, and I'm not advising you to just go ahead and leak away.
John Dibling