From 378416af7542e34196908896c9a69573b995adb0 Mon Sep 17 00:00:00 2001 From: Kent Hansen Date: Thu, 28 Jun 2012 15:29:22 +0200 Subject: Ensure QObject wrappers are garbage-collected if appropriate Address the "###" comment. When QScriptEngine::newQObject() is called with the option PreferExistingWrapperObject, the resulting wrapper object is cached by the engine, in case it will be needed later (e.g., by a subsequent newQObject() call for the same QObject, with the same wrap options). But if a QObject wrapper object is only referenced by the QtScript internals (i.e., not reachable from the JSC stack/heap, or kept in a QScriptValue), the wrapper should not be kept alive if the ownership is ScriptOwnership, or if the ownership is AutoOwnership and the C++ object has no parent. If the wrapper is marked in that case, it won't get collected, and hence the C++ object will be kept alive, too. In practice, QtScript appears to leak memory (the objects will only be destroyed when the engine is destroyed). Our copy of JSC doesn't have a concept of weak references; the ClientData callback in the JSC markRoots() function (which causes QScriptEnginePrivate::mark() to be called) was moved to the end. This enables the wrapper and connection marking logic to determine whether a wrapper can be safely discarded (if it hasn't been marked by JSC by this point, it must be a weak reference). Task-number: QTBUG-21993 Change-Id: I61f6aafc91f080b80d3f5859148e645b80d9b653 Reviewed-by: Olivier Goffart --- src/script/bridge/qscriptqobject.cpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) (limited to 'src/script/bridge/qscriptqobject.cpp') diff --git a/src/script/bridge/qscriptqobject.cpp b/src/script/bridge/qscriptqobject.cpp index 2bc2547..15288a7 100644 --- a/src/script/bridge/qscriptqobject.cpp +++ b/src/script/bridge/qscriptqobject.cpp @@ -97,7 +97,7 @@ struct QObjectConnection QObjectDelegate *inst = static_cast(delegate); if ((inst->ownership() == QScriptEngine::ScriptOwnership) || ((inst->ownership() == QScriptEngine::AutoOwnership) - && inst->value() && !inst->value()->parent())) { + && !inst->hasParent())) { senderWrapper = JSC::JSValue(); } else { markStack.append(senderWrapper); @@ -2199,6 +2199,10 @@ QObjectData::~QObjectData() } } +// This function assumes all objects reachable elsewhere in the JS environment +// (stack, heap) have been marked already (see QScriptEnginePrivate::mark()). +// This determines whether any of QtScript's internal QObject wrappers are only +// weakly referenced and can be discarded. void QObjectData::mark(JSC::MarkStack& markStack) { if (connectionManager) @@ -2207,10 +2211,14 @@ void QObjectData::mark(JSC::MarkStack& markStack) QList::iterator it; for (it = wrappers.begin(); it != wrappers.end(); ) { const QScript::QObjectWrapperInfo &info = *it; - // ### don't mark if there are no other references. - // we need something like isMarked() - markStack.append(info.object); - ++it; + if (JSC::Heap::isCellMarked(info.object)) { + ++it; + } else if (info.isCollectableWhenWeaklyReferenced()) { + it = wrappers.erase(it); + } else { + markStack.append(info.object); + ++it; + } } } } -- cgit v1.2.1