Fix crashes with closures created in QML components

When closures created inside QML components are called after the
surrounding component (and consequently QML context) has been destroyed,
we are in a somewhat limited environment. Initially we would just crash
as the calling QML context is not valid anymore. We can alleviate that
by introducing reference counting on the context and letting the QML
context wrapper keep a strong reference. This avoids the crashes and
also ensures that at least imports continue to be accessible within
these contexts (as the singleton test case demonstrates).

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

1 files changed, 89 insertions, 43 deletions

diff --git a/src/qml/qml/qqmlcontext_p.h b/src/qml/qml/qqmlcontext_p.h
index a259fd62d8..d01820a430 100644
--- a/src/qml/qml/qqmlcontext_p.h
+++ b/src/qml/qml/qqmlcontext_p.h
@@ -78,6 +78,7 @@ class QQmlExpression;
 class QQmlExpressionPrivate;
 class QQmlJavaScriptExpression;
 class QQmlContextData;
+class QQmlGuardedContextData;
 class QQmlIncubatorPrivate;
 
 class QQmlContextPrivate : public QObjectPrivate
@@ -106,7 +107,7 @@ public:
 };
 
 class QQmlComponentAttached;
-class QQmlGuardedContextData;
+
 class Q_QML_PRIVATE_EXPORT QQmlContextData
 {
 public:
@@ -114,7 +115,6 @@ public:
     QQmlContextData(QQmlContext *);
     void emitDestruction();
     void clearContext();
-    void destroy();
     void invalidate();
 
     inline bool isValid() const {
@@ -122,10 +122,10 @@ public:
     }
 
     // My parent context and engine
-    QQmlContextData *parent;
+    QQmlContextData *parent = nullptr;
     QQmlEngine *engine;
 
-    void setParent(QQmlContextData *, bool parentTakesOwnership = false);
+    void setParent(QQmlContextData *);
     void refreshExpressions();
 
     void addObject(QObject *);
@@ -136,14 +136,14 @@ public:
     // If internal is false publicContext owns this.
     QQmlContext *asQQmlContext();
     QQmlContextPrivate *asQQmlContextPrivate();
+    quint32 refCount = 0;
     quint32 isInternal:1;
-    quint32 ownedByParent:1; // unrelated to isInternal; parent context deletes children if true.
     quint32 isJSContext:1;
     quint32 isPragmaLibraryContext:1;
     quint32 unresolvedNames:1; // True if expressions in this context failed to resolve a toplevel name
     quint32 hasEmittedDestruction:1;
     quint32 isRootObjectInCreation:1;
-    quint32 dummy:25;
+    quint32 dummy:26;
     QQmlContext *publicContext;
 
     // The incubator that is constructing this context if any
@@ -178,7 +178,7 @@ public:
     QQmlRefPointer<QQmlTypeNameCache> imports;
 
     // My children
-    QQmlContextData *childContexts;
+    QQmlContextData *childContexts = 0;
 
     // My peers in parent's childContexts list
     QQmlContextData  *nextChild;
@@ -191,7 +191,7 @@ public:
     QQmlData *contextObjects;
 
     // Doubly-linked list of context guards (XXX merge with contextObjects)
-    QQmlGuardedContextData *contextGuards;
+    QQmlGuardedContextData *contextGuards = 0;
 
     // id guards
     struct ContextGuard : public QQmlGuard<QObject>
@@ -210,7 +210,7 @@ public:
     void setIdProperty(int, QObject *);
 
     // Linked contexts. this owns linkedContext.
-    QQmlContextData *linkedContext;
+    QQmlContextDataRef linkedContext;
 
     // Linked list of uses of the Component attached property in this
     // context
@@ -224,91 +224,137 @@ public:
     }
 
 private:
+    friend class QQmlContextDataRef;
+    friend class QQmlContext; // needs to do manual refcounting :/
     void refreshExpressionsRecursive(bool isGlobal);
     void refreshExpressionsRecursive(QQmlJavaScriptExpression *);
-    ~QQmlContextData() {}
+    ~QQmlContextData();
+    void destroy();
 };
 
