QML: Also check for correct destroy() chaining

Check that the destroy() method of Heap::Base was called when a Managed
object needs destruction. This checks if a call to the parent's
destroy() method was accidentally omitted.

Change-Id: Id025ecd6d4744bf3eab23503fbe317ed2a461138
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

25 files changed, 86 insertions, 22 deletions

diff --git a/src/imports/localstorage/plugin.cpp b/src/imports/localstorage/plugin.cpp
index a37c72508f..bf70fd1050 100644
--- a/src/imports/localstorage/plugin.cpp
+++ b/src/imports/localstorage/plugin.cpp
@@ -123,6 +123,7 @@ namespace Heap {
             delete database;
             delete version;
             delete sqlQuery;
+            Object::destroy();
         }
 
         Type type;
diff --git a/src/qml/jsruntime/qv4arraybuffer.cpp b/src/qml/jsruntime/qv4arraybuffer.cpp
index 908fa52680..23075aa78c 100644
--- a/src/qml/jsruntime/qv4arraybuffer.cpp
+++ b/src/qml/jsruntime/qv4arraybuffer.cpp
@@ -118,6 +118,7 @@ void Heap::ArrayBuffer::destroy()
 {
     if (!data->ref.deref())
         QTypedArrayData<char>::deallocate(data);
+    Object::destroy();
 }
 
 QByteArray ArrayBuffer::asByteArray() const
