Detect and abort if an object is deleted during signal handling

If an object gets deleted while one of its own signal handler
expressions is being evaluated, a subsequent crash is inevitable.

While we could introduce guards/checks in the signal handler kernel
(QMetaObject::activate(), QQmlNotifier::emitNotify() and friends) to
detect and mask the sender deletion, this arguably isn't helpful; the
code that emitted the signal is likely to access member variables
directly after emitting the signal, causing semi-random crashes.

This situation is a symptom of misbehaving application code. Catch it
early rather than later, and issue a qFatal() with a helpful message.
Coupled with a backtrace, this should make it easier to track down
the flawed C++ application logic.

Change-Id: I8c77800e49c475def613224f208181c2d0af60e6
Reviewed-by: Lars Knoll <lars.knoll@nokia.com>
Reviewed-by: Aaron Kennedy <aaron.kennedy@nokia.com>

2 files changed, 30 insertions, 0 deletions

diff --git a/src/qml/qml/qqmlboundsignal_p.h b/src/qml/qml/qqmlboundsignal_p.h
index f9159ee70f..e3ef65ed7f 100644
--- a/src/qml/qml/qqmlboundsignal_p.h
+++ b/src/qml/qml/qqmlboundsignal_p.h
@@ -118,6 +118,7 @@ public:
     virtual QQmlBoundSignalExpressionPointer setExpression(QQmlBoundSignalExpression *) = 0;
     virtual QQmlBoundSignalExpressionPointer takeExpression(QQmlBoundSignalExpression *) = 0;
     virtual QObject *scope() = 0;
+    virtual bool isEvaluating() const = 0;
 
     void removeFromObject();
 protected:
diff --git a/src/qml/qml/qqmlengine.cpp b/src/qml/qml/qqmlengine.cpp
index d3ca1b343c..2ebd13bb14 100644
--- a/src/qml/qml/qqmlengine.cpp
+++ b/src/qml/qml/qqmlengine.cpp
@@ -1307,6 +1307,35 @@ void QQmlData::destroyed(QObject *object)
 
     QQmlAbstractBoundSignal *signalHandler = signalHandlers;
     while (signalHandler) {
+        if (signalHandler->isEvaluating()) {
+            // The object is being deleted during signal handler evaluation.
+            // This will cause a crash due to invalid memory access when the
+            // evaluation has completed.
+            // Abort with a friendly message instead.
+            QString locationString;
+            QQmlBoundSignalExpression *expr = signalHandler->expression();
+            if (expr) {
+                QString fileName = expr->sourceFile();
+                if (fileName.isEmpty())
+                    fileName = QStringLiteral("<Unknown File>");
+                locationString.append(fileName);
+                locationString.append(QString::fromLatin1(":%0: ").arg(expr->lineNumber()));
+                QString source = expr->expression();
+                if (source.size() > 100) {
+                    source.truncate(96);
+                    source.append(QStringLiteral(" ..."));
+                }
+                locationString.append(source);
+            } else {
+                locationString = QStringLiteral("<Unknown Location>");
+            }
+            qFatal("Object %p destroyed while one of its QML signal handlers is in progress.\n"
+                   "Most likely the object was deleted synchronously (use QObject::deleteLater() "
+                   "instead), or the application is running a nested event loop.\n"
+                   "This behavior is NOT supported!\n"
+                   "%s", object, qPrintable(locationString));
+        }
+
         QQmlAbstractBoundSignal *next = signalHandler->m_nextSignal;
         signalHandler->m_prevSignal = 0;
         signalHandler->m_nextSignal = 0;