+
 class QQmlGuardedContextData
 {
 public:
-    inline QQmlGuardedContextData();
-    inline QQmlGuardedContextData(QQmlContextData *);
-    inline ~QQmlGuardedContextData();
-
-    inline QQmlContextData *contextData() const;
+    inline QQmlGuardedContextData() = default;
+    inline QQmlGuardedContextData(QQmlContextData *data)
+    { setContextData(data); }
+    inline ~QQmlGuardedContextData()
+    { clear(); }
+
+    inline QQmlContextData *contextData() const
+    { return m_contextData; }
     inline void setContextData(QQmlContextData *);
 
     inline bool isNull() const { return !m_contextData; }
 
     inline operator QQmlContextData*() const { return m_contextData; }
     inline QQmlContextData* operator->() const { return m_contextData; }
-    inline QQmlGuardedContextData &operator=(QQmlContextData *d);
+    inline QQmlGuardedContextData &operator=(QQmlContextData *d) {
+        setContextData(d); return *this;
+    }
 
 private:
-    QQmlGuardedContextData &operator=(const QQmlGuardedContextData &);
-    QQmlGuardedContextData(const QQmlGuardedContextData &);
+    QQmlGuardedContextData &operator=(const QQmlGuardedContextData &) = delete;
+    QQmlGuardedContextData(const QQmlGuardedContextData &) = delete;
     friend class QQmlContextData;
 
     inline void clear();
 
-    QQmlContextData *m_contextData;
-    QQmlGuardedContextData  *m_next;
-    QQmlGuardedContextData **m_prev;
+    QQmlContextData *m_contextData = 0;
+    QQmlGuardedContextData  *m_next = 0;
+    QQmlGuardedContextData **m_prev = 0;
 };
 
-QQmlGuardedContextData::QQmlGuardedContextData()
-: m_contextData(0), m_next(0), m_prev(0)
+
+void QQmlGuardedContextData::setContextData(QQmlContextData *contextData)
+ {
+    if (m_contextData == contextData)
+        return;
+    clear();
+
+    if (contextData) {
+        m_contextData = contextData;
+        m_next = contextData->contextGuards;
+        if (m_next) m_next->m_prev = &m_next;
+        m_prev = &contextData->contextGuards;
+        contextData->contextGuards = this;
+    }
+}
+
+void QQmlGuardedContextData::clear()
+{
+    if (m_prev) {
+        *m_prev = m_next;
+        if (m_next) m_next->m_prev = m_prev;
+        m_contextData = 0;
+        m_next = 0;
+        m_prev = 0;
+    }
+}
+
+QQmlContextDataRef::QQmlContextDataRef()
+    : m_contextData(0)
 {
 }
 
-QQmlGuardedContextData::QQmlGuardedContextData(QQmlContextData *data)
-: m_contextData(0), m_next(0), m_prev(0)
+QQmlContextDataRef::QQmlContextDataRef(const QQmlContextDataRef &other)
+    : m_contextData(other.m_contextData)
 {
-    setContextData(data);
+    if (m_contextData)
+        ++m_contextData->refCount;
 }
 
-QQmlGuardedContextData::~QQmlGuardedContextData()
+QQmlContextDataRef::QQmlContextDataRef(QQmlContextData *data)
+    : m_contextData(data)
+{
+    if (m_contextData)
+        ++m_contextData->refCount;
+}
+
+QQmlContextDataRef::~QQmlContextDataRef()
 {
     clear();
 }
 
-void QQmlGuardedContextData::setContextData(QQmlContextData *contextData)
+void QQmlContextDataRef::setContextData(QQmlContextData *contextData)
 {
+    if (m_contextData == contextData)
+        return;
     clear();
 
     if (contextData) {
         m_contextData = contextData;
-        m_next = contextData->contextGuards;
-        if (m_next) m_next->m_prev = &m_next;
-        m_prev = &contextData->contextGuards;
-        contextData->contextGuards = this;
+        ++m_contextData->refCount;
     }
 }
 
-QQmlContextData *QQmlGuardedContextData::contextData() const
+QQmlContextData *QQmlContextDataRef::contextData() const
 {
     return m_contextData;
 }
 
-void QQmlGuardedContextData::clear()
+void QQmlContextDataRef::clear()
 {
-    if (m_prev) {
-        *m_prev = m_next;
-        if (m_next) m_next->m_prev = m_prev;
-        m_contextData = 0;
-        m_next = 0;
-        m_prev = 0;
-    }
+    if (m_contextData && !--m_contextData->refCount)
+        m_contextData->destroy();
+    m_contextData = 0;
 }
 
-QQmlGuardedContextData &
-QQmlGuardedContextData::operator=(QQmlContextData *d)
+QQmlContextDataRef &
+QQmlContextDataRef::operator=(QQmlContextData *d)
 {
     setContextData(d);
     return *this;
 }
 
+QQmlContextDataRef &
+QQmlContextDataRef::operator=(const QQmlContextDataRef &other)
+{
+    setContextData(other.m_contextData);
+    return *this;
+}
+
 QQmlContextData::ContextGuard::ContextGuard()
 : context(0)
 {