From a13b2a248e5091ddf21e3c5ac08c9ddf0b940b5b Mon Sep 17 00:00:00 2001 From: Murray Read Date: Wed, 8 Feb 2012 14:16:27 +0000 Subject: Avoiding early deleteLater in Symbian with better loopLevel tracking There have been a number of app crashes where deleteLater has been triggering too early, causing an object to be deleted before it has been finished with. This was happening when deleteLater was issued then Symbian's active scheduler loop was nested. Qt keeps track of loop nesting level to implement deleteLater correctly, but it was only tracking the event loop in processEvents and QEventLoop correctly. The wakeup and timer active objects were assuming they were always run from processEvents and its round robin active scheduler and were adjusting the loop level to account for this. However if they happened to run in another event loop, eg the active scheduler, the loop level adjustment meant that it looked like the event loop was re-running at the same level, which allowed deleteLater to act. The fix is to mark active objects as being run from the RR scheduler, then the wakeup and timer active objects can be tested to see which type of scheduler they are actually running in. With this knowledge, the correct loop level adjustment can be made, and deleteLater runs at the correct time. Task-number: ou1cimx1#947013 Change-Id: Id05cd63ad10e100ea807cc276844aaa36c614351 Reviewed-by: Gareth Stockwell Reviewed-by: Shane Kearns --- src/corelib/kernel/qeventdispatcher_symbian.cpp | 52 +++++++---- src/corelib/kernel/qeventdispatcher_symbian_p.h | 4 +- src/s60installs/bwins/QtCoreu.def | 6 +- src/s60installs/eabi/QtCoreu.def | 6 +- tests/auto/qapplication/tst_qapplication.cpp | 112 ++++++++++++++++++++++++ 5 files changed, 159 insertions(+), 21 deletions(-) diff --git a/src/corelib/kernel/qeventdispatcher_symbian.cpp b/src/corelib/kernel/qeventdispatcher_symbian.cpp index 4930a16..5cc38e6 100644 --- a/src/corelib/kernel/qeventdispatcher_symbian.cpp +++ b/src/corelib/kernel/qeventdispatcher_symbian.cpp @@ -245,7 +245,7 @@ void QWakeUpActiveObject::RunL() { iStatus = KRequestPending; SetActive(); - QT_TRYCATCH_LEAVING(m_dispatcher->wakeUpWasCalled()); + QT_TRYCATCH_LEAVING(m_dispatcher->wakeUpWasCalled(this)); } QTimerActiveObject::QTimerActiveObject(QEventDispatcherSymbian *dispatcher, SymbianTimerInfo *timerInfo) @@ -349,7 +349,7 @@ void QTimerActiveObject::Run() m_timerInfo->msLeft = m_timerInfo->interval; StartTimer(); - m_timerInfo->dispatcher->timerFired(m_timerInfo->timerId); + m_timerInfo->dispatcher->timerFired(m_timerInfo->timerId, this); } else { // However, we only complete zero timers after the event has finished, // in order to prevent busy looping when doing nested loops. @@ -357,7 +357,7 @@ void QTimerActiveObject::Run() // Keep the refpointer around in order to avoid deletion until the end of this function. SymbianTimerInfoPtr timerInfoPtr(m_timerInfo); - m_timerInfo->dispatcher->timerFired(m_timerInfo->timerId); + m_timerInfo->dispatcher->timerFired(m_timerInfo->timerId, this); iStatus = KRequestPending; SetActive(); @@ -767,6 +767,7 @@ public: }; static RunResult RunMarkedIfReady(TInt &runPriority, TInt minimumPriority, QEventDispatcherSymbian *dispatcher); static bool UseRRActiveScheduler(); + static bool TestAndClearActiveObjectRunningInRRScheduler(CActive* ao); private: // active scheduler access kit, for gaining access to the internals of active objects for @@ -786,9 +787,11 @@ private: TPriQueLink iLink; enum TMarks { - ENewObject, // CBase zero initialization sets this, new objects cannot be run in the processEvents in which they are created - ENotRun, // This object has not yet run in the current processEvents call - ERan // This object has run in the current processEvents call + ENewObject, // CBase zero initialization sets this, new objects cannot be run in the processEvents in which they are created + ENotRun, // This object has not yet run in the current processEvents call + ERunningUnchecked, // This object is running in the current processEvents call, as yet unacknowledged by the event dispatcher + ERunningChecked, // This object is running in a processEvents call, the event dispatcher knows which loop level + ERan // This object has run in the current processEvents call }; int iMark; //TAny* iSpare; }; @@ -836,11 +839,12 @@ QtRRActiveScheduler::RunResult QtRRActiveScheduler::RunMarkedIfReady(TInt &runPr if (active->IsActive() && (active->iStatus!=KRequestPending)) { int& mark = dataAccess->iMark; if (mark == CActiveDataAccess::ENotRun && active->Priority()>=minimumPriority) { - mark = CActiveDataAccess::ERan; + mark = CActiveDataAccess::ERunningUnchecked; runPriority = active->Priority(); dataAccess->iStatus.iFlags&=~TRequestStatusAccess::ERequestActiveFlags; int vptr = *(int*)active; // vptr can be used to identify type when debugging leaves TRAP(error, QT_TRYCATCH_LEAVING(active->RunL())); + mark = CActiveDataAccess::ERan; if (error!=KErrNone) error=active->RunError(error); if (error) { @@ -869,6 +873,16 @@ bool QtRRActiveScheduler::UseRRActiveScheduler() return schedulerCompatibilityNumber == NULL; } +bool QtRRActiveScheduler::TestAndClearActiveObjectRunningInRRScheduler(CActive* ao) +{ + CActiveDataAccess *dataAccess = (CActiveDataAccess*)ao; + if (dataAccess->iMark == CActiveDataAccess::ERunningUnchecked) { + dataAccess->iMark = CActiveDataAccess::ERunningChecked; + return true; + } + return false; +} + #ifdef QT_SYMBIAN_PRIORITY_DROP class QIdleDetectorThread { @@ -1166,7 +1180,7 @@ bool QEventDispatcherSymbian::processEvents ( QEventLoop::ProcessEventsFlags fla return handledAnyEvent; } -void QEventDispatcherSymbian::timerFired(int timerId) +void QEventDispatcherSymbian::timerFired(int timerId, QTimerActiveObject *ao) { Q_D(QAbstractEventDispatcher); QHash::iterator i = m_timerList.find(timerId); @@ -1187,9 +1201,13 @@ void QEventDispatcherSymbian::timerFired(int timerId) m_insideTimerEvent = true; QTimerEvent event(timerInfo->timerId); - //undo the added nesting level around RunIfReady, since Qt's event system also nests - Decrementer dec(d->threadData->loopLevel); - QCoreApplication::sendEvent(timerInfo->receiver, &event); + if (QtRRActiveScheduler::TestAndClearActiveObjectRunningInRRScheduler(ao)) { + //undo the added nesting level around RunIfReady, since Qt's event system also nests + Decrementer dec(d->threadData->loopLevel); + QCoreApplication::sendEvent(timerInfo->receiver, &event); + } else { + QCoreApplication::sendEvent(timerInfo->receiver, &event); + } m_insideTimerEvent = oldInsideTimerEventValue; timerInfo->inTimerEvent = false; @@ -1197,7 +1215,7 @@ void QEventDispatcherSymbian::timerFired(int timerId) return; } -void QEventDispatcherSymbian::wakeUpWasCalled() +void QEventDispatcherSymbian::wakeUpWasCalled(QWakeUpActiveObject *ao) { Q_D(QAbstractEventDispatcher); // The reactivation should happen in RunL, right before the call to this function. @@ -1209,9 +1227,13 @@ void QEventDispatcherSymbian::wakeUpWasCalled() // the sendPostedEvents was done, but before the object was ready to be completed // again. This could deadlock the application if there are no other posted events. m_wakeUpDone.fetchAndStoreOrdered(0); - //undo the added nesting level around RunIfReady, since Qt's event system also nests - Decrementer dec(d->threadData->loopLevel); - sendPostedEvents(); + if (QtRRActiveScheduler::TestAndClearActiveObjectRunningInRRScheduler(ao)) { + //undo the added nesting level around RunIfReady, since Qt's event system also nests + Decrementer dec(d->threadData->loopLevel); + sendPostedEvents(); + } else { + sendPostedEvents(); + } } void QEventDispatcherSymbian::interrupt() diff --git a/src/corelib/kernel/qeventdispatcher_symbian_p.h b/src/corelib/kernel/qeventdispatcher_symbian_p.h index c520d12..66a439f 100644 --- a/src/corelib/kernel/qeventdispatcher_symbian_p.h +++ b/src/corelib/kernel/qeventdispatcher_symbian_p.h @@ -248,8 +248,8 @@ public: void startingUp(); void closingDown(); - void timerFired(int timerId); - void wakeUpWasCalled(); + void timerFired(int timerId, QTimerActiveObject *ao); + void wakeUpWasCalled(QWakeUpActiveObject *ao); void reactivateSocketNotifier(QSocketNotifier *notifier); void addDeferredActiveObject(QActiveObject *object); diff --git a/src/s60installs/bwins/QtCoreu.def b/src/s60installs/bwins/QtCoreu.def index cfd2cec..d6058d6 100644 --- a/src/s60installs/bwins/QtCoreu.def +++ b/src/s60installs/bwins/QtCoreu.def @@ -3696,7 +3696,7 @@ EXPORTS ?timerEvent@QObject@@MAEXPAVQTimerEvent@@@Z @ 3695 NONAME ; void QObject::timerEvent(class QTimerEvent *) ?timerEvent@QTimeLine@@MAEXPAVQTimerEvent@@@Z @ 3696 NONAME ; void QTimeLine::timerEvent(class QTimerEvent *) ?timerEvent@QTimer@@MAEXPAVQTimerEvent@@@Z @ 3697 NONAME ; void QTimer::timerEvent(class QTimerEvent *) - ?timerFired@QEventDispatcherSymbian@@QAEXH@Z @ 3698 NONAME ; void QEventDispatcherSymbian::timerFired(int) + ?timerFired@QEventDispatcherSymbian@@QAEXH@Z @ 3698 NONAME ABSENT ; void QEventDispatcherSymbian::timerFired(int) ?timerId@QBasicTimer@@QBEHXZ @ 3699 NONAME ; int QBasicTimer::timerId(void) const ?timerId@QTimer@@QBEHXZ @ 3700 NONAME ; int QTimer::timerId(void) const ?timerId@QTimerEvent@@QBEHXZ @ 3701 NONAME ; int QTimerEvent::timerId(void) const @@ -4202,7 +4202,7 @@ EXPORTS ?wakeOne@QWaitCondition@@QAEXXZ @ 4201 NONAME ; void QWaitCondition::wakeOne(void) ?wakeUp@QEventDispatcherSymbian@@UAEXXZ @ 4202 NONAME ; void QEventDispatcherSymbian::wakeUp(void) ?wakeUp@QEventLoop@@QAEXXZ @ 4203 NONAME ; void QEventLoop::wakeUp(void) - ?wakeUpWasCalled@QEventDispatcherSymbian@@QAEXXZ @ 4204 NONAME ; void QEventDispatcherSymbian::wakeUpWasCalled(void) + ?wakeUpWasCalled@QEventDispatcherSymbian@@QAEXXZ @ 4204 NONAME ABSENT ; void QEventDispatcherSymbian::wakeUpWasCalled(void) ?weekNumber@QDate@@QBEHPAH@Z @ 4205 NONAME ; int QDate::weekNumber(int *) const ?width@QRect@@QBEHXZ @ 4206 NONAME ; int QRect::width(void) const ?width@QRectF@@QBEMXZ @ 4207 NONAME ; float QRectF::width(void) const @@ -4889,4 +4889,6 @@ EXPORTS ?removeActiveConnection@QSymbianSocketManager@@QAEXK@Z @ 4888 NONAME ; void QSymbianSocketManager::removeActiveConnection(unsigned long) ?addActiveConnection@QSymbianSocketManager@@QAEXK@Z @ 4889 NONAME ; void QSymbianSocketManager::addActiveConnection(unsigned long) ?activeObjectError@QEventDispatcherSymbian@@QAEXH@Z @ 4890 NONAME ; void QEventDispatcherSymbian::activeObjectError(int) + ?timerFired@QEventDispatcherSymbian@@QAEXHPAVQTimerActiveObject@@@Z @ 4891 NONAME ; void QEventDispatcherSymbian::timerFired(int, class QTimerActiveObject *) + ?wakeUpWasCalled@QEventDispatcherSymbian@@QAEXPAVQWakeUpActiveObject@@@Z @ 4892 NONAME ; void QEventDispatcherSymbian::wakeUpWasCalled(class QWakeUpActiveObject *) diff --git a/src/s60installs/eabi/QtCoreu.def b/src/s60installs/eabi/QtCoreu.def index 44aa48c..7977492 100644 --- a/src/s60installs/eabi/QtCoreu.def +++ b/src/s60installs/eabi/QtCoreu.def @@ -1280,13 +1280,13 @@ EXPORTS _ZN23QCoreApplicationPrivateD1Ev @ 1279 NONAME _ZN23QCoreApplicationPrivateD2Ev @ 1280 NONAME _ZN23QEventDispatcherSymbian10startingUpEv @ 1281 NONAME - _ZN23QEventDispatcherSymbian10timerFiredEi @ 1282 NONAME + _ZN23QEventDispatcherSymbian10timerFiredEi @ 1282 NONAME ABSENT _ZN23QEventDispatcherSymbian11closingDownEv @ 1283 NONAME _ZN23QEventDispatcherSymbian11socketFiredEP19QSocketActiveObject @ 1284 NONAME ABSENT _ZN23QEventDispatcherSymbian13processEventsE6QFlagsIN10QEventLoop17ProcessEventsFlagEE @ 1285 NONAME _ZN23QEventDispatcherSymbian13registerTimerEiiP7QObject @ 1286 NONAME _ZN23QEventDispatcherSymbian15unregisterTimerEi @ 1287 NONAME - _ZN23QEventDispatcherSymbian15wakeUpWasCalledEv @ 1288 NONAME + _ZN23QEventDispatcherSymbian15wakeUpWasCalledEv @ 1288 NONAME ABSENT _ZN23QEventDispatcherSymbian16hasPendingEventsEv @ 1289 NONAME _ZN23QEventDispatcherSymbian16sendPostedEventsEv @ 1290 NONAME _ZN23QEventDispatcherSymbian16unregisterTimersEP7QObject @ 1291 NONAME @@ -4169,4 +4169,6 @@ EXPORTS _ZN21QSymbianSocketManager22removeActiveConnectionEm @ 4168 NONAME _ZN23QEventDispatcherSymbian17activeObjectErrorEi @ 4169 NONAME _ZNK21QSymbianSocketManager17activeConnectionsEv @ 4170 NONAME + _ZN23QEventDispatcherSymbian10timerFiredEiP18QTimerActiveObject @ 4171 NONAME + _ZN23QEventDispatcherSymbian15wakeUpWasCalledEP19QWakeUpActiveObject @ 4172 NONAME diff --git a/tests/auto/qapplication/tst_qapplication.cpp b/tests/auto/qapplication/tst_qapplication.cpp index 6bc1891..5835fe1 100644 --- a/tests/auto/qapplication/tst_qapplication.cpp +++ b/tests/auto/qapplication/tst_qapplication.cpp @@ -1406,6 +1406,101 @@ public slots: QApplication::sendPostedEvents(0, QEvent::DeferredDelete); QVERIFY(!p); } + +#ifdef Q_OS_SYMBIAN + void deleteLaterAndProcessEventsSymbian() + { + CActiveSchedulerWait *eventLoop = new CActiveSchedulerWait; + currentSymLoop = eventLoop; + + QPointer p = this; + deleteLater(); + + // this will not be deleted, but deleteLater on an object within that loop will work + m_ptr = new QObject; + QMetaObject::invokeMethod(m_ptr, "deleteLater", Qt::QueuedConnection); + QTimer::singleShot(100, this, SLOT(quitSymLoop())); + eventLoop->Start(); + QVERIFY(p); + QVERIFY(!m_ptr); + + // further nesting of symbian event loop still works correctly + m_ptr = new QObject; + QMetaObject::invokeMethod(m_ptr, "deleteLater", Qt::QueuedConnection); + QMetaObject::invokeMethod(this, "extraSymbianNesting", Qt::QueuedConnection); + QTimer::singleShot(100, this, SLOT(invokeCheckMPtr())); // queue the check event to ensure wakeup runs before we check that deleteLater has not happened + QTimer::singleShot(200, this, SLOT(quitSymLoop())); + QTimer::singleShot(300, this, SLOT(invokeQuitSymLoop())); // need to queue a new event to trigger wakeup, since Symbian's scheduler loop exit doesn't generate events on exit + eventLoop->Start(); + QVERIFY(p); + QVERIFY(!m_ptr); + + // trying to delete this object in a deeper eventloop just won't work + QMetaObject::invokeMethod(this, + "processEventsOnly", + Qt::QueuedConnection); + QMetaObject::invokeMethod(this, "quitSymLoop", Qt::QueuedConnection); + eventLoop->Start(); + QVERIFY(p); + QMetaObject::invokeMethod(this, + "processEventsWithDeferredDeletion", + Qt::QueuedConnection); + QMetaObject::invokeMethod(this, "quitSymLoop", Qt::QueuedConnection); + eventLoop->Start(); + QVERIFY(p); + QMetaObject::invokeMethod(this, + "sendPostedEventsWithDeferredDelete", + Qt::QueuedConnection); + QMetaObject::invokeMethod(this, "quitSymLoop", Qt::QueuedConnection); + eventLoop->Start(); + QVERIFY(p); + + // trying to delete it from this eventloop still doesn't work + QApplication::processEvents(); + QVERIFY(p); + + // however, it *will* work with this magic incantation + QApplication::processEvents(QEventLoop::DeferredDeletion); + QVERIFY(!p); + + delete eventLoop; + currentSymLoop = 0; + } + + void quitSymLoop() + { + currentSymLoop->AsyncStop(); + } + + void invokeQuitSymLoop() + { + QMetaObject::invokeMethod(this, "quitSymLoop", Qt::QueuedConnection); + } + + void extraSymbianNesting() + { + CActiveSchedulerWait *old = currentSymLoop; + CActiveSchedulerWait *thisLevel = new CActiveSchedulerWait; + currentSymLoop = thisLevel; + thisLevel->Start(); + currentSymLoop = old; + delete thisLevel; + } + + void checkMPtr() + { + QVERIFY(m_ptr); + } + + void invokeCheckMPtr() + { + QMetaObject::invokeMethod(this, "checkMPtr", Qt::QueuedConnection); + } + +private: + QPointer m_ptr; + CActiveSchedulerWait *currentSymLoop; +#endif }; void tst_QApplication::testDeleteLaterProcessEvents() @@ -1512,6 +1607,23 @@ void tst_QApplication::testDeleteLaterProcessEvents() loop.exec(); QVERIFY(!p); } + +#ifdef Q_OS_SYMBIAN + { + // when the event loop that calls deleteLater() also calls + // processEvents() immediately afterwards, the object should + // not die until the parent loop continues + QApplication app(argc, 0, QApplication::GuiServer); + QEventLoop loop; + EventLoopNester *nester = new EventLoopNester(); + p = nester; + QTimer::singleShot(3000, &loop, SLOT(quit())); + QTimer::singleShot(0, nester, SLOT(deleteLaterAndProcessEventsSymbian())); + + loop.exec(); + QVERIFY(!p); + } +#endif } /* -- cgit v0.12