C / C++ / MFC

21-Apr-13 15:31

I think my past sins and assumptions are coming home to haunt me. I have a fairly large/complex application that began its past life in another product. Consider it a dialog application with a lot of dialogs. The dialogs can become 3 or more deep. On rare and random occasions, the application will crash with a stack overflow exception - only when leaving a dialog. The exception happens somewhere after EndDialog processes.

Yes, I have checked stack usage - I'm well under the specified amount. The problem is very rare. My debug efforts have led me to the path of suspecting some sort of race condition in message processing or some other undetected data corruption.

The application itself uses your standard windows messages. However, I have a number of worker threads that post messages into the application message queues. My approach is what I need a basic sanity check on

In the InitInstance, I create a worker thread using ::CreateThread() - system timer, whose sole responsibility is to notify active dialogs that a time event has occurred. This thread:

- acquires a pointer to CWnd via AfxGetApp().
- posts the message to the window using PosteMessage()
- there is no stack allocated data passed of any kind whatsoever in these messages.

Based on what I have read about this approach, it's legal and the only appropriate way for a worker thread to send data to the GUI thread. Opinions?

My next question has to do with what I consider to be a possible race condition. Given that my worker thread is firing off messages irrespective of what is going on elsewhere in the application, is it possible for the worker thread to send a message to a window AND have the user press the "Exit" button before the message arrives? If so, I would expect the dialog to be in some sort of tear down state such that some of it's processing is indeterminate. Is my concern reasonable?

Charlie Gilley
<italic>You're going to tell me what I want to know, or I'm going to beat you to death in your own house.

"Where liberty dwells, there is my country." B. Franklin, 1783
“They who can give up essential liberty to obtain a little temporary safety deserve neither liberty nor safety.” BF, 1759

pasztorpisti21-Apr-13 19:28

21-Apr-13 19:28

Imagine the gui thread as a special thread that has a job queue (because it has one). The main loop of the application (in the gui thread) is spinning and gets/processes messages that it retrievees with PeekMessage() or GetMessage() from its per-thread message queue. Putting a message to this message queue with PostMessage() for later processing is OK. Putting a message to the queue with SendMessage() is also OK. The difference between the two is that SendMessage() waits till the end of message processing so you can safely pass even pointers that point to your stack, and you can process the return value immediately. To tell the exact reason of your problem you need to give more info:
Which CWnd? Main wnd? Dialog wnd? What is the scope/lifetime of the CWnd? What is the scope/lifetime of the thread? Which Exit button? The one that terminates the whole application or just one of your dialog boxes?

There are several other things you should consider: MFC itself is not thread safe. If you use for example CWnd retriever methods than you are accessing a window map that is sometimes modified by the gui thread. Maybe your only problem is that you should get a HWND on the gui thread and pass that as a message target to the worker thread when you create it. Another possible problem is that on the main thread you should ask the worker thread to terminate and wait for it to terminate at some point. For example when the main window is destructed.

charlieg22-Apr-13 4:34

22-Apr-13 4:34

All good points. I know about SendMessage / PostMessage, and I've read many articles documenting why you need to be very careful sending messages in mfc. I'll see if I can come up with a description that isn't worse than I've already written Smile | :)

. Note that this basic architecture was inherited, so I make no claim as to its validity. It's an approach, and it has worked. However in writing this out, I think I've discovered one issue...

- The worker thread is a child of the CWinApp application. It exists for the lifetime of the application.

- Window structure: CWinApp -> CFrameWnd -> Main Dialog -> many more dialogs. The frame object serves as the persistent object of the application. There are times when the main dialog may go away, so the frame object maintains application context. Normally, the CFrameWnd will instantiate the Main Dialog.

The frame object is created like so, its instance is saved away for the application:

C++

CFrameWnd*	pFrameWnd = new CFrameWnd;
m_pMainWnd = pFrameWnd;				// This identifies the application's main wnd

- Every dialog created registers with the frame object its instance (using this). This is done so that when a general message is posted to the frame, the frame then echoes it out to all existing dialogs.

- The timer thread posts the timer message indirectly by invoking a public helper method from the frame object: pFrame->BroadcastTimerEvent(); The fact that a worker thread is using an object from the UI side of application CANNOT be a good thing. Comments?

In this BroadcastTimerEvent method, we finally post a message into the message queue:

C++

<pre>Wnd * pWindow;

   pWindow = 	AfxGetApp()->m_pMainWnd;
   if (NULL != pWindow) pWindow->PostMessage(WM_DATA_TIMER);

