From 729cde55784e17a0e923caa8142cef6918146cd2 Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Mon, 26 Aug 2013 15:25:47 +0200 Subject: [PATCH] Fix crashes when running tst_qqmlecmascript::importScripts with aggressive gc In the case of imported JavaScript files, it may happen that we parse the JS once and then re-use it across different places where it is imported. That means we parse and compile the JS once, keep the QV4::Script around and call it as a function closure with different qml global objects (contexts), depending on where it is imported from. In this situation it is possible that the QV4::Script's run() is called once, a new function object is created, we call it to return the JS library to "eval" itself into the qml scope and afterwards it may happen that the function object is garbage collected. It is at this point possible that the compilation unit's refcount therefore also drops to zero, and thus subsequent calls to QV4::Script::run() that create new QQmlBinding objects will access a dangling compilationUnit pointer. This patch fixes that by making QV4::Script - which is holding a QV4::Function pointer - also have a persistent, which maintainers a refcount on the compilation unit. If the script dies, the persistent will get collected and the last deref will delete the unit. A script can however outlive the engine, but PersistentValue has the mechanism built in to free itself on engine destruction, which will also deref the unit accordingly. Change-Id: I0a7f4e64497bde423ffa55c705af55cdb7d29cf2 Reviewed-by: Lars Knoll --- src/qml/jsruntime/qv4script.cpp | 33 +++++++++++++++++++++++++++++---- src/qml/jsruntime/qv4script_p.h | 1 + tools/v4/main.cpp | 2 -- 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/qml/jsruntime/qv4script.cpp b/src/qml/jsruntime/qv4script.cpp index 2e75caa..7f8e9e2 100644 --- a/src/qml/jsruntime/qv4script.cpp +++ b/src/qml/jsruntime/qv4script.cpp @@ -61,6 +61,8 @@ using namespace QV4; struct QmlBindingWrapper : FunctionObject { + Q_MANAGED + QmlBindingWrapper(ExecutionContext *scope, Function *f, Object *qml) : FunctionObject(scope, scope->engine->id_eval) , qml(qml) @@ -86,17 +88,39 @@ struct QmlBindingWrapper : FunctionObject wrapper->qmlContext->mark(); } -protected: - static const ManagedVTable static_vtbl; - private: Object *qml; CallContext *qmlContext; - }; DEFINE_MANAGED_VTABLE(QmlBindingWrapper); +struct CompilationUnitHolder : public QV4::Object +{ + Q_MANAGED + + CompilationUnitHolder(ExecutionEngine *engine, CompiledData::CompilationUnit *unit) + : Object(engine) + , unit(unit) + { + unit->ref(); + vtbl = &static_vtbl; + } + ~CompilationUnitHolder() + { + unit->deref(); + } + + static void destroy(Managed *that) + { + static_cast(that)->~CompilationUnitHolder(); + } + + QV4::CompiledData::CompilationUnit *unit; +}; + +DEFINE_MANAGED_VTABLE(CompilationUnitHolder); + Value QmlBindingWrapper::call(Managed *that, const Value &, Value *, int) { ExecutionEngine *engine = that->engine(); @@ -170,6 +194,7 @@ void Script::parse() isel->setUseFastLookups(false); QV4::CompiledData::CompilationUnit *compilationUnit = isel->compile(); vmFunction = compilationUnit->linkToEngine(v4); + compilationUnitHolder = Value::fromObject(new (v4->memoryManager) CompilationUnitHolder(v4, compilationUnit)); } if (!vmFunction) diff --git a/src/qml/jsruntime/qv4script_p.h b/src/qml/jsruntime/qv4script_p.h index 20f9285..250fd73 100644 --- a/src/qml/jsruntime/qv4script_p.h +++ b/src/qml/jsruntime/qv4script_p.h @@ -69,6 +69,7 @@ struct Q_QML_EXPORT Script { bool inheritContext; bool parsed; QV4::PersistentValue qml; + QV4::PersistentValue compilationUnitHolder; Function *vmFunction; bool parseAsBinding; diff --git a/tools/v4/main.cpp b/tools/v4/main.cpp index 0244afa..225a4f1 100644 --- a/tools/v4/main.cpp +++ b/tools/v4/main.cpp @@ -206,13 +206,11 @@ int main(int argc, char *argv[]) QV4::Script script(ctx, code, fn); script.parseAsBinding = runAsQml; script.parse(); - script.function()->compilationUnit->ref(); QV4::Value result = script.run(); if (!result.isUndefined()) { if (! qgetenv("SHOW_EXIT_VALUE").isEmpty()) std::cout << "exit value: " << qPrintable(result.toString(ctx)->toQString()) << std::endl; } - script.function()->compilationUnit->deref(); } catch (QV4::Exception& ex) { ex.accept(ctx); showException(ctx, ex); -- 2.7.4