Replace try/catch blocks in favour of destructors in the event loop.

This has two direct benefits:
1) compiles regardless of -fno-exceptions: no need for #ifndef
QT_NO_EXCEPTIONS or QT_TRY/QT_CATCH

2) no QT_RETHROW either, which means the backtrace of an application
crashing due to an uncaught exception will include the actual throw
point.

Change-Id: I18e5500e121bfa81431ef16699df96d962794f0e
Reviewed-on: http://codereview.qt.nokia.com/663
Reviewed-by: Qt Sanity Bot <qt_sanity_bot@ovi.com>
Reviewed-by: Olivier Goffart <olivier.goffart@nokia.com>

diff --git a/src/corelib/kernel/qcoreapplication.cpp b/src/corelib/kernel/qcoreapplication.cpp
index 5f11b10..2458f96 100644 (file)
--- a/src/corelib/kernel/qcoreapplication.cpp
+++ b/src/corelib/kernel/qcoreapplication.cpp
@@ -816,7 +816,16 @@ bool QCoreApplication::notifyInternal(QObject *receiver, QEvent *event)
     // call overhead.
     QObjectPrivate *d = receiver->d_func();
     QThreadData *threadData = d->threadData;
-    ++threadData->loopLevel;
+
+    // Exception-safety without try/catch
+    struct Incrementer {
+        int &variable;
+        inline Incrementer(int &variable) : variable(variable)
+        { ++variable; }
+        inline ~Incrementer()
+        { --variable; }
+    };
+    Incrementer inc(threadData->loopLevel);
 
 #ifdef QT_JAMBI_BUILD
     int deleteWatch = 0;
@@ -827,12 +836,7 @@ bool QCoreApplication::notifyInternal(QObject *receiver, QEvent *event)
 #endif
 
     bool returnValue;
-    QT_TRY {
-        returnValue = notify(receiver, event);
-    } QT_CATCH (...) {
-        --threadData->loopLevel;
-        QT_RETHROW;
-    }
+    returnValue = notify(receiver, event);
 
 #ifdef QT_JAMBI_BUILD
     // Restore the previous state if the object was not deleted..
@@ -841,7 +845,6 @@ bool QCoreApplication::notifyInternal(QObject *receiver, QEvent *event)
     }
     QObjectPrivate::resetDeleteWatch(d, oldDeleteWatch, deleteWatch);
 #endif
-    --threadData->loopLevel;
     return returnValue;
 }
 
