The dangers of mixing synchronous and asynchronous state


The window manager distinguishes between synchronous state (the state of the world based on what messages your program has received) and asynchronous state (the actual state of the world this very instant). We saw this earlier when discussing the difference between GetKeyState and GetAsyncKeyState. Here are some other functions and their relationship to the queue state:

Use synchronous state Use asynchronous state
GetActiveWindow GetForegroundWindow
GetMessagePos GetCursorPos
GetMessageTime GetTickCount

If you query the asynchronous state while processing a message, you can find yourself caught in a race condition, because the synchronous state of the system when the message was generated may not match the asynchronous state of the system when you receive it. For example, if the users presses a key, and then moves the mouse, calling GetCursorPos from your keypress handler will tell you were the cursor is right now, which is not the same as where the cursor was when the key was pressed.

Generally speaking, you should use the synchronous state during message handling so that you react to the state of the system at the time the input event took place. Reacting to the asynchronous state of the system introduces race conditions if there is a change to the system state between the time the message was generated and the time the message is processed.

Of the above functions, GetTickCount is the only one I can think of that has a legitimate usage pattern in common use, namely, when creating timing loops. But if you want to know what time it was when a key was pressed, then GetMessageTime is the function to use.

This is all a rather length lead-in for my remarks regarding a comment claiming that there is no practical reason why you can't use GetForegroundWindow to determine which window was the one that had focus when a keyboard message was generated. Well, actually, there is, and it's precisely the race condition I've spent most of this article describing. Suppose the user presses a key and then switches to another program. Now your program gets around to processing the keyboard input, and you call GetForegroundWindow, and instead of getting a window from your application, you get some other window from another program. You then pass that window handle to TranslateAccelerator, the keyboard event matches an entry in the accelerator, and boom, you just sent a random WM_COMMAND message to a program that will interpret it to mean something completely different.

Remember, just because your program has the line

#define IDC_REFRESH    814

doesn't mean that another program can't have the line

#define IDC_DELETEALL  814

Now the user presses F5 and switches from your program to that other program. Your program processes the message, queries the asynchronous foreground state with GetForegroundWindow, and gets that other program's window back. You then translate the accelerator, and TranslateAccelerator posts the WM_COMMAND(814) message to that other program, which interprets it as "delete all".

The great thing about this is that the users will probably blame the other program. "Sometimes, when I use this program, it spontaneously deletes all my items. Stupid program. It's so buggy."

Commenter poenits correctly points out that I failed to take into account the case where the message is posted directly to the dialog. (The dialog manager tries not to put keyboard focus on the dialog itself, but if you play weird games, you can find yourself backed into that situation, such as if you delete all the controls on a dialog!) The fix, however, is not to translate the message directly to the window with keyboard focus, because the window with keyboard focus might belong to a third dialog that you don't want to translate accelerators for. (That other window might have used the other header file which defines message 814 to be IDC_DELETEALL.) Just check for your specific window directly:

if (hwnd1== msg.hwnd || IsChild(hwnd1, msg.hwnd))
    TranslateAccelerator(hwnd1, hAccel, &msg);
else if (hwnd2 == msg.hwnd || IsChild(hwnd2, msg.hwnd))
    TranslateAccelerator(hwnd2, hAccel, &msg);

Think of TranslateAccelerator as MaybePostWM_COMMAND. The first parameter to TranslateAccelerator must be a window you are certain knows how to interpret the WM_COMMAND message that you might end up posting. You know which windows understand your custom WM_COMMAND messages. Pass one of those known windows, not some random unknown window that you calculated from unknown sources.

Passing an unknown window as the first parameter to TranslateAccelerator is like falling for one of those phishing scams. If you get a random piece of email telling you "Hey, call this number and give me your personal information," you're not going to do it. If you really want to contact your bank, you ignore the phone number in the email and just call the number you know and trust to be your bank's service desk. Similarly, you shouldn't be posting your personal messages to some random window you receive. Post it to the known trusted window. Otherwise you're just sending your money to some unknown recipient in Nigeria.

