Fix crash when QML engine warning handler feeds errors back into QML

When a QQmlEngine warning handler that's called during component
instantiation results in subsequent component instantiations, either via
the signal or via a Qt message handler like in the bug report, then we
might end up modifying the linked list of errored bindings before
returning from the QQmlEnginePrivate::warning() call. The easy fix is to
extract the QQmlError, unlink the delayed error from the linked list and
then deliver the error to the QQmlEngine.

Change-Id: I6b7be61b57b35636282595937046ff76091144a3
Task-number: QTBUG-53293
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

6 files changed, 15 insertions, 33 deletions

diff --git a/src/qml/qml/qqmlcomponent.cpp b/src/qml/qml/qqmlcomponent.cpp
index 3174bbecd3..fe4768db15 100644
--- a/src/qml/qml/qqmlcomponent.cpp
+++ b/src/qml/qml/qqmlcomponent.cpp
@@ -929,8 +929,7 @@ void QQmlComponentPrivate::complete(QQmlEnginePrivate *enginePriv, ConstructionS
 
         if (0 == enginePriv->inProgressCreations) {
             while (enginePriv->erroredBindings) {
-                enginePriv->warning(enginePriv->erroredBindings);
-                enginePriv->erroredBindings->removeError();
+                enginePriv->warning(enginePriv->erroredBindings->removeError());
             }
         }
     }
diff --git a/src/qml/qml/qqmlengine.cpp b/src/qml/qml/qqmlengine.cpp
index 613f9b4fe5..7e11177caa 100644
--- a/src/qml/qml/qqmlengine.cpp
+++ b/src/qml/qml/qqmlengine.cpp
@@ -1996,11 +1996,6 @@ void QQmlEnginePrivate::warning(const QList<QQmlError> &errors)
         dumpwarning(errors);
 }
 
-void QQmlEnginePrivate::warning(QQmlDelayedError *error)
-{
-    warning(error->error());
-}
-
 void QQmlEnginePrivate::warning(QQmlEngine *engine, const QQmlError &error)
 {
     if (engine)
@@ -2017,14 +2012,6 @@ void QQmlEnginePrivate::warning(QQmlEngine *engine, const QList<QQmlError> &erro
         dumpwarning(error);
 }
 
-void QQmlEnginePrivate::warning(QQmlEngine *engine, QQmlDelayedError *error)
-{
-    if (engine)
-        QQmlEnginePrivate::get(engine)->warning(error);
-    else
-        dumpwarning(error->error());
-}
-
 void QQmlEnginePrivate::warning(QQmlEnginePrivate *engine, const QQmlError &error)
 {
     if (engine)
diff --git a/src/qml/qml/qqmlengine_p.h b/src/qml/qml/qqmlengine_p.h
index d6110c6699..da52e01793 100644
--- a/src/qml/qml/qqmlengine_p.h
+++ b/src/qml/qml/qqmlengine_p.h
@@ -231,10 +231,8 @@ public:
     void sendExit(int retCode = 0);
     void warning(const QQmlError &);
     void warning(const QList<QQmlError> &);
-    void warning(QQmlDelayedError *);
     static void warning(QQmlEngine *, const QQmlError &);
     static void warning(QQmlEngine *, const QList<QQmlError> &);
-    static void warning(QQmlEngine *, QQmlDelayedError *);
     static void warning(QQmlEnginePrivate *, const QQmlError &);
     static void warning(QQmlEnginePrivate *, const QList<QQmlError> &);
 
diff --git a/src/qml/qml/qqmlincubator.cpp b/src/qml/qml/qqmlincubator.cpp
index 4546a4423f..df168960c6 100644
--- a/src/qml/qml/qqmlincubator.cpp
+++ b/src/qml/qml/qqmlincubator.cpp
@@ -367,10 +367,8 @@ finishIncubate:
         enginePriv->inProgressCreations--;
 
         if (0 == enginePriv->inProgressCreations) {
-            while (enginePriv->erroredBindings) {
-                enginePriv->warning(enginePriv->erroredBindings);
-                enginePriv->erroredBindings->removeError();
-            }
+            while (enginePriv->erroredBindings)
+                enginePriv->warning(enginePriv->erroredBindings->removeError());
         }
     } else if (!creator.isNull()) {
         vmeGuard.guard(creator.data());
@@ -575,10 +573,8 @@ void QQmlIncubator::clear()
 
         enginePriv->inProgressCreations--;
         if (0 == enginePriv->inProgressCreations) {
-            while (enginePriv->erroredBindings) {
-                enginePriv->warning(enginePriv->erroredBindings);
-                enginePriv->erroredBindings->removeError();
-            }
+            while (enginePriv->erroredBindings)
+                enginePriv->warning(enginePriv->erroredBindings->removeError());
         }
     }
 
diff --git a/src/qml/qml/qqmljavascriptexpression.cpp b/src/qml/qml/qqmljavascriptexpression.cpp
index 3daa107b64..40cf1417d0 100644
--- a/src/qml/qml/qqmljavascriptexpression.cpp
+++ b/src/qml/qml/qqmljavascriptexpression.cpp
@@ -455,7 +455,7 @@ void QQmlJavaScriptExpression::createQmlBinding(QQmlContextData *ctxt, QObject *
         error->catchJavaScriptException(v4);
         error->setErrorObject(qmlScope);
         if (!error->addError(ep))
-            ep->warning(error);
+            ep->warning(error->error());
         return;
     }
     setupFunction(qmlContext, script.vmFunction);
diff --git a/src/qml/qml/qqmljavascriptexpression_p.h b/src/qml/qml/qqmljavascriptexpression_p.h
index a028850074..bff8866011 100644
--- a/src/qml/qml/qqmljavascriptexpression_p.h
+++ b/src/qml/qml/qqmljavascriptexpression_p.h
@@ -63,16 +63,18 @@ class QQmlDelayedError
 {
 public:
     inline QQmlDelayedError() : nextError(nullptr), prevError(nullptr) {}
-    inline ~QQmlDelayedError() { removeError(); }
+    inline ~QQmlDelayedError() { (void)removeError(); }
 
     bool addError(QQmlEnginePrivate *);
 
-    inline void removeError() {
-        if (!prevError) return;
-        if (nextError) nextError->prevError = prevError;
-        *prevError = nextError;
-        nextError = nullptr;
-        prevError = nullptr;
+    Q_REQUIRED_RESULT inline QQmlError removeError() {
+        if (prevError) {
+            if (nextError) nextError->prevError = prevError;
+            *prevError = nextError;
+            nextError = nullptr;
+            prevError = nullptr;
+        }
+        return m_error;
     }
 
     inline bool isValid() const { return m_error.isValid(); }