@@ -1384,6 +1387,40 @@ void QCoreApplicationPrivate::sendPostedEvents(QObject *receiver, int event_type
     int &i = (!event_type && !receiver) ? data->postEventList.startOffset : startOffset;
     data->postEventList.insertionOffset = data->postEventList.size();
 
+    // Exception-safe cleaning up without the need for a try/catch block
+    struct CleanUp {
+        QObject *receiver;
+        int event_type;
+        QThreadData *data;
+        bool exceptionCaught;
+
+        inline CleanUp(QObject *receiver, int event_type, QThreadData *data) :
+            receiver(receiver), event_type(event_type), data(data), exceptionCaught(true)
+        {}
+        inline ~CleanUp()
+        {
+            if (exceptionCaught) {
+                // since we were interrupted, we need another pass to make sure we clean everything up
+                data->canWait = false;
+            }
+
+            --data->postEventList.recursion;
+            if (!data->postEventList.recursion && !data->canWait && data->eventDispatcher)
+                data->eventDispatcher->wakeUp();
+
+            // clear the global list, i.e. remove everything that was
+            // delivered.
+            if (!event_type && !receiver && data->postEventList.startOffset >= 0) {
+                const QPostEventList::iterator it = data->postEventList.begin();
+                data->postEventList.erase(it, it + data->postEventList.startOffset);
+                data->postEventList.insertionOffset -= data->postEventList.startOffset;
+                Q_ASSERT(data->postEventList.insertionOffset >= 0);
+                data->postEventList.startOffset = 0;
+            }
+        }
+    };
+    CleanUp cleanup(receiver, event_type, data);
+
     while (i < data->postEventList.size()) {
         // avoid live-lock
         if (i >= data->postEventList.insertionOffset)
@@ -1422,7 +1459,7 @@ void QCoreApplicationPrivate::sendPostedEvents(QObject *receiver, int event_type
         // first, we diddle the event so that we can deliver
         // it, and that no one will try to touch it later.
         pe.event->posted = false;
-        QEvent * e = pe.event;
+        QScopedPointer<QEvent> e(pe.event);
         QObject * r = pe.receiver;
 
         --r->d_func()->postedEvents;
@@ -1432,49 +1469,23 @@ void QCoreApplicationPrivate::sendPostedEvents(QObject *receiver, int event_type
         // for the next event.
         const_cast<QPostEvent &>(pe).event = 0;
 
-        locker.unlock();
-        // after all that work, it's time to deliver the event.
-#ifdef QT_NO_EXCEPTIONS
-        QCoreApplication::sendEvent(r, e);
-#else
-        try {
-            QCoreApplication::sendEvent(r, e);
-        } catch (...) {
-            delete e;
-            locker.relock();
-
-            // since we were interrupted, we need another pass to make sure we clean everything up
-            data->canWait = false;
-
-            // uglehack: copied from below
-            --data->postEventList.recursion;
-            if (!data->postEventList.recursion && !data->canWait && data->eventDispatcher)
-                data->eventDispatcher->wakeUp();
-            throw;              // rethrow
-        }
-#endif
+        struct MutexUnlocker
+        {
+            QMutexLocker &m;
+            MutexUnlocker(QMutexLocker &m) : m(m) { m.unlock(); }
+            ~MutexUnlocker() { m.relock(); }
+        };
+        MutexUnlocker unlocker(locker);
 
-        delete e;
-        locker.relock();
+        // after all that work, it's time to deliver the event.
+        QCoreApplication::sendEvent(r, e.data());
 
         // careful when adding anything below this point - the
         // sendEvent() call might invalidate any invariants this
         // function depends on.
     }
 
-    --data->postEventList.recursion;
-    if (!data->postEventList.recursion && !data->canWait && data->eventDispatcher)
-        data->eventDispatcher->wakeUp();
-
-    // clear the global list, i.e. remove everything that was
-    // delivered.
-    if (!event_type && !receiver && data->postEventList.startOffset >= 0) {
-        const QPostEventList::iterator it = data->postEventList.begin();
-        data->postEventList.erase(it, it + data->postEventList.startOffset);
-        data->postEventList.insertionOffset -= data->postEventList.startOffset;
-        Q_ASSERT(data->postEventList.insertionOffset >= 0);
-        data->postEventList.startOffset = 0;
-    }
+    cleanup.exceptionCaught = false;
 }
 
 /*!

diff --git a/src/corelib/kernel/qeventloop.cpp b/src/corelib/kernel/qeventloop.cpp
index 5a4ce97..37c06a2 100644 (file)
--- a/src/corelib/kernel/qeventloop.cpp
+++ b/src/corelib/kernel/qeventloop.cpp
@@ -184,49 +184,47 @@ int QEventLoop::exec(ProcessEventsFlags flags)
         qWarning("QEventLoop::exec: instance %p has already called exec()", this);
         return -1;
     }
-    d->inExec = true;
-    d->exit = false;
-    ++d->threadData->loopLevel;
-    d->threadData->eventLoops.push(this);
-    locker.unlock();
+
+    struct LoopReference {
+        QEventLoopPrivate *d;
+        QMutexLocker &locker;
+
+        bool exceptionCaught;
+        LoopReference(QEventLoopPrivate *d, QMutexLocker &locker) : d(d), locker(locker), exceptionCaught(true)
+        {
+            d->inExec = true;
+            d->exit = false;
+            ++d->threadData->loopLevel;
+            d->threadData->eventLoops.push(d->q_func());
+            locker.unlock();
+        }
+
+        ~LoopReference()
+        {
+            if (exceptionCaught) {
+                qWarning("Qt has caught an exception thrown from an event handler. Throwing\n"
+                         "exceptions from an event handler is not supported in Qt. You must\n"
+                         "reimplement QApplication::notify() and catch all exceptions there.\n");
+            }
+            locker.relock();
+            QEventLoop *eventLoop = d->threadData->eventLoops.pop();
+            Q_ASSERT_X(eventLoop == d->q_func(), "QEventLoop::exec()", "internal error");
+            Q_UNUSED(eventLoop); // --release warning
+            d->inExec = false;
+            --d->threadData->loopLevel;
+        }
+    };
+    LoopReference ref(d, locker);
 
     // remove posted quit events when entering a new event loop
     QCoreApplication *app = QCoreApplication::instance();
     if (app && app->thread() == thread())
         QCoreApplication::removePostedEvents(app, QEvent::Quit);
 
-#if defined(QT_NO_EXCEPTIONS)
     while (!d->exit)
         processEvents(flags | WaitForMoreEvents | EventLoopExec);
-#else
-    try {
-        while (!d->exit)
-            processEvents(flags | WaitForMoreEvents | EventLoopExec);
-    } catch (...) {
-        qWarning("Qt has caught an exception thrown from an event handler. Throwing\n"
-                 "exceptions from an event handler is not supported in Qt. You must\n"
-                 "reimplement QApplication::notify() and catch all exceptions there.\n");
-
-        // copied from below
-        locker.relock();
-        QEventLoop *eventLoop = d->threadData->eventLoops.pop();
-        Q_ASSERT_X(eventLoop == this, "QEventLoop::exec()", "internal error");
-        Q_UNUSED(eventLoop); // --release warning
-        d->inExec = false;
-        --d->threadData->loopLevel;
-
-        throw;
-    }
-#endif
-
-    // copied above
-    locker.relock();
-    QEventLoop *eventLoop = d->threadData->eventLoops.pop();
-    Q_ASSERT_X(eventLoop == this, "QEventLoop::exec()", "internal error");
-    Q_UNUSED(eventLoop); // --release warning
-    d->inExec = false;
-    --d->threadData->loopLevel;
 
+    ref.exceptionCaught = false;
     return d->returnCode;
 }
 

diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp
index 192e0ac..c1ac70d 100644 (file)
--- a/src/corelib/kernel/qobject.cpp
+++ b/src/corelib/kernel/qobject.cpp
@@ -125,6 +125,39 @@ extern "C" Q_CORE_EXPORT void qt_removeObject(QObject *)
     }
 }
 
+struct QConnectionSenderSwitcher {
+    QObject *receiver;
+    QObjectPrivate::Sender *previousSender;
+    QObjectPrivate::Sender currentSender;
+    bool switched;
+
+    inline QConnectionSenderSwitcher() : switched(false) {}
+
+    inline QConnectionSenderSwitcher(QObject *receiver, QObject *sender, int signal_absolute_id)
+    {
+        switchSender(receiver, sender, signal_absolute_id);
+    }
+
+    inline void switchSender(QObject *receiver, QObject *sender, int signal_absolute_id)
+    {
+        this->receiver = receiver;
+        currentSender.sender = sender;
+        currentSender.signal = signal_absolute_id;
+        currentSender.ref = 1;
+        previousSender = QObjectPrivate::setCurrentSender(receiver, &currentSender);
+        switched = true;
+    }
+
+    inline ~QConnectionSenderSwitcher()
+    {
+        if (switched)
+            QObjectPrivate::resetCurrentSender(receiver, &currentSender, previousSender);
+    }
+private:
+    Q_DISABLE_COPY(QConnectionSenderSwitcher)
+};
+
+
 void (*QAbstractDeclarativeData::destroyed)(QAbstractDeclarativeData *, QObject *) = 0;
 void (*QAbstractDeclarativeData::parentChanged)(QAbstractDeclarativeData *, QObject *, QObject *) = 0;
 void (*QAbstractDeclarativeData::objectNameChanged)(QAbstractDeclarativeData *, QObject *) = 0;
@@ -1083,23 +1116,10 @@ bool QObject::event(QEvent *e)
             d_func()->inEventHandler = false;
 #endif
             QMetaCallEvent *mce = static_cast<QMetaCallEvent*>(e);
-            QObjectPrivate::Sender currentSender;
-            currentSender.sender = const_cast<QObject*>(mce->sender());
-            currentSender.signal = mce->signalId();
-            currentSender.ref = 1;
-            QObjectPrivate::Sender * const previousSender =
-                QObjectPrivate::setCurrentSender(this, &currentSender);
-#if defined(QT_NO_EXCEPTIONS)
+
+            QConnectionSenderSwitcher sw(this, const_cast<QObject*>(mce->sender()), mce->signalId());
+
             mce->placeMetaCall(this);
-#else
-            QT_TRY {
-                mce->placeMetaCall(this);
-            } QT_CATCH(...) {
-                QObjectPrivate::resetCurrentSender(this, &currentSender, previousSender);
-                QT_RETHROW;
-            }
-#endif
-            QObjectPrivate::resetCurrentSender(this, &currentSender, previousSender);
             break;
         }
 
@@ -2976,7 +2996,7 @@ bool QMetaObjectPrivate::connect(const QObject *sender, int signal_index,
         type &= Qt::UniqueConnection - 1;
     }
 
-    QObjectPrivate::Connection *c = new QObjectPrivate::Connection;
+    QScopedPointer<QObjectPrivate::Connection> c(new QObjectPrivate::Connection);
     c->sender = s;
     c->receiver = r;
     c->method_relative = method_index;
@@ -2986,16 +3006,11 @@ bool QMetaObjectPrivate::connect(const QObject *sender, int signal_index,
     c->nextConnectionList = 0;
     c->callFunction = callFunction;
 
-    QT_TRY {
-        QObjectPrivate::get(s)->addConnection(signal_index, c);
-    } QT_CATCH(...) {
-        delete c;
-        QT_RETHROW;
-    }
+    QObjectPrivate::get(s)->addConnection(signal_index, c.data());
 
     c->prev = &(QObjectPrivate::get(r)->senders);
     c->next = *c->prev;
-    *c->prev = c;
+    *c->prev = c.data();
     if (c->next)
         c->next->prev = &c->next;
 
@@ -3006,6 +3021,7 @@ bool QMetaObjectPrivate::connect(const QObject *sender, int signal_index,
         sender_d->connectedSignals[signal_index >> 5] |= (1 << (signal_index & 0x1f));
     }
 
+    c.take(); // stop tracking
     return true;
 }
 
@@ -3264,16 +3280,37 @@ void QMetaObject::activate(QObject *sender, const QMetaObject *m, int local_sign
 
     Qt::HANDLE currentThreadId = QThread::currentThreadId();
 
+    {
     QMutexLocker locker(signalSlotLock(sender));
-    QObjectConnectionListVector *connectionLists = sender->d_func()->connectionLists;
-    if (!connectionLists) {
+    struct ConnectionListsRef {
+        QObjectConnectionListVector *connectionLists;
+        ConnectionListsRef(QObjectConnectionListVector *connectionLists) : connectionLists(connectionLists)
+        {
+            if (connectionLists)
+                ++connectionLists->inUse;
+        }
+        ~ConnectionListsRef()
+        {
+            if (!connectionLists)
+                return;
+
+            --connectionLists->inUse;
+            Q_ASSERT(connectionLists->inUse >= 0);
+            if (connectionLists->orphaned) {
+                if (!connectionLists->inUse)
+                    delete connectionLists;
+            }
+        }
+
+        QObjectConnectionListVector *operator->() const { return connectionLists; }
+    };
+    ConnectionListsRef connectionLists = sender->d_func()->connectionLists;
+    if (!connectionLists.connectionLists) {
         locker.unlock();
         if (qt_signal_spy_callback_set.signal_end_callback != 0)
             qt_signal_spy_callback_set.signal_end_callback(sender, signal_absolute_index);
         return;
     }
-    ++connectionLists->inUse;
-
 
     const QObjectPrivate::ConnectionList *list;
     if (signal_index < connectionLists->count())
@@ -3323,13 +3360,10 @@ void QMetaObject::activate(QObject *sender, const QMetaObject *m, int local_sign
 #endif
             }
 
-            QObjectPrivate::Sender currentSender;
-            QObjectPrivate::Sender *previousSender = 0;
+            QConnectionSenderSwitcher sw;
+
             if (receiverInSameThread) {
-                currentSender.sender = sender;
-                currentSender.signal = signal_absolute_index;
-                currentSender.ref = 1;
-                previousSender = QObjectPrivate::setCurrentSender(receiver, &currentSender);
+                sw.switchSender(receiver, sender, signal_absolute_index);
             }
             const QObjectPrivate::StaticMetaCallFunction callFunction = c->callFunction;
             const int method_relative = c->method_relative;
@@ -3354,23 +3388,7 @@ void QMetaObject::activate(QObject *sender, const QMetaObject *m, int local_sign
                                                                 argv ? argv : empty_argv);
                 }
 
-#if defined(QT_NO_EXCEPTIONS)
                 metacall(receiver, QMetaObject::InvokeMetaMethod, method, argv ? argv : empty_argv);
-#else
-                QT_TRY {
-                    metacall(receiver, QMetaObject::InvokeMetaMethod, method, argv ? argv : empty_argv);
-                } QT_CATCH(...) {
-                    locker.relock();
-                    if (receiverInSameThread)
-                        QObjectPrivate::resetCurrentSender(receiver, &currentSender, previousSender);
-
-                    --connectionLists->inUse;
-                    Q_ASSERT(connectionLists->inUse >= 0);
-                    if (connectionLists->orphaned && !connectionLists->inUse)
-                        delete connectionLists;
-                    QT_RETHROW;
-                }
-#endif
 
                 if (qt_signal_spy_callback_set.slot_end_callback != 0)
                     qt_signal_spy_callback_set.slot_end_callback(receiver, method);
@@ -3378,9 +3396,6 @@ void QMetaObject::activate(QObject *sender, const QMetaObject *m, int local_sign
                 locker.relock();
             }
 
-            if (receiverInSameThread)
-                QObjectPrivate::resetCurrentSender(receiver, &currentSender, previousSender);
-
             if (connectionLists->orphaned)
                 break;
         } while (c != last && (c = c->nextConnectionList) != 0);
@@ -3391,17 +3406,8 @@ void QMetaObject::activate(QObject *sender, const QMetaObject *m, int local_sign
         //start over for all signals;
         ((list = &connectionLists->allsignals), true));
 
-    --connectionLists->inUse;
-    Q_ASSERT(connectionLists->inUse >= 0);
-    if (connectionLists->orphaned) {
-        if (!connectionLists->inUse)
-            delete connectionLists;
-    } else if (connectionLists->dirty) {
-        sender->d_func()->cleanConnectionLists();
     }
 
-    locker.unlock();
-
     if (qt_signal_spy_callback_set.signal_end_callback != 0)
         qt_signal_spy_callback_set.signal_end_callback(sender, signal_absolute_index);