Modality, part 4: The importance of setting the correct owner for modal UI


If you decide to display some modal UI, it is important that you set the correct owner for that UI. If you fail to heed this rule, you will find yourself chasing some very strange bugs.

Let’s return to our scratch program and intentionally introduce a bug related to incorrect owner windows, so that we can see the consequences.

void OnChar(HWND hwnd, TCHAR ch, int cRepeat)
{
  switch (ch) {
  case ' ':
    // Wrong!
    MessageBox(NULL, TEXT("Message"), TEXT("Title"), MB_OK);
    if (!IsWindow(hwnd)) MessageBeep(-1);
    break;
  }
}

// Add to WndProc
    HANDLE_MSG(hwnd, WM_CHAR, OnChar);

Run this program, press the space bar, and instead of dismissing the message box, click the “X” button in the corner of the main window. Notice that you get a beep before the program exits.

What happened?

The beep is coming from our call to the MessageBeep function, which in turn is telling us that our window handle is no longer valid. In a real program which kept its state in per-window instance variables (instead of in globals like we do), you would more likely crash because all the instance variables would have gone away when the window was destroyed. In this case, the window was destroyed while inside a nested modal loop. As a result, when control returned to the caller, it is now a method running inside an object that has been destroyed. Any access to an instance variable is going to access memory that was already freed, resulting in memory corruption or an outright crash.

Here’s an explanation in a call stack diagram:

 WinMain
  DispatchMessage(hwnd, WM_CHAR)
   OnChar
    MessageBox(NULL)
     ... modal dialog loop ...
     DispatchMessage(hwnd, WM_CLOSE)
      DestroyWindow(hwnd)
       WndProc(WM_DESTROY)
        ... clean up the window ...

When you clean up the window, you typically destroy all the data structures associated with the window. But notice that you are freeing data structures that are still being used by the OnChar handler deeper in the stack. Eventually, control unwinds back to the OnChar, which is now running with an invalid instance pointer. (If you believe in C++ objects, you would find that its “this” pointer has gone invalid.)

This was caused by failing to set the correct owner for the modal MessageBox call, allowing the user to interact with the frame window at a time when the frame window isn’t expecting to have its state changed.

Even more problematic, the user can switch back to the frame window and hit the space bar again. The result: Another message box. Repeat another time and you end up with a stack that looks like this:

 WinMain
  DispatchMessage(hwnd, WM_CHAR)
   OnChar
    MessageBox(NULL)
     ... modal dialog loop ...
     DispatchMessage(hwnd, WM_CHAR)
       OnChar
	MessageBox(NULL)
	 ... modal dialog loop ...
	 DispatchMessage(hwnd, WM_CHAR)
	   OnChar
	    MessageBox(NULL)
	     ... modal dialog loop ...

There are now four top-level windows, all active. If the user dismisses them in any order other than the reverse order in which they were created, you’re going to have a problem on your hands. For example, if the user dismisses the second message box first, the part of the stack corresponding to that nesting level will end up returning to a destroyed window when the third message box is finally dismissed.

The fix is simple, and we’ll pick up there next time.

Comments (19)
  1. Ben Cooke says:

    This reminds me of a problem I had a while back, and that I’ve seen in many other programs. I’m not sure if it’s related, though.

    Most applications which show icons in the little recessed area of the taskbar with the clock in it will produce a popup menu if the user right-clicks the icon. In several applications, including the only one I’ve ever wrote which had this kind of interface, the user could only dismiss the menu by selecting an item from it or by clicking on the main application window.

    My guess as to why this was had to do with the window having the wrong parent HWND, so the messages about loss of focus and so on were going to the wrong place. However, I never did get around to finding a solution because the project was terminated not long afterward.

    So my question is this: what is the correct way to launch a popup menu from a notification icon? Should its parent be the taskbar window?

    As I say, mine is not the only application I’ve seen which had this problem. I even saw MSN Messenger do it once, although I was unable to recreate it later so perhaps that was just caused by strange circumstances.

  2. Ben: I’ve had that problem too with my applications. The way I "solved" it was by having a "<no action>" menu item which people could click to get the thing to go away, which was not a very good solution. I’m not sure what I was doing wrong either, but I think I had the parent window set correctly.

  3. rrrr says:

    This is the solution. There is a knowledge base entry to this issue.

    SetForegroundWindow(hWnd);

    TrackPopupMenu(..);

    PostMessage(hWnd, WM_NULL, 0, 0);

  4. Matt Ellis says:

    Ben: That problem is described, and a solution/workaround is given in this knowledge base article http://support.microsoft.com/kb/q135788/

    While we’re on the subject, it would be nice to know why this is "behaviour is by design", and what is actually going on. It does sound as though it could be related…

  5. Raymond Chen says:

    I used to know why, once, twelve years ago, for about two days, and then I forgot because it turns out the reason isn’t important.

  6. Shog9 says:

    Regarding the reason for TrackPopupMenu() problems: Sounds like you’re hitting a restriction in how windows can capture input. AFAIK, only the foreground window can capture mouse input, so if you’re app isn’t then you’ll lose all those clicks outside of the menu.

  7. "I used to know why, once, twelve years ago, for about two days, and then I forgot because it turns out the reason isn’t important."

    I would be happy beyond belief if I could just remember that I remembered something once and how long I remembered it for just 6 years ago.

    James

  8. "I used to know why, once, twelve years ago, for about two days, and then I forgot because it turns out the reason isn’t important."

    That was cold…

  9. Josh Koppang says:

    It seems Raymond may have been watching too much American Idol. Simon is wearing off on him…

    Of course, he happens to be my favorite judge. He tells it how it is.

    Maybe we should have a Microsoft Idol competition :)

  10. Jon Potter says:

    Raymond doesn’t like people asking questions. HE has the knowledge and HE dishes it out as HE sees fit :)

  11. Moi says:

    I think this sums up my whole problem with Windows programing. I just don’t agree that DestroyWindow should call the destructor of the object.

  12. memory says:

    Things that’s not necessary to know is frustrating to look up a second time.

  13. Ben Hutchings says:

    Moi: It doesn’t. Windows doesn’t know anything about your wrapper objects.

  14. Which window are you supposed to use?

  15. Moi says:

    Ben – then what does "its "this" pointer has gone invalid" mean?

  16. Good Point says:

    "Raymond doesn’t like people asking questions. HE has the knowledge and HE dishes it out as HE sees fit :)"

    Astute observation. See the discussion on 64 bit Windows for another example.

    Actually, pretty much every person I’ve met from Microsoft has this attitude.

    Maybe this is the reason why the problems with Windows APIs are not documented worth a darn in MSDN. Before using any, always search Google, you’ll find the shortcomings (See the recent discussion on MsgWaitForMultipleObjects).

  17. Following up on the questions asked at the end.

  18. Answer to previous exercise.

  19. Disable the owner window, but make sure it’s really the owner.

Comments are closed.