Author: gadamopoulos Date: Sun Mar 13 11:22:18 2011 New Revision: 51031
URL: http://svn.reactos.org/svn/reactos?rev=51031&view=rev Log: [win32] - Rafal Harabien: - Always call IntKeyboardInput and co_MsqPostKeyboardMessage while holding the user lock - Simplify cleaning up sent messages See issue #5580 for more details.
Modified: trunk/reactos/subsystems/win32/win32k/ntuser/input.c trunk/reactos/subsystems/win32/win32k/ntuser/message.c trunk/reactos/subsystems/win32/win32k/ntuser/msgqueue.c
Modified: trunk/reactos/subsystems/win32/win32k/ntuser/input.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/subsystems/win32/win32k/ntu... ============================================================================== --- trunk/reactos/subsystems/win32/win32k/ntuser/input.c [iso-8859-1] (original) +++ trunk/reactos/subsystems/win32/win32k/ntuser/input.c [iso-8859-1] Sun Mar 13 11:22:18 2011 @@ -708,6 +708,7 @@
if (ModifierState == 0) { + UserEnterExclusive(); if (fsModifiers == MOD_WIN) IntKeyboardSendWinKeyMsg(); else if (fsModifiers == MOD_ALT) @@ -723,6 +724,7 @@ } co_IntKeyboardSendAltKeyMsg(); } + UserLeave(); continue; }
@@ -730,6 +732,8 @@ } } } + + UserEnterExclusive();
for (;NumKeys;memcpy(&KeyInput, &NextKeyInput, sizeof(KeyInput)), NumKeys--) @@ -860,6 +864,8 @@ */ co_MsqPostKeyboardMessage(msg.message,msg.wParam,msg.lParam); } + + UserLeave(); }
KeyboardEscape: @@ -1243,17 +1249,11 @@ LARGE_INTEGER LargeTickCount; KBDLLHOOKSTRUCT KbdHookData; WORD flags, wVkStripped, wVkL, wVkR, wVk = ki->wVk, vk_hook = ki->wVk; - BOOLEAN Entered = FALSE;
Msg.lParam = 0;
- // Condition may arise when calling MsqPostMessage and waiting for an event. - if (!UserIsEntered()) - { - // Fixme: Not sure ATM if this thread is locked. - UserEnterExclusive(); - Entered = TRUE; - } + // Condition may arise when calling MsqPostMessage and waiting for an event. + ASSERT (UserIsEntered());
wVk = LOBYTE(wVk); Msg.wParam = wVk; @@ -1352,7 +1352,7 @@ { DPRINT1("Kbd msg %d wParam %d lParam 0x%08x dropped by WH_KEYBOARD_LL hook\n", Msg.message, vk_hook, Msg.lParam); - if (Entered) UserLeave(); + return FALSE; }
@@ -1380,7 +1380,7 @@ if (FocusMessageQueue == NULL) { DPRINT("No focus message queue\n"); - if (Entered) UserLeave(); + return FALSE; }
@@ -1400,8 +1400,6 @@ { DPRINT("Invalid focus window handle\n"); } - - if (Entered) UserLeave();
return TRUE; }
Modified: trunk/reactos/subsystems/win32/win32k/ntuser/message.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/subsystems/win32/win32k/ntu... ============================================================================== --- trunk/reactos/subsystems/win32/win32k/ntuser/message.c [iso-8859-1] (original) +++ trunk/reactos/subsystems/win32/win32k/ntuser/message.c [iso-8859-1] Sun Mar 13 11:22:18 2011 @@ -1447,7 +1447,7 @@ if(!(Message = ExAllocatePoolWithTag(NonPagedPool, sizeof(USER_SENT_MESSAGE), TAG_USRMSG))) { DPRINT1("MsqSendMessage(): Not enough memory to allocate a message"); - return STATUS_INSUFFICIENT_RESOURCES; + RETURN( FALSE); }
Message->Msg.hwnd = hWnd; @@ -1459,19 +1459,21 @@ Message->lResult = 0; Message->QS_Flags = 0; Message->SenderQueue = NULL; // mjmartin, you are right! This is null. + IntReferenceMessageQueue(Win32Thread->MessageQueue); Message->CallBackSenderQueue = Win32Thread->MessageQueue; - - IntReferenceMessageQueue(Window->head.pti->MessageQueue); Message->CompletionCallback = CompletionCallback; Message->CompletionCallbackContext = CompletionCallbackContext; - Message->HookMessage = MSQ_NORMAL | MSQ_SENTNOWAIT; + Message->HookMessage = MSQ_NORMAL; // | MSQ_SENTNOWAIT Message->HasPackedLParam = (lParamBufferSize > 0); - + Message->DispatchingListEntry.Flink = NULL; Message->QS_Flags = QS_SENDMESSAGE; + + IntReferenceMessageQueue(Window->head.pti->MessageQueue); + MsqWakeQueue(Window->head.pti->MessageQueue, QS_SENDMESSAGE, FALSE);
InsertTailList(&Window->head.pti->MessageQueue->SentMessagesListHead, &Message->ListEntry); - IntDereferenceMessageQueue(Window->head.pti->MessageQueue); + //IntDereferenceMessageQueue(Window->head.pti->MessageQueue);
RETURN(TRUE);
Modified: trunk/reactos/subsystems/win32/win32k/ntuser/msgqueue.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/subsystems/win32/win32k/ntu... ============================================================================== --- trunk/reactos/subsystems/win32/win32k/ntuser/msgqueue.c [iso-8859-1] (original) +++ trunk/reactos/subsystems/win32/win32k/ntuser/msgqueue.c [iso-8859-1] Sun Mar 13 11:22:18 2011 @@ -279,18 +279,12 @@ MSG Msg; LARGE_INTEGER LargeTickCount; KBDLLHOOKSTRUCT KbdHookData; - BOOLEAN Entered = FALSE;
DPRINT("MsqPostKeyboardMessage(uMsg 0x%x, wParam 0x%x, lParam 0x%x)\n", uMsg, wParam, lParam);
// Condition may arise when calling MsqPostMessage and waiting for an event. - if (!UserIsEntered()) - { - // Fixme: Not sure ATM if this thread is locked. - UserEnterExclusive(); - Entered = TRUE; - } + ASSERT(UserIsEntered());
FocusMessageQueue = IntGetFocusMessageQueue();
@@ -320,14 +314,12 @@ { DPRINT1("Kbd msg %d wParam %d lParam 0x%08x dropped by WH_KEYBOARD_LL hook\n", Msg.message, Msg.wParam, Msg.lParam); - if (Entered) UserLeave(); return; }
if (FocusMessageQueue == NULL) { DPRINT("No focus message queue\n"); - if (Entered) UserLeave(); return; }
@@ -346,7 +338,6 @@ DPRINT("Invalid focus window handle\n"); }
- if (Entered) UserLeave(); return; }
@@ -415,6 +406,42 @@ ExFreeToPagedLookasideList(&MessageLookasideList, Message); }
+VOID FASTCALL +MsqDestroySentMessage(PUSER_MESSAGE_QUEUE MessageQueue, PUSER_SENT_MESSAGE SentMessage) +{ + /* remove the message from the dispatching list if needed */ + if (SentMessage->DispatchingListEntry.Flink != NULL) + { + RemoveEntryList(&SentMessage->DispatchingListEntry); + } + + /* wake the sender's thread */ + if (SentMessage->CompletionEvent != NULL) + { + KeSetEvent(SentMessage->CompletionEvent, IO_NO_INCREMENT, FALSE); + } + + /* dereference message queues */ + IntDereferenceMessageQueue(MessageQueue); + if (SentMessage->SenderQueue) + { + IntDereferenceMessageQueue(SentMessage->SenderQueue); + } + if (SentMessage->CallBackSenderQueue) + { + IntDereferenceMessageQueue(SentMessage->CallBackSenderQueue); + } + + /* free lParam if needed */ + if (SentMessage->HasPackedLParam == TRUE && SentMessage->Msg.lParam) + { + ExFreePool((PVOID)SentMessage->Msg.lParam); + } + + /* free the message */ + ExFreePoolWithTag(SentMessage, TAG_USRMSG); +} + BOOLEAN FASTCALL co_MsqDispatchOneSentMessage(PUSER_MESSAGE_QUEUE MessageQueue) { @@ -478,13 +505,10 @@ RemoveEntryList(&Message->ListEntry);
/* remove the message from the dispatching list if needed, so lock the sender's message queue */ - if (!(Message->HookMessage & MSQ_SENTNOWAIT)) - { - if (Message->DispatchingListEntry.Flink != NULL) - { - /* only remove it from the dispatching list if not already removed by a timeout */ - RemoveEntryList(&Message->DispatchingListEntry); - } + if (Message->DispatchingListEntry.Flink != NULL) + { + RemoveEntryList(&Message->DispatchingListEntry); + Message->DispatchingListEntry.Flink = NULL; } /* still keep the sender's message queue locked, so the sender can't exit the MsqSendMessage() function (if timed out) */ @@ -500,16 +524,11 @@ *Message->Result = Result; }
- if (Message->HasPackedLParam == TRUE) - { - if (Message->Msg.lParam) - ExFreePool((PVOID)Message->Msg.lParam); - } - /* Notify the sender. */ if (Message->CompletionEvent != NULL) { KeSetEvent(Message->CompletionEvent, IO_NO_INCREMENT, FALSE); + Message->CompletionEvent = NULL; /* prevent MsqDestroySentMessage from setting this event again */ }
/* Call the callback if the message was sent with SendMessageCallback */ @@ -522,15 +541,7 @@ Result); }
- /* Only if it is not a no wait message */ - if (!(Message->HookMessage & MSQ_SENTNOWAIT)) - { - IntDereferenceMessageQueue(Message->SenderQueue); - IntDereferenceMessageQueue(MessageQueue); - } - - /* free the message */ - ExFreePoolWithTag(Message, TAG_USRMSG); + MsqDestroySentMessage(MessageQueue, Message);
/* do not hangup on the user if this is reentering */ if (!SaveMsg) pti->pcti->CTI_flags &= ~CTI_INSENDMESSAGE; @@ -560,16 +571,14 @@ { PostedMessage = CONTAINING_RECORD(CurrentEntry, USER_MESSAGE, ListEntry); + /* set CurrentEntry to next before destroying message */ + CurrentEntry = CurrentEntry->Flink; + if (PostedMessage->Msg.hwnd == Window->head.h) { RemoveEntryList(&PostedMessage->ListEntry); ClearMsgBitsMask(MessageQueue, PostedMessage->QS_Flags); MsqDestroyMessage(PostedMessage); - CurrentEntry = MessageQueue->PostedMessagesListHead.Flink; - } - else - { - CurrentEntry = CurrentEntry->Flink; } }
@@ -580,6 +589,9 @@ { SentMessage = CONTAINING_RECORD(CurrentEntry, USER_SENT_MESSAGE, ListEntry); + /* set CurrentEntry to next before destroying message */ + CurrentEntry = CurrentEntry->Flink; + if(SentMessage->Msg.hwnd == Window->head.h) { DPRINT("Notify the sender and remove a message from the queue that had not been dispatched\n"); @@ -587,41 +599,7 @@ RemoveEntryList(&SentMessage->ListEntry); ClearMsgBitsMask(MessageQueue, SentMessage->QS_Flags);
- /* remove the message from the dispatching list if neede */ - if ((!(SentMessage->HookMessage & MSQ_SENTNOWAIT)) - && (SentMessage->DispatchingListEntry.Flink != NULL)) - { - RemoveEntryList(&SentMessage->DispatchingListEntry); - } - - /* wake the sender's thread */ - if (SentMessage->CompletionEvent != NULL) - { - KeSetEvent(SentMessage->CompletionEvent, IO_NO_INCREMENT, FALSE); - } - - if (SentMessage->HasPackedLParam == TRUE) - { - if (SentMessage->Msg.lParam) - ExFreePool((PVOID)SentMessage->Msg.lParam); - } - - /* Only if it is not a no wait message */ - if (!(SentMessage->HookMessage & MSQ_SENTNOWAIT)) - { - /* dereference our and the sender's message queue */ - IntDereferenceMessageQueue(MessageQueue); - IntDereferenceMessageQueue(SentMessage->SenderQueue); - } - - /* free the message */ - ExFreePoolWithTag(SentMessage, TAG_USRMSG); - - CurrentEntry = MessageQueue->SentMessagesListHead.Flink; - } - else - { - CurrentEntry = CurrentEntry->Flink; + MsqDestroySentMessage(MessageQueue, SentMessage); } } } @@ -655,7 +633,7 @@
Timeout.QuadPart = (LONGLONG) uTimeout * (LONGLONG) -10000;
- /* FIXME - increase reference counter of sender's message queue here */ + /* FIXME - increase reference counter of sender's message queue here - isn't it done? */
Message->Msg.hwnd = Wnd; Message->Msg.message = Msg; @@ -665,9 +643,9 @@ Message->Result = &Result; Message->lResult = 0; Message->QS_Flags = 0; + IntReferenceMessageQueue(ThreadQueue); Message->SenderQueue = ThreadQueue; Message->CallBackSenderQueue = NULL; - IntReferenceMessageQueue(ThreadQueue); Message->CompletionCallback = NULL; Message->CompletionCallbackContext = 0; Message->HookMessage = HookMessage; @@ -1436,35 +1414,7 @@
DPRINT("Notify the sender and remove a message from the queue that had not been dispatched\n");
- /* remove the message from the dispatching list if needed */ - if ((!(CurrentSentMessage->HookMessage & MSQ_SENTNOWAIT)) - && (CurrentSentMessage->DispatchingListEntry.Flink != NULL)) - { - RemoveEntryList(&CurrentSentMessage->DispatchingListEntry); - } - - /* wake the sender's thread */ - if (CurrentSentMessage->CompletionEvent != NULL) - { - KeSetEvent(CurrentSentMessage->CompletionEvent, IO_NO_INCREMENT, FALSE); - } - - if (CurrentSentMessage->HasPackedLParam == TRUE) - { - if (CurrentSentMessage->Msg.lParam) - ExFreePool((PVOID)CurrentSentMessage->Msg.lParam); - } - - /* Only if it is not a no wait message */ - if (!(CurrentSentMessage->HookMessage & MSQ_SENTNOWAIT)) - { - /* dereference our and the sender's message queue */ - IntDereferenceMessageQueue(MessageQueue); - IntDereferenceMessageQueue(CurrentSentMessage->SenderQueue); - } - - /* free the message */ - ExFreePool(CurrentSentMessage); + MsqDestroySentMessage(MessageQueue, CurrentSentMessage); }
/* notify senders of dispatching messages. This needs to be cleaned up if e.g. @@ -1475,36 +1425,9 @@ CurrentSentMessage = CONTAINING_RECORD(CurrentEntry, USER_SENT_MESSAGE, ListEntry);
- /* remove the message from the dispatching list */ - if(CurrentSentMessage->DispatchingListEntry.Flink != NULL) - { - RemoveEntryList(&CurrentSentMessage->DispatchingListEntry); - } - DPRINT("Notify the sender, the thread has been terminated while dispatching a message!\n");
- /* wake the sender's thread */ - if (CurrentSentMessage->CompletionEvent != NULL) - { - KeSetEvent(CurrentSentMessage->CompletionEvent, IO_NO_INCREMENT, FALSE); - } - - if (CurrentSentMessage->HasPackedLParam == TRUE) - { - if (CurrentSentMessage->Msg.lParam) - ExFreePool((PVOID)CurrentSentMessage->Msg.lParam); - } - - /* Only if it is not a no wait message */ - if (!(CurrentSentMessage->HookMessage & MSQ_SENTNOWAIT)) - { - /* dereference our and the sender's message queue */ - IntDereferenceMessageQueue(MessageQueue); - IntDereferenceMessageQueue(CurrentSentMessage->SenderQueue); - } - - /* free the message */ - ExFreePool(CurrentSentMessage); + MsqDestroySentMessage(MessageQueue, CurrentSentMessage); }
/* tell other threads not to bother returning any info to us */ @@ -1515,6 +1438,7 @@ DispatchingListEntry); CurrentSentMessage->CompletionEvent = NULL; CurrentSentMessage->Result = NULL; + CurrentSentMessage->DispatchingListEntry.Flink = NULL; // yeah!
/* do NOT dereference our message queue as it might get attempted to be locked later */