diff options
author | Kent Hansen <kent.hansen@nokia.com> | 2012-06-28 15:29:22 +0200 |
---|---|---|
committer | Qt by Nokia <qt-info@nokia.com> | 2012-07-03 19:30:46 +0200 |
commit | 378416af7542e34196908896c9a69573b995adb0 (patch) | |
tree | 750186a60e69cf0e9335b23ce3e1ca245f176de2 /tests/auto/qscriptengine/tst_qscriptengine.cpp | |
parent | b9a5e257882c6418e28a82b9af1e9a6ac3044aea (diff) | |
download | qtscript-378416af7542e34196908896c9a69573b995adb0.tar.gz |
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 <ogoffart@woboq.com>
Diffstat (limited to 'tests/auto/qscriptengine/tst_qscriptengine.cpp')
-rw-r--r-- | tests/auto/qscriptengine/tst_qscriptengine.cpp | 76 |
1 files changed, 76 insertions, 0 deletions
diff --git a/tests/auto/qscriptengine/tst_qscriptengine.cpp b/tests/auto/qscriptengine/tst_qscriptengine.cpp index 23ab8ef..b1b416c 100644 --- a/tests/auto/qscriptengine/tst_qscriptengine.cpp +++ b/tests/auto/qscriptengine/tst_qscriptengine.cpp @@ -248,6 +248,8 @@ private slots: void dateConversionJSQt(); void dateConversionQtJS(); void stringListFromArrayWithEmptyElement(); + void collectQObjectWithCachedWrapper_data(); + void collectQObjectWithCachedWrapper(); }; tst_QScriptEngine::tst_QScriptEngine() @@ -6142,5 +6144,79 @@ void tst_QScriptEngine::stringListFromArrayWithEmptyElement() QStringList() << "" << "hello"); } +// QTBUG-21993 +void tst_QScriptEngine::collectQObjectWithCachedWrapper_data() +{ + QTest::addColumn<QString>("program"); + QTest::addColumn<QString>("ownership"); + QTest::addColumn<bool>("giveParent"); + QTest::addColumn<bool>("shouldBeCollected"); + + QString prog1 = QString::fromLatin1("newQObject(ownership, parent)"); + QTest::newRow("unassigned,cpp,no-parent") << prog1 << "cpp" << false << false; + QTest::newRow("unassigned,cpp,parent") << prog1 << "cpp" << true << false; + QTest::newRow("unassigned,auto,no-parent") << prog1 << "auto" << false << true; + QTest::newRow("unassigned,auto,parent") << prog1 << "auto" << true << false; + QTest::newRow("unassigned,script,no-parent") << prog1 << "script" << false << true; + QTest::newRow("unassigned,script,parent") << prog1 << "script" << true << true; + + QString prog2 = QString::fromLatin1("myObject = { foo: newQObject(ownership, parent) }; myObject.foo"); + QTest::newRow("global-property-property,cpp,no-parent") << prog2 << "cpp" << false << false; + QTest::newRow("global-property-property,cpp,parent") << prog2 << "cpp" << true << false; + QTest::newRow("global-property-property,auto,no-parent") << prog2 << "auto" << false << false; + QTest::newRow("global-property-property,auto,parent") << prog2 << "auto" << true << false; + QTest::newRow("global-property-property,script,no-parent") << prog2 << "script" << false << false; + QTest::newRow("global-property-property,script,parent") << prog2 << "script" << true << false; + +} + +void tst_QScriptEngine::collectQObjectWithCachedWrapper() +{ + struct Functions { + static QScriptValue newQObject(QScriptContext *ctx, QScriptEngine *eng) + { + QString ownershipString = ctx->argument(0).toString(); + QScriptEngine::ValueOwnership ownership; + if (ownershipString == "cpp") + ownership = QScriptEngine::QtOwnership; + else if (ownershipString == "auto") + ownership = QScriptEngine::AutoOwnership; + else if (ownershipString == "script") + ownership = QScriptEngine::ScriptOwnership; + else + return ctx->throwError("Ownership specifier 'cpp', 'auto' or 'script' expected"); + + QObject *parent = ctx->argument(1).toQObject(); + return eng->newQObject(new QObject(parent), ownership, + QScriptEngine::PreferExistingWrapperObject); + } + }; + + QFETCH(QString, program); + QFETCH(QString, ownership); + QFETCH(bool, giveParent); + QFETCH(bool, shouldBeCollected); + + QScriptEngine eng; + eng.globalObject().setProperty("ownership", ownership); + eng.globalObject().setProperty("newQObject", + eng.newFunction(Functions::newQObject)); + + QObject parent; + eng.globalObject().setProperty("parent", + giveParent ? eng.newQObject(&parent) + : QScriptValue(QScriptValue::NullValue)); + + QPointer<QObject> ptr = eng.evaluate(program).toQObject(); + QVERIFY(ptr != 0); + QVERIFY(ptr->parent() == (giveParent ? &parent : 0)); + + collectGarbage_helper(eng); + + if (ptr && shouldBeCollected) + QEXPECT_FAIL("", "Test can fail because the garbage collector is conservative", Continue); + QCOMPARE(ptr == 0, shouldBeCollected); +} + QTEST_MAIN(tst_QScriptEngine) #include "tst_qscriptengine.moc" |