From e0d80167c18b5a71acf4dac59f3e6f37292fa397 Mon Sep 17 00:00:00 2001 From: Alex Trotsenko Date: Wed, 18 Mar 2020 20:04:47 +0200 Subject: QEventDispatcherWin32: fix posted events delivering To avoid livelocks, posted events should be delivered when all pending messages have been processed, and the thread's message queue becomes empty. Although the logic of the previous patch is correct, it turned out that determining the moment when the message queue is really empty is not so simple. It is worth noting that the GetQueueStatus function sometimes reports unexpected results due to internal filtering and processing. Indeed, Windows docs say that "the return value from GetQueueStatus should be considered only a hint as to whether GetMessage or PeekMessage should be called". Thus, we cannot rely on GetQueueStatus in unambiguous logic inside the qt_GetMessageHook. To solve the problem, this patch introduces a guard timer which guarantees low priority processing for posted events in foreign loop. The wakeUps flag reset logic has also been changed to provide clearer synchronization of the Qt internal loop. Fixes: QTBUG-82701 Fixes: QTBUG-83151 Change-Id: I33d5001a40d2a4879ef4eb878c09bc1c0616e289 Reviewed-by: Volker Hilsheimer --- src/corelib/kernel/qeventdispatcher_win.cpp | 82 +++++++++++++++++------------ src/corelib/kernel/qeventdispatcher_win_p.h | 1 + 2 files changed, 50 insertions(+), 33 deletions(-) diff --git a/src/corelib/kernel/qeventdispatcher_win.cpp b/src/corelib/kernel/qeventdispatcher_win.cpp index 87623f304a..56b60e67e0 100644 --- a/src/corelib/kernel/qeventdispatcher_win.cpp +++ b/src/corelib/kernel/qeventdispatcher_win.cpp @@ -84,10 +84,8 @@ enum { WM_QT_ACTIVATENOTIFIERS = WM_USER + 2 }; -// WM_QT_SENDPOSTEDEVENTS message parameter enum { - WMWP_QT_TOFOREIGNLOOP = 0, - WMWP_QT_FROMWAKEUP + SendPostedEventsTimerId = ~1u }; class QEventDispatcherWin32Private; @@ -100,8 +98,8 @@ LRESULT QT_WIN_CALLBACK qt_internal_proc(HWND hwnd, UINT message, WPARAM wp, LPA QEventDispatcherWin32Private::QEventDispatcherWin32Private() : threadId(GetCurrentThreadId()), interrupt(false), internalHwnd(0), - getMessageHook(0), wakeUps(0), activateNotifiersPosted(false), - winEventNotifierActivatedEvent(NULL) + getMessageHook(0), sendPostedEventsTimerId(0), wakeUps(0), + activateNotifiersPosted(false), winEventNotifierActivatedEvent(NULL) { } @@ -129,6 +127,18 @@ void WINAPI QT_WIN_CALLBACK qt_fast_timer_proc(uint timerId, uint /*reserved*/, QCoreApplication::postEvent(t->dispatcher, new QTimerEvent(t->timerId)); } +static inline UINT inputQueueMask() +{ + UINT result = QS_ALLEVENTS; + // QTBUG 28513, QTBUG-29097, QTBUG-29435: QS_TOUCH, QS_POINTER became part of + // QS_INPUT in Windows Kit 8. They should not be used when running on pre-Windows 8. +#if WINVER > 0x0601 + if (QOperatingSystemVersion::current() < QOperatingSystemVersion::Windows8) + result &= ~(QS_TOUCH | QS_POINTER); +#endif // WINVER > 0x0601 + return result; +} + LRESULT QT_WIN_CALLBACK qt_internal_proc(HWND hwnd, UINT message, WPARAM wp, LPARAM lp) { if (message == WM_NCCREATE) @@ -240,47 +250,39 @@ LRESULT QT_WIN_CALLBACK qt_internal_proc(HWND hwnd, UINT message, WPARAM wp, LPA case WM_TIMER: Q_ASSERT(d != 0); - d->sendTimerEvent(wp); + if (wp == d->sendPostedEventsTimerId) + q->sendPostedEvents(); + else + d->sendTimerEvent(wp); return 0; case WM_QT_SENDPOSTEDEVENTS: Q_ASSERT(d != 0); // We send posted events manually, if the window procedure was invoked // by the foreign event loop (e.g. from the native modal dialog). - q->sendPostedEvents(); + // Skip sending, if the message queue is not empty. + // sendPostedEventsTimer will deliver posted events later. + static const UINT mask = inputQueueMask(); + if (HIWORD(GetQueueStatus(mask)) == 0) + q->sendPostedEvents(); return 0; } // switch (message) return DefWindowProc(hwnd, message, wp, lp); } -static inline UINT inputQueueMask() -{ - UINT result = QS_ALLEVENTS; - // QTBUG 28513, QTBUG-29097, QTBUG-29435: QS_TOUCH, QS_POINTER became part of - // QS_INPUT in Windows Kit 8. They should not be used when running on pre-Windows 8. -#if WINVER > 0x0601 - if (QOperatingSystemVersion::current() < QOperatingSystemVersion::Windows8) - result &= ~(QS_TOUCH | QS_POINTER); -#endif // WINVER > 0x0601 - return result; -} - LRESULT QT_WIN_CALLBACK qt_GetMessageHook(int code, WPARAM wp, LPARAM lp) { QEventDispatcherWin32 *q = qobject_cast(QAbstractEventDispatcher::instance()); Q_ASSERT(q != 0); QEventDispatcherWin32Private *d = q->d_func(); MSG *msg = reinterpret_cast(lp); - static const UINT mask = inputQueueMask(); - - if (HIWORD(GetQueueStatus(mask)) == 0 && wp == PM_REMOVE) { - // Allow posting WM_QT_SENDPOSTEDEVENTS message. - d->wakeUps.storeRelaxed(0); - if (!(msg->hwnd == d->internalHwnd && msg->message == WM_QT_SENDPOSTEDEVENTS)) { - PostMessage(d->internalHwnd, WM_QT_SENDPOSTEDEVENTS, - WMWP_QT_TOFOREIGNLOOP, 0); - } + + if (msg->hwnd == d->internalHwnd && msg->message == WM_QT_SENDPOSTEDEVENTS + && wp == PM_REMOVE && d->sendPostedEventsTimerId == 0) { + // Start a timer to deliver posted events when the message queue is emptied. + d->sendPostedEventsTimerId = SetTimer(d->internalHwnd, SendPostedEventsTimerId, + USER_TIMER_MINIMUM, NULL); } return d->getMessageHook ? CallNextHookEx(0, code, wp, lp) : 0; } @@ -571,12 +573,15 @@ bool QEventDispatcherWin32::processEvents(QEventLoop::ProcessEventsFlags flags) } if (haveMessage) { if (d->internalHwnd == msg.hwnd && msg.message == WM_QT_SENDPOSTEDEVENTS) { - // Set result to 'true', if the message was sent by wakeUp(). - if (msg.wParam == WMWP_QT_FROMWAKEUP) - retVal = true; + // Set result to 'true' because the message was sent by wakeUp(). + retVal = true; continue; } if (msg.message == WM_TIMER) { + // Skip timer event intended for use inside foreign loop. + if (d->internalHwnd == msg.hwnd && msg.wParam == d->sendPostedEventsTimerId) + continue; + // avoid live-lock by keeping track of the timers we've already sent bool found = false; for (int i = 0; !found && i < processedTimers.count(); ++i) { @@ -964,8 +969,7 @@ void QEventDispatcherWin32::wakeUp() Q_D(QEventDispatcherWin32); if (d->internalHwnd && d->wakeUps.testAndSetAcquire(0, 1)) { // post a WM_QT_SENDPOSTEDEVENTS to this thread if there isn't one already pending - if (!PostMessage(d->internalHwnd, WM_QT_SENDPOSTEDEVENTS, - WMWP_QT_FROMWAKEUP, 0)) + if (!PostMessage(d->internalHwnd, WM_QT_SENDPOSTEDEVENTS, 0, 0)) qErrnoWarning("QEventDispatcherWin32::wakeUp: Failed to post a message"); } } @@ -1007,6 +1011,10 @@ void QEventDispatcherWin32::closingDown() if (d->getMessageHook) UnhookWindowsHookEx(d->getMessageHook); d->getMessageHook = 0; + + if (d->sendPostedEventsTimerId != 0) + KillTimer(d->internalHwnd, d->sendPostedEventsTimerId); + d->sendPostedEventsTimerId = 0; } bool QEventDispatcherWin32::event(QEvent *e) @@ -1048,6 +1056,14 @@ bool QEventDispatcherWin32::event(QEvent *e) void QEventDispatcherWin32::sendPostedEvents() { Q_D(QEventDispatcherWin32); + + if (d->sendPostedEventsTimerId != 0) + KillTimer(d->internalHwnd, d->sendPostedEventsTimerId); + d->sendPostedEventsTimerId = 0; + + // Allow posting WM_QT_SENDPOSTEDEVENTS message. + d->wakeUps.storeRelaxed(0); + QCoreApplicationPrivate::sendPostedEvents(0, 0, d->threadData); } diff --git a/src/corelib/kernel/qeventdispatcher_win_p.h b/src/corelib/kernel/qeventdispatcher_win_p.h index e6620178d8..8f6f05e0a7 100644 --- a/src/corelib/kernel/qeventdispatcher_win_p.h +++ b/src/corelib/kernel/qeventdispatcher_win_p.h @@ -170,6 +170,7 @@ public: HHOOK getMessageHook; // for controlling when to send posted events + UINT_PTR sendPostedEventsTimerId; QAtomicInt wakeUps; // timers -- cgit v1.2.3