Comments (19)
  1. Alexandre Grigoriev says:

    One example when you just can’t get the synchronous mouse position is when you want to display a context menu in response to the click on your icon in the tray.

  2. Mark says:

    This is one of the things people "just don’t get" about Win32 programming.  Perhaps it would help if APIs were tagged and separated?

  3. Ivo says:

    Isn’t GetFocus the right way to get the focused window? Most likely you care about the focus in your particular thread.

    BTW, the docs for GetFocus mention GetForegroundWindow so maybe that’s why people use it instead.

  4. Karellen says:

    OK, I realise that back when Win32 was created, passing an extra pointer parameter to a function involved something like 3 extra instructions which is clearly a performance hit you didn’t want to incur, but in this day and age is it really too much to ask for an API which does not rely on hidden/opaque global state?

    How about:

    DWORD GetCursorPosEx(LPMSG lpMsg);

    HWND GetForegroundWindow(LPMSG lpMsg);

    If lpMsg is non-NULL, it gets the data (cursor position, foreground window) as was at the time the message was created. If lpMsg is NULL, it gets the async data.

    Less “magic”, more understandable, harder to misuse.

    Global variables/state; just say “no^H^Hsurely there must be a better way”

    [There is not enough information in a MSG structure to uniquely identify the message. Each time a timer ticks, the exact same MSG structure is returned, even though the mouse may very well have been in a different position at each tick. (And what happens if you pass in a message that was generated 5 minutes ago? Does the window manager have to remember all message history so it can produce this information on the off chance somebody asks for it an hour from now?) The window manager remembers the current state. If you want older state, you’ll have to save it yourself. -Raymond]
  5. Bob says:

    Karellen: People would just pass NULL by default anyway.

  6. Duran says:

    Very interesting post.  

    This seems like a very subtle problem.  I’ve definitely coded this brokenly before (and I’ve put a TODO on my list to fix this in my code).  

    This really raises the question, though, of why the window manager/message queue was designed to ever allow this broken pattern.  One could imagine different designs that would disallow this, or have the default case do the right thing.

    Stuff like this is, IMO, one of the main differentiators between OSes (or frameworks in general).  If it’s hard for developers to program your framework properly, you’re going to have a lot of subtly buggy apps.  Users will then have a negative opinion of it.

    I’d love to see a breakdown of “ease of programmability” for various areas across major OSes (Windows, Linux, …).

    [The alternative is… what? Support only synchronous state queries? (What if somebody wants the asynchronous state?) -Raymond]
  7. arnshea says:

    A similar problem exists in Windows Forms when handling control events.

    As far as I can tell the best you can do is to create your own synchronous state.  At least you’re processing with respect to a (mostly) single point in time…

  8. Steve Thresher says:

    Yet another gem! Keep ’em coming Raymond.

  9. Ben Cooke says:

    I guess this is why, when I ctrl+shift+click on a link in Opera, I need to keep the modifiers held down until the new tab has opened or the modifiers are ignored. It catches me out all the time.

  10. Karellen says:

    “There is not enough information in a MSG structure to uniquely identify the message.”

    Oh, Sorry. I assumed that when you called GetMessage(), the out parameter which received a copy of the message data, received a copy of the message data, which specifically included the window handle, {l,w}param, time and cursor position for that message.

    [Post WM_USER twice within the same millisecond. The MSG structures will be identical. And how long a message history should the window manager retain? -Raymond]
  11. Karellen says:

    Oops. Premature submission. Anyway…

    “Each time a timer ticks, the exact same MSG structure is returned”

    Huh? Are you required to pass a pointer to the same MSG structure each time you call GetMessage() or something? I don’t see that documented.

    “Does the window manager have to remember all message history so it can produce this information on the off chance somebody asks for it an hour from now? […] If you want older state, you’ll have to save it yourself”

    Yeah, I was sort of assuming that if you wanted to keep message history around, you’d just do that yourself with your own managed set of MSG structures. That was the point I was trying to make.

    Although, I’m not entirely clear on the point of the synchronous functions. Why not just access the members of the MSG structure directly?

    [Oh, you’re suggesting that the operating system track each MSG structure by its address. “Somebody’s passing me (MSG*)0x00419148, ah the last time I filled in that MSG structure was this message here, now let me go look up whether the ‘A’ key was up or down at that time.” How long does it keep track of all these message pointers before it says, “Sorry, that was too long ago, I no longer have information about the state of the keyboard at that time”? -Raymond]
  12. Karellen: Another reason this will not work well is that the window procedure does not get to see the MSG structure, only a few selected fields from it. So code called from the window procedure (i.e., most everything UI-related in a typical app) would not be able to call your functions anyway.

    If we were to start from scratch and design a windowing system that did not have to be source code compatible with anything in existence, event handlers would probably receive pointers to a much richer event structure than what fits in the few WndProc arguments, including all this kind of synchronous information …

  13. Dave Harris says:

    “There is not enough information in a MSG structure to uniquely identify the message.”

    Even if the window procedure did receive the full MSG structure, and even if that was enough to identify the message, it’s not enough to implement functions like GetKeyState().

    For a notional GetMsgKeyState( MSG *pMsg, int nVirtKey ) you’d need to know the position of every key in the keyboard at the time the MSG was generated. That information is not stored in MSG. Raymond was hinting it could be stored in a history somewhere, with the MSG as an index. An alternative would be to massively increase the size of MSG to include it, and all other state available synchronously. Both approaches have more overhead than the feature is worth.

    [Putting the state in the MSG structure is a non-starter because it prevents new synchronous state from being added in the future. “We want to add a synchronous InSendMessageEx function but, oh, we can’t because there’s no room in the MSG structure for the additional state bits. I guess we can provide only an async version, even though that has race conditions.” -Raymond]
  14. Karellen says:

    "Oh, you’re suggesting that the operating system track each MSG structure by its address."

    Sorry, I’m having a "understanding the prerequisites" failure, and explaining myself really badly.

    The "understanding the prerequisites" failure was (as HEnning Makholm poitned out) me forgetting that the window procedure does not receive the lpMsg parameter passed to DispatchMessage(), but instead just a few select fields from it. So it doesn’t have all the information that I thought it did. (Why not?)

    As for explaining myself, I was trying to suggest that the OS not track MSG structures at all. Instead of hanging on to *more* data in "magic" opaque global state somewhere, I was trying to suggest that it become more transparent and do *less*. i.e. once an application has retrieved a message via GetMessage() or the window procedure, then it should be able to keep track of all the state associated with the message itself.

    Don’t worry. I’m just smart enough to know when I’m making a fool of myself. I’ll sit the rest of this thread out. My apologies.

  15. MP says:

    ” The great thing about this is that the users will probably blame the other program. “Sometimes, when I use this program, it spontaneously deletes all my items. Stupid program. It’s so buggy.” “

    It seems to me, as another poster alluded to previously, that it’s a bit broken for the API to allow nonsense messages to be inadvertently sent to some other process’s window. I imagine at this point it would be a backwards compatibility nightmare but wouldn’t it be better to require the programmer to be more explicit about their intentions to message a different process?

    [In 1983, the philosophy was that programmers were to be trusted to know what they were doing. -Raymond]
  16. Yuhong Bao says:

    What about GetKeyboardState? Sync or async?

    [The docs on GetKeyboardState appear to be sufficiently clear. The table above was intended to be illustrative, not comprehensive. I apologize for not making that clear. -Raymond]
  17. Ray Trent says:

    What about GetKeyboardState? Sync or async?

    Let me google that for you: http://lmgtfy.com/?q=GetKeyboardState&l=1

    (sync)

  18. Yuhong Bao says:

    "Let me google that for you: http://lmgtfy.com/?q=GetKeyboardState&l=1"

    That animation is so funny!

  19. Anonymous Coward says:

    Thanks Raymond for giving this issue a bit of a higher profile. There are still a lot of programs doing this wrong.

Comments are closed.