This effectively queues the message to arrive and be handled by the frame's OnDataTimer handler. Note from above that I mentioned I keep track of all active dialogs. This is done to ease pushing out the timer events to all existing dialogs:

C++

<pre>LRESULT CFrameWnd::OnPanelDataTimer(WPARAM wParam, LPARAM lParam)
{
  if (NULL != m_pClientWnd)  // m_pClientWnd is the main dialog
   {
      if (NULL != m_pClientWnd->m_hWnd)
      {
         // Only send events to registered dialogs with legal windows.  Some dialogs exist w/o
         // having yet been completely instantiated.  Send the events in reverse order, as the array
         // list implies a creation/stacking last in the newest order.

	for(SHORT j = m_pDialogsInUse.GetSize() - 1; j >= 0; j--)
	  if (m_pDialogsInUse[j]->m_hWnd != NULL)
             m_pDialogsInUse[j]->PostMessage(WM_DATA_TIMER);	
      }
   }

return 0;
}

So, I finally come back to my race condition issue - the timer thread fires and starts to push messages out as described. The user happens to press the dialog exit button. Standard button, invokes EndDialog() and terminates. Windows now goes about tearing down that dialog. This dialog has been created on the stack, so it's still out there for a brief period of time. This is where I think there might be a problem...

pasztorpisti22-Apr-13 6:46

22-Apr-13 6:46

You described a lot of things but the 2 main things you have to make sure are still the same:
1. From the worker thread you shouldn't use any data structures and mfc calls that are not thread safe. You can provide this by posting messages directly to a HWND, but calling object methods is also OK if you can guarantee thread safety.
2. Make sure that the thread is terminated before the objects it interacts with are destroyed. I recommended sending an exit request signal to the thread and the waiting for it to terminate from the destroy of the main window.
From your comments none of these can be checked.

charlieg22-Apr-13 11:20

22-Apr-13 11:20

Item 2 - got it. But remember, it's not the application that's terminating, it's a single dialog.

Item 1 - you see how I am invoking PostMessage, from the CWnd object. This object has an m_hWnd variable, but how would I post a message using this? Based on what you have said, I'm not thread safe.

I'm tired, so might be a brain fart question....

PJ Arends22-Apr-13 12:00

PJ Arends

22-Apr-13 12:00

charlieg wrote:
Item 1 - you see how I am invoking PostMessage, from the CWnd object. This
object has an m_hWnd variable, but how would I post a message using this? Based
on what you have said, I'm not thread safe.

Do not use CWnd::PostMessage, use the windows API ::PostMessage instead. It allows you to specify the HWND that is to recieve the message.

When you create the worker thread, pass the main window's HWND to it via the LPVOID parameter of the CreateThread function:

CreateThread(NULL, 0, MyWorkerThreadFunc, (LPVOID)m_pMainWnd->GetSafeHwnd(), 0, NULL)

Then from the worker thread you can directly post your message to the main window

DWORD MyWorkerThreadFunc(LPVOID pParam)
{
    HWND MainWnd = (HWND)pParam;
    ...
    ::PostMessage(MainWnd, WM_MYMESSAGE, 0, 0);
    ...
}

Independent ACN Business Owner

Need a new cell phone? We supply most of the major carriers. Telus in Canada. Flash, Verizon, T-Mobile and Sprint in the USA. O₂, talkmobile, tmobile, orange, three, and vodafone in Europe. See my website for details.

Within you lies the power for good - Use it!

charlieg22-Apr-13 11:20

22-Apr-13 11:20

oh, I truly appreciate the comments...

pasztorpisti22-Apr-13 22:46

22-Apr-13 22:46

No worries, but you should still fix the problems I described and post up the modifications for checking.

Stephen Hewitt22-Apr-13 20:48

Stephen Hewitt

22-Apr-13 20:48

Post a stack trace after the problem has occurred.

Steve

charlieg23-Apr-13 14:17

Re: Name Space Vs Include

23-Apr-13 14:17

Steve,

I would if I could. So far, the application crashes in extremely rare circumstances and only in retail. Working hard on the debug version crashing. We shall see. Changes in the works per the comments above.

thanks all

Name Space Vs Include

Bram van Kampen21-Apr-13 14:51

Bram van Kampen

21-Apr-13 14:51

Hi,

I want to learn about Namespaces. I know how includes work, I have used them for many years. When I google the subject, I get explanations about 'include',(which I already know), and a statement that Name Spaces are a Compiler Construct.
How does this 'Compiler Construct' work in terms of the "Compiler's Symbol Table" Compiler Action and "Linker".

Smile | :)

Bram van Kampen

pasztorpisti21-Apr-13 19:40