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 <lars.knoll@digia.com>

diff --git a/src/qml/jsruntime/qv4script.cpp b/src/qml/jsruntime/qv4script.cpp
index 2e75caa..7f8e9e2 100644 (file)
--- 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<CompilationUnitHolder*>(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 (file)
--- 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 (file)
--- 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);