From 1dcef0480901ec736af70edc80d4b821e0c8ebe5 Mon Sep 17 00:00:00 2001 From: axis Date: Tue, 21 Sep 2010 11:39:32 +0200 Subject: Made posted events part of the round robin queue. That means that posted events will only be sent once per event queue iteration, just like timers. This helps prevent event starvation. RevBy: mread AutoTest: Included Task: QTBUG-13743 --- src/corelib/kernel/qeventdispatcher_symbian.cpp | 6 ++++-- src/corelib/kernel/qeventdispatcher_symbian_p.h | 5 +---- tests/auto/qtimer/tst_qtimer.cpp | 27 +++++++++++++++++++++++++ 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/corelib/kernel/qeventdispatcher_symbian.cpp b/src/corelib/kernel/qeventdispatcher_symbian.cpp index 5cc6ae3..4ac500f 100644 --- a/src/corelib/kernel/qeventdispatcher_symbian.cpp +++ b/src/corelib/kernel/qeventdispatcher_symbian.cpp @@ -178,8 +178,7 @@ void QActiveObject::reactivateAndComplete() } QWakeUpActiveObject::QWakeUpActiveObject(QEventDispatcherSymbian *dispatcher) - : CActive(WAKE_UP_PRIORITY), - m_dispatcher(dispatcher) + : QActiveObject(WAKE_UP_PRIORITY, dispatcher) { CActiveScheduler::Add(this); iStatus = KRequestPending; @@ -201,6 +200,9 @@ void QWakeUpActiveObject::DoCancel() void QWakeUpActiveObject::RunL() { + if (!okToRun()) + return; + iStatus = KRequestPending; SetActive(); QT_TRYCATCH_LEAVING(m_dispatcher->wakeUpWasCalled()); diff --git a/src/corelib/kernel/qeventdispatcher_symbian_p.h b/src/corelib/kernel/qeventdispatcher_symbian_p.h index bc42753..53afebe 100644 --- a/src/corelib/kernel/qeventdispatcher_symbian_p.h +++ b/src/corelib/kernel/qeventdispatcher_symbian_p.h @@ -95,7 +95,7 @@ private: int m_iterationCount; }; -class QWakeUpActiveObject : public CActive +class QWakeUpActiveObject : public QActiveObject { public: QWakeUpActiveObject(QEventDispatcherSymbian *dispatcher); @@ -106,9 +106,6 @@ public: protected: void DoCancel(); void RunL(); - -private: - QEventDispatcherSymbian *m_dispatcher; }; struct SymbianTimerInfo : public QSharedData diff --git a/tests/auto/qtimer/tst_qtimer.cpp b/tests/auto/qtimer/tst_qtimer.cpp index f23d065..102308e 100644 --- a/tests/auto/qtimer/tst_qtimer.cpp +++ b/tests/auto/qtimer/tst_qtimer.cpp @@ -89,6 +89,7 @@ private slots: void recurseOnTimeoutAndStopTimer(); void QTBUG13633_dontBlockEvents(); + void postedEventsShouldNotStarveTimers(); }; class TimerHelper : public QObject @@ -723,5 +724,31 @@ void tst_QTimer::QTBUG13633_dontBlockEvents() QVERIFY(t.total > 2); } +class SlotRepeater : public QObject { + Q_OBJECT +public: + SlotRepeater() {} + +public slots: + void repeatThisSlot() + { + QMetaObject::invokeMethod(this, "repeatThisSlot", Qt::QueuedConnection); + } +}; + +void tst_QTimer::postedEventsShouldNotStarveTimers() +{ + TimerHelper timerHelper; + QTimer timer; + connect(&timer, SIGNAL(timeout()), &timerHelper, SLOT(timeout())); + timer.setInterval(0); + timer.setSingleShot(false); + timer.start(); + SlotRepeater slotRepeater; + slotRepeater.repeatThisSlot(); + QTest::qWait(100); + QVERIFY(timerHelper.count > 5); +} + QTEST_MAIN(tst_QTimer) #include "tst_qtimer.moc" -- cgit v0.12 From af847c5cc080c6b9730c022b29374841a68cf356 Mon Sep 17 00:00:00 2001 From: axis Date: Tue, 21 Sep 2010 13:19:14 +0200 Subject: Marked a test as XFAIL on Symbian. Task: QTBUG-13773 --- tests/auto/qtimer/tst_qtimer.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/auto/qtimer/tst_qtimer.cpp b/tests/auto/qtimer/tst_qtimer.cpp index 102308e..73179fd 100644 --- a/tests/auto/qtimer/tst_qtimer.cpp +++ b/tests/auto/qtimer/tst_qtimer.cpp @@ -719,6 +719,10 @@ void DontBlockEvents::paintEvent() void tst_QTimer::QTBUG13633_dontBlockEvents() { +#ifdef Q_OS_SYMBIAN + QEXPECT_FAIL("", "Expect failure because of QTBUG-13773", Abort); + QVERIFY2(false, "This test hangs on Symbian"); +#endif DontBlockEvents t; QTest::qWait(60); QVERIFY(t.total > 2); -- cgit v0.12 From 5c29d5be5197b265f16c4895776e9be97db845d6 Mon Sep 17 00:00:00 2001 From: axis Date: Tue, 21 Sep 2010 14:16:21 +0200 Subject: Fixed deployment when using shadow builds. --- tests/auto/qapplication/test/test.pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/auto/qapplication/test/test.pro b/tests/auto/qapplication/test/test.pro index 30eb751..2c54c37 100644 --- a/tests/auto/qapplication/test/test.pro +++ b/tests/auto/qapplication/test/test.pro @@ -12,7 +12,7 @@ wince* { } symbian: { - additional.sources = ../desktopsettingsaware/desktopsettingsaware.exe + additional.sources = $$OUT_PWD/../desktopsettingsaware/desktopsettingsaware.exe additional.path = desktopsettingsaware someTest.sources = test.pro someTest.path = test -- cgit v0.12 From 8d8d147f1dd5d2922a4c61c43d378c8784224f13 Mon Sep 17 00:00:00 2001 From: axis Date: Thu, 23 Sep 2010 13:38:04 +0200 Subject: Fixed event starvation on Symbian if timers were constantly recreated If a timer event occurred, and inside its handler a new timer was created that would also immediately occur, the events would execute over and over and starve other events. This happened because the code in Symbian that is supposed to detect when the same event happens more than once does not work if it is always a new timer which executes. The bug was fixed by introducing a state variable which signals whether we are currently inside the handler of a timer event, and then pretending that newly created timers inside this handler have already been executed once, thereby delaying their execution until the next iteration. Because we reset the state variable in processEvents, the behavior will be slightly different if run directly under CActiveScheduler::Start(). There, we have no way of resetting the state on the next iteration, so all timers inside such a handler (even if it recurses into a new CActiveScheduler::Start()) will be delayed by one event iteration. However, this is considered acceptable, since there is no real "iteration count" in the active scheduler; the event will simply be run after the deferred queue is reactivated (which will be immediately if there are no other events). This is the same as what would have happened for a real timer that executed once and then recursed into CActiveScheduler::Start() (it would also be delayed by one iteration). Task: QTBUG-13773 RevBy: mread AutoTest: Passed --- src/corelib/kernel/qeventdispatcher_symbian.cpp | 16 ++++++++++++++++ src/corelib/kernel/qeventdispatcher_symbian_p.h | 1 + tests/auto/qtimer/tst_qtimer.cpp | 4 ---- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/corelib/kernel/qeventdispatcher_symbian.cpp b/src/corelib/kernel/qeventdispatcher_symbian.cpp index 4ac500f..89012d1 100644 --- a/src/corelib/kernel/qeventdispatcher_symbian.cpp +++ b/src/corelib/kernel/qeventdispatcher_symbian.cpp @@ -724,6 +724,7 @@ QEventDispatcherSymbian::QEventDispatcherSymbian(QObject *parent) m_interrupt(false), m_wakeUpDone(0), m_iterationCount(0), + m_insideTimerEvent(false), m_noSocketEvents(false) { #ifdef QT_SYMBIAN_PRIORITY_DROP @@ -774,6 +775,9 @@ bool QEventDispatcherSymbian::processEvents ( QEventLoop::ProcessEventsFlags fla { bool handledAnyEvent = false; bool oldNoSocketEventsValue = m_noSocketEvents; + bool oldInsideTimerEventValue = m_insideTimerEvent; + + m_insideTimerEvent = false; QT_TRY { Q_D(QAbstractEventDispatcher); @@ -864,6 +868,7 @@ bool QEventDispatcherSymbian::processEvents ( QEventLoop::ProcessEventsFlags fla } m_noSocketEvents = oldNoSocketEventsValue; + m_insideTimerEvent = oldInsideTimerEventValue; return handledAnyEvent; } @@ -884,10 +889,13 @@ void QEventDispatcherSymbian::timerFired(int timerId) } timerInfo->inTimerEvent = true; + bool oldInsideTimerEventValue = m_insideTimerEvent; + m_insideTimerEvent = true; QTimerEvent event(timerInfo->timerId); QCoreApplication::sendEvent(timerInfo->receiver, &event); + m_insideTimerEvent = oldInsideTimerEventValue; timerInfo->inTimerEvent = false; return; @@ -1054,6 +1062,14 @@ void QEventDispatcherSymbian::registerTimer ( int timerId, int interval, QObject m_timerList.insert(timerId, timer); timer->timerAO->Start(); + + if (m_insideTimerEvent) + // If we are inside a timer event, we need to prevent event starvation + // by preventing newly created timers from running in the same event processing + // iteration. Do this by calling the okToRun() function to "fake" that we have + // already run once. This will cause the next run to be added to the deferred + // queue instead. + timer->timerAO->okToRun(); } bool QEventDispatcherSymbian::unregisterTimer ( int timerId ) diff --git a/src/corelib/kernel/qeventdispatcher_symbian_p.h b/src/corelib/kernel/qeventdispatcher_symbian_p.h index 53afebe..8e20d56 100644 --- a/src/corelib/kernel/qeventdispatcher_symbian_p.h +++ b/src/corelib/kernel/qeventdispatcher_symbian_p.h @@ -274,6 +274,7 @@ private: QAtomicInt m_wakeUpDone; unsigned char m_iterationCount; + bool m_insideTimerEvent; bool m_noSocketEvents; QList m_deferredSocketEvents; diff --git a/tests/auto/qtimer/tst_qtimer.cpp b/tests/auto/qtimer/tst_qtimer.cpp index 73179fd..102308e 100644 --- a/tests/auto/qtimer/tst_qtimer.cpp +++ b/tests/auto/qtimer/tst_qtimer.cpp @@ -719,10 +719,6 @@ void DontBlockEvents::paintEvent() void tst_QTimer::QTBUG13633_dontBlockEvents() { -#ifdef Q_OS_SYMBIAN - QEXPECT_FAIL("", "Expect failure because of QTBUG-13773", Abort); - QVERIFY2(false, "This test hangs on Symbian"); -#endif DontBlockEvents t; QTest::qWait(60); QVERIFY(t.total > 2); -- cgit v0.12 From ecc7beca87bdfdad2e260f6b9621b77055cf313c Mon Sep 17 00:00:00 2001 From: axis Date: Thu, 23 Sep 2010 14:20:13 +0200 Subject: Made it more clear what the okToRun function does by renaming it. Since it has the side effect of possibly adding the object to the deferred run queue, this name is more appropriate. AutoTest: Passed RevBy: mread --- src/corelib/kernel/qeventdispatcher_symbian.cpp | 22 +++++++++++----------- src/corelib/kernel/qeventdispatcher_symbian_p.h | 2 +- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/corelib/kernel/qeventdispatcher_symbian.cpp b/src/corelib/kernel/qeventdispatcher_symbian.cpp index 89012d1..d8cc344 100644 --- a/src/corelib/kernel/qeventdispatcher_symbian.cpp +++ b/src/corelib/kernel/qeventdispatcher_symbian.cpp @@ -127,9 +127,9 @@ private: * cannot change active objects that we do not own, but the active objects that Qt owns will use * this as a base class with convenience functions. * - * Here is how it works: On every RunL, the deriving class should call okToRun(). This will allow - * exactly one run of the active object, and mark it as such. If it is called again, it will return - * false, and add the object to a queue so it can be run later. + * Here is how it works: On every RunL, the deriving class should call maybeQueueForLater(). + * This will return whether the active object has been queued, or whether it should run immediately. + * Queued objects will run again after other events have been processed. * * The QCompleteDeferredAOs class is a special object that runs after all others, which will * reactivate the objects that were previously not run. @@ -149,7 +149,7 @@ QActiveObject::~QActiveObject() m_dispatcher->removeDeferredActiveObject(this); } -bool QActiveObject::okToRun() +bool QActiveObject::maybeQueueForLater() { Q_ASSERT(!m_hasRunAgain); @@ -157,12 +157,12 @@ bool QActiveObject::okToRun() // First occurrence of this event in this iteration. m_hasAlreadyRun = true; m_iterationCount = m_dispatcher->iterationCount(); - return true; + return false; } else { // The event has already occurred. m_dispatcher->addDeferredActiveObject(this); m_hasRunAgain = true; - return false; + return true; } } @@ -200,7 +200,7 @@ void QWakeUpActiveObject::DoCancel() void QWakeUpActiveObject::RunL() { - if (!okToRun()) + if (maybeQueueForLater()) return; iStatus = KRequestPending; @@ -272,7 +272,7 @@ void QTimerActiveObject::Run() return; } - if (!okToRun()) + if (maybeQueueForLater()) return; if (m_timerInfo->interval > 0) { @@ -632,7 +632,7 @@ void QSocketActiveObject::DoCancel() void QSocketActiveObject::RunL() { - if (!okToRun()) + if (maybeQueueForLater()) return; QT_TRYCATCH_LEAVING(m_dispatcher->socketFired(this)); @@ -1066,10 +1066,10 @@ void QEventDispatcherSymbian::registerTimer ( int timerId, int interval, QObject if (m_insideTimerEvent) // If we are inside a timer event, we need to prevent event starvation // by preventing newly created timers from running in the same event processing - // iteration. Do this by calling the okToRun() function to "fake" that we have + // iteration. Do this by calling the maybeQueueForLater() function to "fake" that we have // already run once. This will cause the next run to be added to the deferred // queue instead. - timer->timerAO->okToRun(); + timer->timerAO->maybeQueueForLater(); } bool QEventDispatcherSymbian::unregisterTimer ( int timerId ) diff --git a/src/corelib/kernel/qeventdispatcher_symbian_p.h b/src/corelib/kernel/qeventdispatcher_symbian_p.h index 8e20d56..1486db5 100644 --- a/src/corelib/kernel/qeventdispatcher_symbian_p.h +++ b/src/corelib/kernel/qeventdispatcher_symbian_p.h @@ -82,7 +82,7 @@ public: QActiveObject(TInt priority, QEventDispatcherSymbian *dispatcher); ~QActiveObject(); - bool okToRun(); + bool maybeQueueForLater(); void reactivateAndComplete(); -- cgit v0.12