diff --git a/src/qml/jsruntime/qv4arraydata_p.h b/src/qml/jsruntime/qv4arraydata_p.h
index 9f810a32d5..24b948f01e 100644
--- a/src/qml/jsruntime/qv4arraydata_p.h
+++ b/src/qml/jsruntime/qv4arraydata_p.h
@@ -156,7 +156,10 @@ struct SimpleArrayData : public ArrayData {
 V4_ASSERT_IS_TRIVIAL(SimpleArrayData)
 
 struct SparseArrayData : public ArrayData {
-    void destroy() { delete sparse; }
+    void destroy() {
+        delete sparse;
+        ArrayData::destroy();
+    }
 
     uint mappedIndex(uint index) const {
         SparseArrayNode *n = sparse->findNode(index);
diff --git a/src/qml/jsruntime/qv4errorobject_p.h b/src/qml/jsruntime/qv4errorobject_p.h
index b7a0889e08..42a6e0b4b1 100644
--- a/src/qml/jsruntime/qv4errorobject_p.h
+++ b/src/qml/jsruntime/qv4errorobject_p.h
@@ -76,7 +76,10 @@ struct ErrorObject : Object {
     void init();
     void init(const Value &message, ErrorType t = Error);
     void init(const Value &message, const QString &fileName, int line, int column, ErrorType t = Error);
-    void destroy() { delete stackTrace; }
+    void destroy() {
+        delete stackTrace;
+        Object::destroy();
+    }
 
     ErrorType errorType;
     StackTrace *stackTrace;
diff --git a/src/qml/jsruntime/qv4functionobject.cpp b/src/qml/jsruntime/qv4functionobject.cpp
index 118ed89c43..2cc58b74a6 100644
--- a/src/qml/jsruntime/qv4functionobject.cpp
+++ b/src/qml/jsruntime/qv4functionobject.cpp
@@ -147,6 +147,7 @@ void Heap::FunctionObject::destroy()
 {
     if (function)
         function->compilationUnit->release();
+    Object::destroy();
 }
 
 void FunctionObject::init(String *n, bool createProto)
diff --git a/src/qml/jsruntime/qv4object_p.h b/src/qml/jsruntime/qv4object_p.h
index 6ddeb84d3c..e13a701b0f 100644
--- a/src/qml/jsruntime/qv4object_p.h
+++ b/src/qml/jsruntime/qv4object_p.h
@@ -69,6 +69,7 @@ namespace Heap {
 
 struct Object : Base {
     void init() { Base::init(); }
+    void destroy() { Base::destroy(); }
 
     const Value *propertyData(uint index) const { if (index < inlineMemberSize) return reinterpret_cast<const Value *>(this) + inlineMemberOffset + index; return memberData->data + index - inlineMemberSize; }
     Value *propertyData(uint index) { if (index < inlineMemberSize) return reinterpret_cast<Value *>(this) + inlineMemberOffset + index; return memberData->data + index - inlineMemberSize; }
diff --git a/src/qml/jsruntime/qv4qobjectwrapper_p.h b/src/qml/jsruntime/qv4qobjectwrapper_p.h
index 727af6c9c6..a32244c8de 100644
--- a/src/qml/jsruntime/qv4qobjectwrapper_p.h
+++ b/src/qml/jsruntime/qv4qobjectwrapper_p.h
@@ -79,7 +79,10 @@ struct QQmlValueTypeWrapper;
 
 struct QObjectWrapper : Object {
     void init(QObject *object);
-    void destroy() { qObj.destroy(); }
+    void destroy() {
+        qObj.destroy();
+        Object::destroy();
+    }
 
     QObject *object() const { return qObj.data(); }
 
@@ -131,7 +134,10 @@ struct QMetaObjectWrapper : FunctionObject {
 
 struct QmlSignalHandler : Object {
     void init(QObject *object, int signalIndex);
-    void destroy() { qObj.destroy(); }
+    void destroy() {
+        qObj.destroy();
+        Object::destroy();
+    }
     int signalIndex;
 
     QObject *object() const { return qObj.data(); }
diff --git a/src/qml/jsruntime/qv4regexp.cpp b/src/qml/jsruntime/qv4regexp.cpp
index c2bc905ef5..9e94c58432 100644
--- a/src/qml/jsruntime/qv4regexp.cpp
+++ b/src/qml/jsruntime/qv4regexp.cpp
@@ -124,6 +124,7 @@ void Heap::RegExp::destroy()
 #endif
     delete byteCode;
     delete pattern;
+    Base::destroy();
 }
 
 void RegExp::markObjects(Heap::Base *that, ExecutionEngine *e)
diff --git a/src/qml/jsruntime/qv4sequenceobject.cpp b/src/qml/jsruntime/qv4sequenceobject.cpp
index 4b82a7d83f..24890fdb18 100644
--- a/src/qml/jsruntime/qv4sequenceobject.cpp
+++ b/src/qml/jsruntime/qv4sequenceobject.cpp
@@ -221,6 +221,7 @@ struct QQmlSequence : Object {
     void destroy() {
         delete container;
         object.destroy();
+        Object::destroy();
     }
 
     mutable Container *container;
diff --git a/src/qml/jsruntime/qv4string_p.h b/src/qml/jsruntime/qv4string_p.h
index df34ec0151..23ec3349b9 100644
--- a/src/qml/jsruntime/qv4string_p.h
+++ b/src/qml/jsruntime/qv4string_p.h
@@ -76,6 +76,7 @@ struct Q_QML_PRIVATE_EXPORT String : Base {
     void destroy() {
         if (!largestSubLength && !text->ref.deref())
             QStringData::deallocate(text);
+        Base::destroy();
     }
     void simplifyString() const;
     int length() const {
diff --git a/src/qml/jsruntime/qv4variantobject_p.h b/src/qml/jsruntime/qv4variantobject_p.h
index efaf5e1e65..9a04069c12 100644
--- a/src/qml/jsruntime/qv4variantobject_p.h
+++ b/src/qml/jsruntime/qv4variantobject_p.h
@@ -73,6 +73,7 @@ struct VariantObject : Object
         if (isScarce())
             addVmePropertyReference();
         delete scarceData;
+        Object::destroy();
     }
     bool isScarce() const;
     int vmePropertyReferenceCount;
diff --git a/src/qml/memory/qv4heap_p.h b/src/qml/memory/qv4heap_p.h
index abbe85c04e..5a3797f397 100644
--- a/src/qml/memory/qv4heap_p.h
+++ b/src/qml/memory/qv4heap_p.h
@@ -133,19 +133,41 @@ struct Q_QML_EXPORT Base {
     void *operator new(size_t, Heap::Base *m) { return m; }
     void operator delete(void *, Heap::Base *) {}
 
-    void init() { setInitialized(); }
+    void init() { _setInitialized(); }
+    void destroy() { _setDestroyed(); }
 #ifdef QML_CHECK_INIT_DESTROY_CALLS
-    bool _isInitialized;
+    enum { Uninitialized = 0, Initialized, Destroyed } _livenessStatus;
     void _checkIsInitialized() {
-        if (!_isInitialized)
+        if (_livenessStatus == Uninitialized)
             fprintf(stderr, "ERROR: use of object '%s' before call to init() !!\n",
                     vtable()->className);
-        Q_ASSERT(_isInitialized);
+        else if (_livenessStatus == Destroyed)
+            fprintf(stderr, "ERROR: use of object '%s' after call to destroy() !!\n",
+                    vtable()->className);
+        Q_ASSERT(_livenessStatus = Initialized);
+    }
+    void _checkIsDestroyed() {
+        if (_livenessStatus == Initialized)
+            fprintf(stderr, "ERROR: object '%s' was never destroyed completely !!\n",
+                    vtable()->className);
+        Q_ASSERT(_livenessStatus == Destroyed);
+    }
+    void _setInitialized() { Q_ASSERT(_livenessStatus == Uninitialized); _livenessStatus = Initialized; }
+    void _setDestroyed() {
+        if (_livenessStatus == Uninitialized)
+            fprintf(stderr, "ERROR: attempting to destroy an uninitialized object '%s' !!\n",
+                    vtable()->className);
+        else if (_livenessStatus == Destroyed)
+            fprintf(stderr, "ERROR: attempting to destroy repeatedly object '%s' !!\n",
+                    vtable()->className);
+        Q_ASSERT(_livenessStatus == Initialized);
+        _livenessStatus = Destroyed;
     }
-    void setInitialized() { Q_ASSERT(!_isInitialized); _isInitialized = true; }
 #else
     Q_ALWAYS_INLINE void _checkIsInitialized() {}
-    Q_ALWAYS_INLINE void setInitialized() {}
+    Q_ALWAYS_INLINE void _checkIsDestroyed() {}
+    Q_ALWAYS_INLINE void _setInitialized() {}
+    Q_ALWAYS_INLINE void _setDestroyed() {}
 #endif
 };
 V4_ASSERT_IS_TRIVIAL(Base)
diff --git a/src/qml/memory/qv4mm.cpp b/src/qml/memory/qv4mm.cpp
index 3d25bd1639..ad4ecfe76d 100644
--- a/src/qml/memory/qv4mm.cpp
+++ b/src/qml/memory/qv4mm.cpp
@@ -229,8 +229,10 @@ bool sweepChunk(MemoryManager::Data::ChunkHeader *header, uint *itemsInUse, Exec
                     *unmanagedHeapSize -= heapBytes;
                 }
 
-                if (m->vtable()->destroy)
+                if (m->vtable()->destroy) {
                     m->vtable()->destroy(m);
+                    m->_checkIsDestroyed();
+                }
 
                 memset(m, 0, header->itemSize);
 #ifdef V4_USE_VALGRIND
diff --git a/src/qml/qml/qqmlcomponent.cpp b/src/qml/qml/qqmlcomponent.cpp
index 23b8e5a712..8be5172cd4 100644
--- a/src/qml/qml/qqmlcomponent.cpp
+++ b/src/qml/qml/qqmlcomponent.cpp
@@ -1497,6 +1497,7 @@ void QV4::Heap::QmlIncubatorObject::init(QQmlIncubator::IncubationMode m)
 void QV4::Heap::QmlIncubatorObject::destroy() {
     delete incubator;
     parent.destroy();
+    Object::destroy();
 }
 
 void QV4::QmlIncubatorObject::setInitialState(QObject *o)
diff --git a/src/qml/qml/qqmlcontextwrapper.cpp b/src/qml/qml/qqmlcontextwrapper.cpp
index 8827216136..2418003519 100644
--- a/src/qml/qml/qqmlcontextwrapper.cpp
+++ b/src/qml/qml/qqmlcontextwrapper.cpp
@@ -77,6 +77,7 @@ void Heap::QmlContextWrapper::destroy()
         (*context)->destroy();
     delete context;
     scopeObject.destroy();
+    Object::destroy();
 }
 
 ReturnedValue QmlContextWrapper::qmlScope(ExecutionEngine *v4, QQmlContextData *ctxt, QObject *scope)
diff --git a/src/qml/qml/qqmllistwrapper.cpp b/src/qml/qml/qqmllistwrapper.cpp
index 91225a1fea..8aa107dc17 100644
--- a/src/qml/qml/qqmllistwrapper.cpp
+++ b/src/qml/qml/qqmllistwrapper.cpp
@@ -64,6 +64,7 @@ void Heap::QmlListWrapper::init()
 void Heap::QmlListWrapper::destroy()
 {
     object.destroy();
+    Object::destroy();
 }
 
 ReturnedValue QmlListWrapper::create(ExecutionEngine *engine, QObject *object, int propId, int propType)
diff --git a/src/qml/qml/qqmllocale_p.h b/src/qml/qml/qqmllocale_p.h
index ea1b7bf369..275f58db7d 100644
--- a/src/qml/qml/qqmllocale_p.h
+++ b/src/qml/qml/qqmllocale_p.h
@@ -144,7 +144,10 @@ namespace Heap {
 
 struct QQmlLocaleData : Object {
     inline void init() { locale = new QLocale; }
-    void destroy() { delete locale; }
+    void destroy() {
+        delete locale;
+        Object::destroy();
+    }
     QLocale *locale;
 };
 
diff --git a/src/qml/qml/qqmltypewrapper.cpp b/src/qml/qml/qqmltypewrapper.cpp
index a3ac207fc3..5c3ad6b2a6 100644
--- a/src/qml/qml/qqmltypewrapper.cpp
+++ b/src/qml/qml/qqmltypewrapper.cpp
@@ -67,6 +67,7 @@ void Heap::QmlTypeWrapper::destroy()
     if (typeNamespace)
         typeNamespace->release();
     object.destroy();
+    Object::destroy();
 }
 
 bool QmlTypeWrapper::isSingleton() const
diff --git a/src/qml/qml/qqmlvaluetypewrapper.cpp b/src/qml/qml/qqmlvaluetypewrapper.cpp
index 11849d2c63..b23bc033d1 100644
--- a/src/qml/qml/qqmlvaluetypewrapper.cpp
+++ b/src/qml/qml/qqmlvaluetypewrapper.cpp
@@ -67,7 +67,10 @@ struct QQmlValueTypeReference : QQmlValueTypeWrapper
         QQmlValueTypeWrapper::init();
         object.init();
     }
-    void destroy() { object.destroy(); }
+    void destroy() {
+        object.destroy();
+        QQmlValueTypeWrapper::destroy();
+    }
     QQmlQPointer<QObject> object;
     int property;
 };
@@ -77,8 +80,7 @@ struct QQmlValueTypeReference : QQmlValueTypeWrapper
 struct QQmlValueTypeReference : public QQmlValueTypeWrapper
 {
     V4_OBJECT2(QQmlValueTypeReference, QQmlValueTypeWrapper)
-
-    static void destroy(Heap::Base *that);
+    V4_NEEDS_DESTROY
 
     bool readReferenceValue() const;
 };
@@ -95,6 +97,7 @@ void Heap::QQmlValueTypeWrapper::destroy()
         valueType->metaType.destruct(gadgetPtr);
         ::operator delete(gadgetPtr);
     }
+    Object::destroy();
 }
 
 void Heap::QQmlValueTypeWrapper::setValue(const QVariant &value) const
@@ -491,9 +494,4 @@ void QQmlValueTypeWrapper::put(Managed *m, String *name, const Value &value)
     }
 }
 
-void QQmlValueTypeReference::destroy(Heap::Base *that)
-{
-    static_cast<Heap::QQmlValueTypeReference*>(that)->Heap::QQmlValueTypeReference::~QQmlValueTypeReference();
-}
-
 QT_END_NAMESPACE
diff --git a/src/qml/qml/qqmlxmlhttprequest.cpp b/src/qml/qml/qqmlxmlhttprequest.cpp
index fe2d7da694..f94946820f 100644
--- a/src/qml/qml/qqmlxmlhttprequest.cpp
+++ b/src/qml/qml/qqmlxmlhttprequest.cpp
@@ -179,6 +179,7 @@ struct NamedNodeMap : Object {
         delete listPtr;
         if (d)
             d->release();
+        Object::destroy();
     }
     QList<NodeImpl *> &list() {
         if (listPtr == nullptr)
@@ -195,6 +196,7 @@ struct NodeList : Object {
     void destroy() {
         if (d)
             d->release();
+        Object::destroy();
     }
     NodeImpl *d;
 };
@@ -208,6 +210,7 @@ struct Node : Object {
     void destroy() {
         if (d)
             d->release();
+        Object::destroy();
     }
     NodeImpl *d;
 };
@@ -1605,6 +1608,7 @@ struct QQmlXMLHttpRequestWrapper : Object {
 
     void destroy() {
         delete request;
+        Object::destroy();
     }
     QQmlXMLHttpRequest *request;
 };
diff --git a/src/qml/qml/v8/qqmlbuiltinfunctions_p.h b/src/qml/qml/v8/qqmlbuiltinfunctions_p.h
index c4bb5504a3..7602a92582 100644
--- a/src/qml/qml/v8/qqmlbuiltinfunctions_p.h
+++ b/src/qml/qml/v8/qqmlbuiltinfunctions_p.h
@@ -84,6 +84,7 @@ struct QQmlBindingFunction : FunctionObject {
     void init(const QV4::FunctionObject *originalFunction);
     void destroy() {
         delete bindingLocation;
+        Object::destroy();
     }
     Pointer<FunctionObject> originalFunction;
     // Set when the binding is created later
diff --git a/src/qml/types/qqmldelegatemodel.cpp b/src/qml/types/qqmldelegatemodel.cpp
index 395a3e5641..9b58ec35f8 100644
--- a/src/qml/types/qqmldelegatemodel.cpp
+++ b/src/qml/types/qqmldelegatemodel.cpp
@@ -78,6 +78,7 @@ struct QQmlDelegateModelGroupChangeArray : Object {
     void init(const QVector<QQmlChangeSet::Change> &changes);
     void destroy() {
         delete changes;
+        Object::destroy();
     }
 
     QVector<QQmlChangeSet::Change> *changes;
@@ -1872,6 +1873,7 @@ DEFINE_OBJECT_VTABLE(QQmlDelegateModelItemObject);
 void QV4::Heap::QQmlDelegateModelItemObject::destroy()
 {
     item->Dispose();
+    Object::destroy();
 }
 
 
diff --git a/src/qml/types/qqmllistmodel_p_p.h b/src/qml/types/qqmllistmodel_p_p.h
index bd2102679b..6e17b55e79 100644
--- a/src/qml/types/qqmllistmodel_p_p.h
+++ b/src/qml/types/qqmllistmodel_p_p.h
@@ -168,6 +168,7 @@ struct ModelObject : public QObjectWrapper {
         m_model = model;
         m_elementIndex = elementIndex;
     }
+    void destroy() { QObjectWrapper::destroy(); }
     QQmlListModel *m_model;
     int m_elementIndex;
 };
diff --git a/src/quick/items/context2d/qquickcontext2d.cpp b/src/quick/items/context2d/qquickcontext2d.cpp
index d94a098897..2483a8eadb 100644
--- a/src/quick/items/context2d/qquickcontext2d.cpp
+++ b/src/quick/items/context2d/qquickcontext2d.cpp
@@ -504,7 +504,10 @@ struct QQuickContext2DStyle : Object {
         patternRepeatX = false;
         patternRepeatY = false;
     }
-    void destroy() { delete brush; }
+    void destroy() {
+        delete brush;
+        Object::destroy();
+    }
 
     QBrush *brush;
     bool patternRepeatX:1;
@@ -513,7 +516,10 @@ struct QQuickContext2DStyle : Object {
 
 struct QQuickJSContext2DPixelData : Object {
     void init();
-    void destroy() { delete image; }
+    void destroy() {
+        delete image;
+        Object::destroy();
+    }
 
     QImage *image;
 };
diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
index 20f7b40a6b..a077eb13d1 100644
--- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
+++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
@@ -7267,6 +7267,7 @@ struct WeakReferenceSentinel : public Object {
 
     void destroy() {
         *resultPtr = weakRef->isNullOrUndefined();
+        Object::destroy();
     }
 
     WeakValue *weakRef;