Fixed evaluateInClosure and other V8 stuff

interface/src/ui/overlays/Overlays.cpp

@@ -642,6 +642,7 @@ EntityItemProperties Overlays::convertOverlayToEntityProperties(QVariantMap& ove
 
     ScriptEnginePointer scriptEngine = newScriptEngine();
     ScriptValue props = variantMapToScriptValue(overlayProps, *scriptEngine);
+    qDebug() << "Overlay props: " << scriptEngine->scriptValueDebugDetails(props);
     EntityItemProperties toReturn;
     EntityItemPropertiesFromScriptValueHonorReadOnly(props, toReturn);
     return toReturn;

libraries/script-engine/src/ScriptEngine.h

@@ -119,7 +119,7 @@ public:
     virtual void setProcessEventsInterval(int interval) = 0;
     virtual QThread* thread() const = 0;
     virtual void setThread(QThread* thread) = 0;
-    Q_INVOKABLE virtual void enterIsolateOnThisThread() = 0;
+    //Q_INVOKABLE virtual void enterIsolateOnThisThread() = 0;
     virtual ScriptValue undefinedValue() = 0;
     virtual ScriptValue uncaughtException() const = 0;
     virtual QStringList uncaughtExceptionBacktrace() const = 0;
@@ -127,6 +127,7 @@ public:
     virtual void updateMemoryCost(const qint64& deltaSize) = 0;
     virtual void requestCollectGarbage() = 0;
     virtual void compileTest() = 0;
+    virtual QString scriptValueDebugDetails(ScriptValue &value) = 0;
 
 public:
     // helper to detect and log warnings when other code invokes QScriptEngine/BaseScriptEngine in thread-unsafe ways

libraries/script-engine/src/ScriptManager.cpp

@@ -828,7 +828,7 @@ void ScriptManager::run() {
     }
     
 
-    _engine->enterIsolateOnThisThread();
+    //_engine->enterIsolateOnThisThread();
 
     _engine->compileTest();
 

libraries/script-engine/src/v8/ScriptEngineV8.cpp

@@ -113,6 +113,8 @@ ScriptValue ScriptEngineV8::makeError(const ScriptValue& _other, const QString&
     if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) {
         return nullValue();
     }
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     return nullValue();
@@ -138,6 +140,8 @@ ScriptValue ScriptEngineV8::checkScriptSyntax(ScriptProgramPointer program) {
     if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) {
         return nullValue();
     }
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     ScriptSyntaxCheckResultPointer syntaxCheck = program->checkSyntax();
@@ -194,6 +198,8 @@ ScriptValue ScriptEngineV8::cloneUncaughtException(const QString& extraDetail) {
     if (!hasUncaughtException()) {
         return nullValue();
     }
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     return nullValue();
@@ -343,7 +349,8 @@ V8ScriptValue Lambda::call() {
 
 #ifdef DEBUG_JS
 void ScriptEngineV8::_debugDump(const QString& header, const V8ScriptValue& object, const QString& footer) {
-    if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) {
+    // V8TODO
+    /*if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) {
         return;
     }
     if (!header.isEmpty()) {
@@ -360,7 +367,7 @@ void ScriptEngineV8::_debugDump(const QString& header, const V8ScriptValue& obje
     }
     if (!footer.isEmpty()) {
         qCDebug(shared) << footer;
-    }
+    }*/
 }
 #endif
 
@@ -370,7 +377,7 @@ v8::Platform* ScriptEngineV8::getV8Platform() {
 }
 
 ScriptEngineV8::ScriptEngineV8(ScriptManager* scriptManager) :
-    _scriptManager(scriptManager), _isEvaluating(false)
+    _scriptManager(scriptManager), _evaluatingCounter(0)
     //V8TODO
     //_arrayBufferClass(new ArrayBufferClass(this))
 {
@@ -395,7 +402,8 @@ ScriptEngineV8::ScriptEngineV8(ScriptManager* scriptManager) :
         v8::Isolate::CreateParams isolateParams;
         isolateParams.array_buffer_allocator = v8::ArrayBuffer::Allocator::NewDefaultAllocator();
         _v8Isolate = v8::Isolate::New(isolateParams);
-        _v8Isolate->Enter();
+        v8::Locker locker(_v8Isolate);
+        v8::Isolate::Scope isolateScope(_v8Isolate);
         v8::HandleScope handleScope(_v8Isolate);
         v8::Local<v8::Context> context = v8::Context::New(_v8Isolate);
         Q_ASSERT(!context.IsEmpty());
@@ -447,6 +455,8 @@ void ScriptEngineV8::registerEnum(const QString& enumName, QMetaEnum newEnum) {
         qCCritical(scriptengine) << "registerEnum called on invalid enum with name " << enumName;
         return;
     }
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
 
@@ -467,6 +477,8 @@ void ScriptEngineV8::registerValue(const QString& valueName, V8ScriptValue value
                                   Q_ARG(V8ScriptValue, value));
         return;
     }
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     QStringList pathToValue = valueName.split(".");
@@ -514,34 +526,36 @@ void ScriptEngineV8::registerGlobalObject(const QString& name, QObject* object)
 #ifdef THREAD_DEBUGGING
     qCDebug(scriptengine) << "ScriptEngineV8::registerGlobalObject() called on thread [" << QThread::currentThread() << "] name:" << name;
 #endif
-    bool is_isolate_exit_needed = false;
+    /*bool is_isolate_exit_needed = false;
     if(!_v8Isolate->IsCurrent() && !_v8Locker) {
         // V8TODO: Theoretically only script thread should access this, so it should be safe
         _v8Locker.reset(new v8::Locker(_v8Isolate));
         _v8Isolate->Enter();
         is_isolate_exit_needed = true;
-    }
+    }*/
-    {
+    //{
-        v8::HandleScope handleScope(_v8Isolate);
+    v8::Locker locker(_v8Isolate);
-        Q_ASSERT(_v8Isolate->IsCurrent());
+    v8::Isolate::Scope isolateScope(_v8Isolate);
-        v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
+    v8::HandleScope handleScope(_v8Isolate);
-        v8::Local<v8::Object> v8GlobalObject = getContext()->Global();
+    Q_ASSERT(_v8Isolate->IsCurrent());
-        v8::Local<v8::String> v8Name = v8::String::NewFromUtf8(_v8Isolate, name.toStdString().c_str()).ToLocalChecked();
+    v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
+    v8::Local<v8::Object> v8GlobalObject = getContext()->Global();
+    v8::Local<v8::String> v8Name = v8::String::NewFromUtf8(_v8Isolate, name.toStdString().c_str()).ToLocalChecked();
 
-        // V8TODO: Is IsEmpty check enough or IsValid is needed too?
+    // V8TODO: Is IsEmpty check enough or IsValid is needed too?
-        if (!v8GlobalObject->Get(getContext(), v8Name).IsEmpty()) {
+    if (!v8GlobalObject->Get(getContext(), v8Name).IsEmpty()) {
-            if (object) {
+        if (object) {
-                V8ScriptValue value = ScriptObjectV8Proxy::newQObject(this, object, ScriptEngine::QtOwnership);
+            V8ScriptValue value = ScriptObjectV8Proxy::newQObject(this, object, ScriptEngine::QtOwnership);
-                v8GlobalObject->Set(getContext(), v8Name, value.get());
+            v8GlobalObject->Set(getContext(), v8Name, value.get());
-            } else {
+        } else {
-                v8GlobalObject->Set(getContext(), v8Name, v8::Null(_v8Isolate));
+            v8GlobalObject->Set(getContext(), v8Name, v8::Null(_v8Isolate));
-            }
         }
     }
-    if (is_isolate_exit_needed) {
+    //}
+    /*if (is_isolate_exit_needed) {
         _v8Isolate->Exit();
         _v8Locker.reset(nullptr);
-    }
+    }*/
 }
 
 void ScriptEngineV8::registerFunction(const QString& name, ScriptEngine::FunctionSignature functionSignature, int numArguments) {
@@ -561,26 +575,28 @@ void ScriptEngineV8::registerFunction(const QString& name, ScriptEngine::Functio
     qCDebug(scriptengine) << "ScriptEngineV8::registerFunction() called on thread [" << QThread::currentThread() << "] name:" << name;
 #endif
 
-    bool is_isolate_exit_needed = false;
+    /*bool is_isolate_exit_needed = false;
     if(!_v8Isolate->IsCurrent() && !_v8Locker) {
         // V8TODO: Theoretically only script thread should access this, so it should be safe
         _v8Locker.reset(new v8::Locker(_v8Isolate));
         _v8Isolate->Enter();
         is_isolate_exit_needed = true;
     }
-    {
+    {*/
         //auto scriptFun = static_cast<ScriptValueV8Wrapper*>(newFunction(functionSignature, numArguments).ptr())->toV8Value().constGet();
-        v8::HandleScope handleScope(_v8Isolate);
+    v8::Locker locker(_v8Isolate);
-        v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
+    v8::Isolate::Scope isolateScope(_v8Isolate);
-        auto scriptFun = newFunction(functionSignature, numArguments);
+    v8::HandleScope handleScope(_v8Isolate);
+    v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
+    auto scriptFun = newFunction(functionSignature, numArguments);
 
-        //getContext()->Global().Set();
+    //getContext()->Global().Set();
-        globalObject().setProperty(name, scriptFun);
+    globalObject().setProperty(name, scriptFun);
-    }
+    /*}
     if (is_isolate_exit_needed) {
         _v8Isolate->Exit();
         _v8Locker.reset(nullptr);
-    }
+    }*/
 }
 
 void ScriptEngineV8::registerFunction(const QString& parent, const QString& name, ScriptEngine::FunctionSignature functionSignature, int numArguments) {
@@ -598,26 +614,28 @@ void ScriptEngineV8::registerFunction(const QString& parent, const QString& name
     qCDebug(scriptengine) << "ScriptEngineV8::registerFunction() called on thread [" << QThread::currentThread() << "] parent:" << parent << "name:" << name;
 #endif
 
-    bool is_isolate_exit_needed = false;
+    /*bool is_isolate_exit_needed = false;
     if(!_v8Isolate->IsCurrent() && !_v8Locker) {
         // V8TODO: Theoretically only script thread should access this, so it should be safe
         _v8Locker.reset(new v8::Locker(_v8Isolate));
         _v8Isolate->Enter();
         is_isolate_exit_needed = true;
     }
-    {
+    {*/
-        v8::HandleScope handleScope(_v8Isolate);
+    v8::Locker locker(_v8Isolate);
-        v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
+    v8::Isolate::Scope isolateScope(_v8Isolate);
-        ScriptValue object = globalObject().property(parent);
+    v8::HandleScope handleScope(_v8Isolate);
-        if (object.isValid()) {
+    v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
-            ScriptValue scriptFun = newFunction(functionSignature, numArguments);
+    ScriptValue object = globalObject().property(parent);
-            object.setProperty(name, scriptFun);
+    if (object.isValid()) {
-        }
+        ScriptValue scriptFun = newFunction(functionSignature, numArguments);
+        object.setProperty(name, scriptFun);
     }
+    /*}
     if (is_isolate_exit_needed) {
         _v8Isolate->Exit();
         _v8Locker.reset(nullptr);
-    }
+    }*/
 }
 
 void ScriptEngineV8::registerGetterSetter(const QString& name, ScriptEngine::FunctionSignature getter,
@@ -638,14 +656,16 @@ void ScriptEngineV8::registerGetterSetter(const QString& name, ScriptEngine::Fun
     qCDebug(scriptengine) << "ScriptEngineV8::registerGetterSetter() called on thread [" << QThread::currentThread() << "] name:" << name << "parent:" << parent;
 #endif
 
-    bool is_isolate_exit_needed = false;
+    /*bool is_isolate_exit_needed = false;
     if(!_v8Isolate->IsCurrent() && !_v8Locker) {
         // V8TODO: Theoretically only script thread should access this, so it should be safe
         _v8Locker.reset(new v8::Locker(_v8Isolate));
         _v8Isolate->Enter();
         is_isolate_exit_needed = true;
     }
-    {
+    {*/
+        v8::Locker locker(_v8Isolate);
+        v8::Isolate::Scope isolateScope(_v8Isolate);
         v8::HandleScope handleScope(_v8Isolate);
         v8::Context::Scope contextScope(getContext());
 
@@ -717,11 +737,11 @@ void ScriptEngineV8::registerGetterSetter(const QString& name, ScriptEngine::Fun
             //globalObject().setProperty(name, setterFunction, ScriptValue::PropertySetter);
             //globalObject().setProperty(name, getterFunction, ScriptValue::PropertyGetter);
         }
-    }
+    /*}
     if (is_isolate_exit_needed) {
         _v8Isolate->Exit();
         _v8Locker.reset(nullptr);
-    }
+    }*/
 }
 
 ScriptValue ScriptEngineV8::evaluateInClosure(const ScriptValue& _closure,
@@ -730,42 +750,59 @@ ScriptValue ScriptEngineV8::evaluateInClosure(const ScriptValue& _closure,
     if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) {
         return nullValue();
     }
-    Q_ASSERT(!isEvaluating());
+    _evaluatingCounter++;
-    _isEvaluating = true;
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
-    v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
-    ScriptProgramV8Wrapper* unwrappedProgram = ScriptProgramV8Wrapper::unwrap(_program);
-    if (unwrappedProgram == nullptr) {
-        _isEvaluating = false;
-        return nullValue();
-    }
-    const V8ScriptProgram& program = unwrappedProgram->toV8Value();
 
-    const auto fileName = unwrappedProgram->fileName();
+    v8::Local<v8::Object> closureObject;
-    const auto shortName = QUrl(fileName).fileName();
+    //v8::Local<v8::Value> oldGlobal;
-
-    ScriptValueV8Wrapper* unwrappedClosure = ScriptValueV8Wrapper::unwrap(_closure);
-    if (unwrappedClosure == nullptr) {
-        _isEvaluating = false;
-        return nullValue();
-    }
-    const V8ScriptValue& closure = unwrappedClosure->toV8Value();
-    if (!closure.constGet()->IsObject()) {
-        _isEvaluating = false;
-        return nullValue();
-    }
-    const v8::Local<v8::Object> closureObject = v8::Local<v8::Object>::Cast(closure.constGet());
-
-    v8::Local<v8::Value> oldGlobal;
     v8::Local<v8::Value> closureGlobal;
-    if (!closureObject->Get(closure.constGetContext() ,v8::String::NewFromUtf8(_v8Isolate, "global").ToLocalChecked()).ToLocal(&closureGlobal)) {
+    ScriptValueV8Wrapper* unwrappedClosure;
-        _isEvaluating = false;
+    ScriptProgramV8Wrapper* unwrappedProgram;
-        return nullValue();
-    }
 
-    _v8Context.Get(_v8Isolate)->Exit();
+    {
-    _v8Context.Get(_v8Isolate)->DetachGlobal();
+        v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
-    oldGlobal = _v8Context.Get(_v8Isolate)->Global();
+        unwrappedProgram = ScriptProgramV8Wrapper::unwrap(_program);
+        if (unwrappedProgram == nullptr) {
+            _evaluatingCounter--;
+            qDebug(scriptengine) << "Cannot unwrap program for closure";
+            Q_ASSERT(false);
+            return nullValue();
+        }
+        // V8TODO: is another context switch necessary after unwrapping closure?
+
+        const auto fileName = unwrappedProgram->fileName();
+        const auto shortName = QUrl(fileName).fileName();
+
+        unwrappedClosure = ScriptValueV8Wrapper::unwrap(_closure);
+        if (unwrappedClosure == nullptr) {
+            _evaluatingCounter--;
+            qDebug(scriptengine) << "Cannot unwrap closure";
+            Q_ASSERT(false);
+            return nullValue();
+        }
+
+        const V8ScriptValue& closure = unwrappedClosure->toV8Value();
+        const V8ScriptProgram& program = unwrappedProgram->toV8Value();
+        if (!closure.constGet()->IsObject()) {
+            _evaluatingCounter--;
+            qDebug(scriptengine) << "Unwrapped closure is not an object";
+            Q_ASSERT(false);
+            return nullValue();
+        }
+        closureObject = v8::Local<v8::Object>::Cast(closure.constGet());
+
+        if (!closureObject->Get(closure.constGetContext(), v8::String::NewFromUtf8(_v8Isolate, "global").ToLocalChecked())
+                 .ToLocal(&closureGlobal)) {
+            _evaluatingCounter--;
+            qDebug(scriptengine) << "Cannot get global from unwrapped closure";
+            Q_ASSERT(false);
+            return nullValue();
+        }
+    }
+    //_v8Context.Get(_v8Isolate)->DetachGlobal();
+    //oldGlobal = _v8Context.Get(_v8Isolate)->Global();
     v8::Local<v8::Context> closureContext;
 
     if (closureGlobal->IsObject()) {
@@ -775,61 +812,71 @@ ScriptValue ScriptEngineV8::evaluateInClosure(const ScriptValue& _closure,
         closureContext = v8::Context::New(_v8Isolate, nullptr, v8::Local<v8::ObjectTemplate>(), closureGlobal);
         //setGlobalObject(global);
     } else {
-        closureContext = v8::Context::New(_v8Isolate, nullptr, v8::Local<v8::ObjectTemplate>(), oldGlobal);
+        closureContext = v8::Context::New(_v8Isolate);
     }
 
-    //auto context = pushContext();
-
-    v8::Local<v8::Value> thiz;
-    // V8TODO: not sure if "this" doesn't exist or is empty in some cases
-    if (!closureObject->Get(closure.constGetContext() ,v8::String::NewFromUtf8(_v8Isolate, "this").ToLocalChecked()).ToLocal(&thiz)) {
-        _isEvaluating = false;
-        return nullValue();
-    }
-    //thiz = closure.property("this");
-    if (thiz->IsObject()) {
-#ifdef DEBUG_JS
-        qCDebug(shared) << " setting this = closure.this" << shortName;
-#endif
-        //V8TODO I don't know how to do this in V8, will adding "this" to global object work?
-        closureContext->Global()->Set(closureContext, v8::String::NewFromUtf8(_v8Isolate, "this").ToLocalChecked(), thiz);
-        //context->setThisObject(thiz);
-    }
-
-    //context->pushScope(closure);
-#ifdef DEBUG_JS
-    qCDebug(shared) << QString("[%1] evaluateInClosure %2").arg(isEvaluating()).arg(shortName);
-#endif
     ScriptValue result;
+    //auto context = pushContext();
     {
-        v8::TryCatch tryCatch(getIsolate());
+        v8::Context::Scope contextScope(closureContext);
-        auto maybeResult = program.constGet()->Run(closureContext);
+        const V8ScriptValue& closure = unwrappedClosure->toV8Value();
-        v8::Local<v8::Value> v8Result;
+        if (!unwrappedProgram->compile()) {
-        if (!maybeResult.ToLocal(&v8Result)) {
+            qDebug(scriptengine) << "Can't compile script for evaluating in closure";
-            v8::String::Utf8Value utf8Value(getIsolate(), tryCatch.Exception());
+            Q_ASSERT(false);
-            QString errorMessage = QString(*utf8Value);
+            return nullValue();
-            qWarning() << __FUNCTION__ << "---------- hasCaught:" << errorMessage;
-            //V8TODO: better error reporting
         }
+        const V8ScriptProgram& program = unwrappedProgram->toV8Value();
 
-        if (hasUncaughtException()) {
+        v8::Local<v8::Value> thiz;
-            auto err = cloneUncaughtException(__FUNCTION__);
+        // V8TODO: not sure if "this" doesn't exist or is empty in some cases
-#ifdef DEBUG_JS_EXCEPTIONS
+        if (!closureObject->Get(closure.constGetContext(), v8::String::NewFromUtf8(_v8Isolate, "this").ToLocalChecked())
-            qCWarning(shared) << __FUNCTION__ << "---------- hasCaught:" << err.toString() << result.toString();
+                 .ToLocal(&thiz)) {
-            err.setProperty("_result", result);
+            _evaluatingCounter--;
-#endif
+            qDebug(scriptengine) << "Empty this object in closure";
-            result = err;
+            Q_ASSERT(false);
-        } else {
+            return nullValue();
-            result = ScriptValue(new ScriptValueV8Wrapper(this, V8ScriptValue(_v8Isolate, v8Result)));
         }
-    }
+        //thiz = closure.property("this");
+        if (thiz->IsObject()) {
 #ifdef DEBUG_JS
-    qCDebug(shared) << QString("[%1] //evaluateInClosure %2").arg(isEvaluating()).arg(shortName);
+            qCDebug(shared) << " setting this = closure.this" << shortName;
 #endif
-    //popContext();
+            //V8TODO I don't know how to do this in V8, will adding "this" to global object work?
-    closureContext->Exit();
+            closureContext->Global()->Set(closureContext, v8::String::NewFromUtf8(_v8Isolate, "this").ToLocalChecked(), thiz);
-    _v8Context.Get(_v8Isolate)->Enter();
+            //context->setThisObject(thiz);
+        }
 
+        //context->pushScope(closure);
+#ifdef DEBUG_JS
+        qCDebug(shared) << QString("[%1] evaluateInClosure %2").arg(isEvaluating()).arg(shortName);
+#endif
+        {
+            v8::TryCatch tryCatch(getIsolate());
+            auto maybeResult = program.constGet()->Run(closureContext);
+            v8::Local<v8::Value> v8Result;
+            if (!maybeResult.ToLocal(&v8Result)) {
+                v8::String::Utf8Value utf8Value(getIsolate(), tryCatch.Exception());
+                QString errorMessage = QString(*utf8Value);
+                qWarning(scriptengine) << __FUNCTION__ << "---------- hasCaught:" << errorMessage;
+                //V8TODO: better error reporting
+            }
+
+            if (hasUncaughtException()) {
+                auto err = cloneUncaughtException(__FUNCTION__);
+#ifdef DEBUG_JS_EXCEPTIONS
+                qCWarning(shared) << __FUNCTION__ << "---------- hasCaught:" << err.toString() << result.toString();
+                err.setProperty("_result", result);
+#endif
+                result = err;
+            } else {
+                result = ScriptValue(new ScriptValueV8Wrapper(this, V8ScriptValue(_v8Isolate, v8Result)));
+            }
+        }
+#ifdef DEBUG_JS
+        qCDebug(shared) << QString("[%1] //evaluateInClosure %2").arg(isEvaluating()).arg(shortName);
+#endif
+        //popContext();
+    }
     //This is probably unnecessary in V8
     /*if (oldGlobal.isValid()) {
 #ifdef DEBUG_JS
@@ -838,8 +885,8 @@ ScriptValue ScriptEngineV8::evaluateInClosure(const ScriptValue& _closure,
         setGlobalObject(oldGlobal);
     }*/
 
-    //_v8Context.Get(_v8Isolate)->Enter();
+    closureContext->DetachGlobal();
-    _isEvaluating = false;
+    _evaluatingCounter--;
     return result;
 }
 
@@ -865,8 +912,9 @@ ScriptValue ScriptEngineV8::evaluate(const QString& sourceCode, const QString& f
     // Compile and check syntax
     // V8TODO: Could these all be replaced with checkSyntax function from wrapper?
     Q_ASSERT(!_v8Isolate->IsDead());
-    Q_ASSERT(!isEvaluating());
+    _evaluatingCounter++;
-    _isEvaluating = true;
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     v8::TryCatch tryCatch(getIsolate());
@@ -896,7 +944,7 @@ ScriptValue ScriptEngineV8::evaluate(const QString& sourceCode, const QString& f
         auto err = makeError(newValue(errorMessage));
         raiseException(err);
         maybeEmitUncaughtException("compile");
-        _isEvaluating = false;
+        _evaluatingCounter--;
         return err;
     }
     qCDebug(scriptengine) << "Script compilation succesful: " << fileName;
@@ -930,11 +978,11 @@ ScriptValue ScriptEngineV8::evaluate(const QString& sourceCode, const QString& f
         //V8TODO
         //raiseException(errorValue);
         //maybeEmitUncaughtException("evaluate");
-        _isEvaluating = false;
+        _evaluatingCounter--;
         return errorValue;
     }
     V8ScriptValue resultValue(_v8Isolate, result);
-    _isEvaluating = false;
+    _evaluatingCounter--;
     return ScriptValue(new ScriptValueV8Wrapper(this, std::move(resultValue)));
 }
 
@@ -979,19 +1027,20 @@ Q_INVOKABLE ScriptValue ScriptEngineV8::evaluate(const ScriptProgramPointer& pro
                                   Q_ARG(const ScriptProgramPointer&, program));
         return result;
     }
-    Q_ASSERT(!isEvaluating());
+    _evaluatingCounter++;
-    _isEvaluating = true;
+    /*bool is_isolate_exit_needed = false;
-    bool is_isolate_exit_needed = false;
     if(!_v8Isolate->IsCurrent() && !_v8Locker) {
         // V8TODO: Theoretically only script thread should access this, so it should be safe
         _v8Locker.reset(new v8::Locker(_v8Isolate));
         _v8Isolate->Enter();
         is_isolate_exit_needed = true;
-    }
+    }*/
     ScriptValue errorValue;
     ScriptValue resultValue;
     bool hasFailed = false;
     {
+        v8::Locker locker(_v8Isolate);
+        v8::Isolate::Scope isolateScope(_v8Isolate);
         v8::HandleScope handleScope(_v8Isolate);
         v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
         ScriptProgramV8Wrapper* unwrapped = ScriptProgramV8Wrapper::unwrap(program);
@@ -1039,11 +1088,11 @@ Q_INVOKABLE ScriptValue ScriptEngineV8::evaluate(const ScriptProgramPointer& pro
             resultValue = ScriptValue(new ScriptValueV8Wrapper(this, std::move(resultValueV8)));
         }
     }
-    _isEvaluating = false;
+    _evaluatingCounter--;
-    if (is_isolate_exit_needed) {
+    /*if (is_isolate_exit_needed) {
         _v8Isolate->Exit();
         _v8Locker.reset(nullptr);
-    }
+    }*/
     if (hasFailed) {
         return errorValue;
     } else {
@@ -1066,6 +1115,8 @@ void ScriptEngineV8::updateMemoryCost(const qint64& deltaSize) {
 // ScriptEngine implementation
 
 ScriptValue ScriptEngineV8::globalObject() const {
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     V8ScriptValue global(_v8Isolate, getConstContext()->Global());// = QScriptEngine::globalObject(); // can't cache the value as it may change
@@ -1077,6 +1128,8 @@ ScriptManager* ScriptEngineV8::manager() const {
 }
 
 ScriptValue ScriptEngineV8::newArray(uint length) {
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     V8ScriptValue result(_v8Isolate, v8::Array::New(_v8Isolate, static_cast<int>(length)));
@@ -1084,6 +1137,8 @@ ScriptValue ScriptEngineV8::newArray(uint length) {
 }
 
 ScriptValue ScriptEngineV8::newArrayBuffer(const QByteArray& message) {
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     //V8TODO: this will leak memory
@@ -1101,14 +1156,33 @@ ScriptValue ScriptEngineV8::newArrayBuffer(const QByteArray& message) {
 }
 
 ScriptValue ScriptEngineV8::newObject() {
-    v8::HandleScope handleScope(_v8Isolate);
+    /*bool is_isolate_exit_needed = false;
-    v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
+    if(!_v8Isolate->IsCurrent() && !_v8Locker) {
-    V8ScriptValue result(_v8Isolate, v8::Object::New(_v8Isolate));
+        // V8TODO: Theoretically only script thread should access this, so it should be safe
-    return ScriptValue(new ScriptValueV8Wrapper(this, std::move(result)));
+        _v8Locker.reset(new v8::Locker(_v8Isolate));
+        _v8Isolate->Enter();
+        is_isolate_exit_needed = true;
+    }*/
+    ScriptValue result;
+    {
+        v8::Locker locker(_v8Isolate);
+        v8::Isolate::Scope isolateScope(_v8Isolate);
+        v8::HandleScope handleScope(_v8Isolate);
+        v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
+        V8ScriptValue resultV8 = V8ScriptValue(_v8Isolate, v8::Object::New(_v8Isolate));
+        result = ScriptValue(new ScriptValueV8Wrapper(this, std::move(resultV8)));
+    }
+    /*if (is_isolate_exit_needed) {
+        _v8Isolate->Exit();
+        _v8Locker.reset(nullptr);
+    }*/
+    return result;
 }
 
 ScriptValue ScriptEngineV8::newMethod(QObject* object, V8ScriptValue lifetime,
                                const QList<QMetaMethod>& metas, int numMaxParams) {
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     V8ScriptValue result(ScriptMethodV8Proxy::newMethod(this, object, lifetime, metas, numMaxParams));
@@ -1120,6 +1194,8 @@ ScriptProgramPointer ScriptEngineV8::newProgram(const QString& sourceCode, const
     //V8TODO: should it be compiled on creation?
     //V8ScriptProgram result(sourceCode, fileName);
 
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     return std::make_shared<ScriptProgramV8Wrapper>(this, sourceCode, fileName);
@@ -1128,6 +1204,8 @@ ScriptProgramPointer ScriptEngineV8::newProgram(const QString& sourceCode, const
 ScriptValue ScriptEngineV8::newQObject(QObject* object,
                                                     ScriptEngine::ValueOwnership ownership,
                                                     const ScriptEngine::QObjectWrapOptions& options) {
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     V8ScriptValue result = ScriptObjectV8Proxy::newQObject(this, object, ownership, options);
@@ -1135,6 +1213,8 @@ ScriptValue ScriptEngineV8::newQObject(QObject* object,
 }
 
 ScriptValue ScriptEngineV8::newValue(bool value) {
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     V8ScriptValue result(_v8Isolate, v8::Boolean::New(_v8Isolate, value));
@@ -1142,6 +1222,8 @@ ScriptValue ScriptEngineV8::newValue(bool value) {
 }
 
 ScriptValue ScriptEngineV8::newValue(int value) {
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     V8ScriptValue result(_v8Isolate, v8::Integer::New(_v8Isolate, value));
@@ -1149,6 +1231,8 @@ ScriptValue ScriptEngineV8::newValue(int value) {
 }
 
 ScriptValue ScriptEngineV8::newValue(uint value) {
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     V8ScriptValue result(_v8Isolate, v8::Uint32::New(_v8Isolate, value));
@@ -1156,6 +1240,8 @@ ScriptValue ScriptEngineV8::newValue(uint value) {
 }
 
 ScriptValue ScriptEngineV8::newValue(double value) {
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     V8ScriptValue result(_v8Isolate, v8::Number::New(_v8Isolate, value));
@@ -1163,6 +1249,8 @@ ScriptValue ScriptEngineV8::newValue(double value) {
 }
 
 ScriptValue ScriptEngineV8::newValue(const QString& value) {
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     v8::Local<v8::String> valueV8 = v8::String::NewFromUtf8(_v8Isolate, value.toStdString().c_str(), v8::NewStringType::kNormal).ToLocalChecked();
@@ -1171,6 +1259,8 @@ ScriptValue ScriptEngineV8::newValue(const QString& value) {
 }
 
 ScriptValue ScriptEngineV8::newValue(const QLatin1String& value) {
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     v8::Local<v8::String> valueV8 = v8::String::NewFromUtf8(_v8Isolate, value.latin1(), v8::NewStringType::kNormal).ToLocalChecked();
@@ -1179,6 +1269,8 @@ ScriptValue ScriptEngineV8::newValue(const QLatin1String& value) {
 }
 
 ScriptValue ScriptEngineV8::newValue(const char* value) {
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     v8::Local<v8::String> valueV8 = v8::String::NewFromUtf8(_v8Isolate, value, v8::NewStringType::kNormal).ToLocalChecked();
@@ -1187,6 +1279,8 @@ ScriptValue ScriptEngineV8::newValue(const char* value) {
 }
 
 ScriptValue ScriptEngineV8::newVariant(const QVariant& value) {
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     V8ScriptValue result = castVariantToValue(value);
@@ -1236,7 +1330,7 @@ bool ScriptEngineV8::hasUncaughtException() const {
 
 bool ScriptEngineV8::isEvaluating() const {
     //return QScriptEngine::isEvaluating();
-    return _isEvaluating;
+    return _evaluatingCounter > 0;
     return false;
 }
 
@@ -1274,6 +1368,8 @@ ScriptValue ScriptEngineV8::newFunction(ScriptEngine::FunctionSignature fun, int
     //auto functionTemplate = v8::FunctionTemplate::New(_v8Isolate, v8FunctionCallback, v8::Local<v8::Value>(), v8::Local<v8::Signature>(), length);
     //auto functionData = v8::Object::New(_v8Isolate);
     //functionData->setIn
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     auto functionDataTemplate = v8::ObjectTemplate::New(_v8Isolate);
@@ -1299,6 +1395,8 @@ void ScriptEngineV8::setObjectName(const QString& name) {
 
 //V8TODO
 bool ScriptEngineV8::setProperty(const char* name, const QVariant& value) {
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     v8::Local<v8::Object> global = getContext()->Global();
@@ -1322,14 +1420,14 @@ void ScriptEngineV8::setThread(QThread* thread) {
         qDebug() << "Script engine " << objectName() << " exited isolate";
     }
     Q_ASSERT(QObject::thread() == QThread::currentThread());
-    if (_v8Locker) {
+    /*if (_v8Locker) {
         _v8Locker.reset();
-    }
+    }*/
     moveToThread(thread);
     qDebug() << "Moved script engine " << objectName() << " to different thread";
 }
 
-void ScriptEngineV8::enterIsolateOnThisThread() {
+/*void ScriptEngineV8::enterIsolateOnThisThread() {
     Q_ASSERT(thread() == QThread::currentThread());
     Q_ASSERT(!_v8Locker);
     _v8Locker.reset(new v8::Locker(_v8Isolate));
@@ -1337,7 +1435,7 @@ void ScriptEngineV8::enterIsolateOnThisThread() {
         _v8Isolate->Enter();
         qDebug() << "Script engine " << objectName() << " entered isolate on a new thread";
     }
-}
+}*/
 
 
 ScriptValue ScriptEngineV8::uncaughtException() const {
@@ -1367,6 +1465,8 @@ bool ScriptEngineV8::raiseException(const ScriptValue& exception) {
 }
 
 ScriptValue ScriptEngineV8::create(int type, const void* ptr) {
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     QVariant variant(type, ptr);
@@ -1375,6 +1475,8 @@ ScriptValue ScriptEngineV8::create(int type, const void* ptr) {
 }
 
 QVariant ScriptEngineV8::convert(const ScriptValue& value, int typeId) {
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     ScriptValueV8Wrapper* unwrapped = ScriptValueV8Wrapper::unwrap(value);
@@ -1397,20 +1499,44 @@ QVariant ScriptEngineV8::convert(const ScriptValue& value, int typeId) {
 }
 
 void ScriptEngineV8::compileTest() {
-    //v8::Locker locker(_v8Isolate);
+    v8::Locker locker(_v8Isolate);
-    //v8::Isolate::Scope isolateScope(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
     v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     v8::Local<v8::Script> script;
     v8::ScriptOrigin scriptOrigin(getIsolate(), v8::String::NewFromUtf8(getIsolate(),"test").ToLocalChecked());
     if (v8::Script::Compile(getContext(), v8::String::NewFromUtf8(getIsolate(), "print(\"hello world\");").ToLocalChecked(), &scriptOrigin).ToLocal(&script)) {
-        qCDebug(scriptengine) << "Compile test succesful";
+        qCDebug(scriptengine) << "Compile test successful";
     } else {
         qCDebug(scriptengine) << "Compile test failed";
         Q_ASSERT(false);
     }
 }
 
+QString ScriptEngineV8::scriptValueDebugDetails(ScriptValue &value) {
+    V8ScriptValue v8Value = ScriptValueV8Wrapper::fullUnwrap(this, value);
+    return scriptValueDebugDetailsV8(v8Value);
+}
+
+QString ScriptEngineV8::scriptValueDebugDetailsV8(const V8ScriptValue &v8Value) {
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
+    v8::HandleScope handleScope(_v8Isolate);
+    v8::Context::Scope contextScope(getContext());
+
+    QString parentValueQString("");
+    v8::Local<v8::String> parentValueString;
+    if (v8Value.constGet()->ToDetailString(getContext()).ToLocal(&parentValueString)) {
+        parentValueQString = QString(*v8::String::Utf8Value(_v8Isolate, parentValueString));
+    }
+    QString JSONQString;
+    v8::Local<v8::String> JSONString;
+    if (v8::JSON::Stringify(getContext(), v8Value.constGet()).ToLocal(&JSONString)) {
+        JSONQString = QString(*v8::String::Utf8Value(_v8Isolate, JSONString));
+    }
+    return parentValueQString + QString(" JSON: ") + JSONQString;
+}
+
 /*QStringList ScriptEngineV8::getCurrentStackTrace() {
     v8::Local<v8::StackTrace> stackTrace = v8::StackTrace::CurrentStackTrace(_v8Isolate, 100);
     QStringList backtrace;

libraries/script-engine/src/v8/ScriptEngineV8.h

@@ -119,7 +119,7 @@ public:  // ScriptEngine implementation
     virtual void setProcessEventsInterval(int interval) override;
     virtual QThread* thread() const override;
     virtual void setThread(QThread* thread) override;
-    Q_INVOKABLE virtual void enterIsolateOnThisThread() override;
+    //Q_INVOKABLE virtual void enterIsolateOnThisThread() override;
     virtual ScriptValue undefinedValue() override;
     virtual ScriptValue uncaughtException() const override;
     virtual QStringList uncaughtExceptionBacktrace() const override;
@@ -127,6 +127,8 @@ public:  // ScriptEngine implementation
     virtual void updateMemoryCost(const qint64& deltaSize) override;
     virtual void requestCollectGarbage() override { while(!_v8Isolate->IdleNotificationDeadline(getV8Platform()->MonotonicallyIncreasingTime() + GARBAGE_COLLECTION_TIME_LIMIT_S)) {}; }
     virtual void compileTest() override;
+    virtual QString scriptValueDebugDetails(ScriptValue &value) override;
+    QString scriptValueDebugDetailsV8(const V8ScriptValue &value);
 
     // helper to detect and log warnings when other code invokes QScriptEngine/BaseScriptEngine in thread-unsafe ways
     inline bool IS_THREADSAFE_INVOCATION(const QString& method) { return ScriptEngine::IS_THREADSAFE_INVOCATION(method); }
@@ -226,11 +228,13 @@ protected:
     ScriptValue _undefinedValue;
     mutable ScriptContextQtPointer _currContext;
     //QThread *_currentThread;
-    std::unique_ptr<v8::Locker> _v8Locker;
+    //V8TODO: probably should be removed and its occurrences replaced with local locker
+    //std::unique_ptr<v8::Locker> _v8Locker;
 
     //V8TODO
     //ArrayBufferClass* _arrayBufferClass;
-    bool _isEvaluating;
+    // Counts how many nested evaluate calls are there at a given point
+    int _evaluatingCounter;
 };
 
 // Lambda helps create callable V8ScriptValues out of std::functions:

libraries/script-engine/src/v8/ScriptEngineV8_cast.cpp

@@ -292,9 +292,18 @@ int ScriptEngineV8::computeCastPenalty(const V8ScriptValue& v8Val, int destTypeI
 }
 
 bool ScriptEngineV8::castValueToVariant(const V8ScriptValue& v8Val, QVariant& dest, int destTypeId) {
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
     v8::HandleScope handleScope(_v8Isolate);
+    v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate));
     const v8::Local<v8::Value> val = v8Val.constGet();
 
+    // Conversion debugging:
+    if (destTypeId == QMetaType::QVariant && val->IsBoolean()) {
+        //It's for placing breakpoint here
+        qDebug() << "Conversion Debug: " << scriptValueDebugDetailsV8(v8Val);
+    }
+
     // if we're not particularly interested in a specific type, try to detect if we're dealing with a registered type
     if (destTypeId == QMetaType::UnknownType) {
         QObject* obj = ScriptObjectV8Proxy::unwrap(v8Val);
@@ -376,9 +385,10 @@ bool ScriptEngineV8::castValueToVariant(const V8ScriptValue& v8Val, QVariant& de
                 }
                 // V8TODO
                 errorMessage = QString() + "Conversion failure: " + QString(*v8::String::Utf8Value(_v8Isolate, val->ToDetailString(getConstContext()).ToLocalChecked()))
-                               + "to variant. Destination type: " + QMetaType::typeName(destTypeId);
+                               + "to variant. Destination type: " + QMetaType::typeName(destTypeId) +" details: "+ scriptValueDebugDetailsV8(v8Val);
                 qDebug() << errorMessage;
-                Q_ASSERT(false);
+
+                //Q_ASSERT(false);
                 //dest = val->ToVariant();
                 break;
             case QMetaType::Bool:
@@ -440,16 +450,16 @@ bool ScriptEngineV8::castValueToVariant(const V8ScriptValue& v8Val, QVariant& de
             case QMetaType::QVariant:
                 if (val->IsUndefined()) {
                     dest = QVariant();
-                    break;
+                    return true;
                 }
                 if (val->IsNull()) {
                     dest = QVariant::fromValue(nullptr);
-                    break;
+                    return true;
                 }
                 if (val->IsBoolean()) {
                     //V8TODO is it right isolate? What if value from different script engine is used here
                     dest = QVariant::fromValue(val->BooleanValue(_v8Isolate));
-                    break;
+                    return true;
                 }
                 if (val->IsString()) {
                     //V8TODO is it right context? What if value from different script engine is used here
@@ -457,30 +467,30 @@ bool ScriptEngineV8::castValueToVariant(const V8ScriptValue& v8Val, QVariant& de
                     Q_ASSERT(*string != nullptr);
                     dest = QVariant::fromValue(QString(*string));
                     //dest = QVariant::fromValue(val->ToString(_v8Context.Get(_v8Isolate)).ToLocalChecked()->);
-                    break;
+                    return true;
                 }
                 if (val->IsNumber()) {
                     dest = QVariant::fromValue(val->ToNumber(_v8Context.Get(_v8Isolate)).ToLocalChecked()->Value());
-                    break;
+                    return true;
                 }
                 {
                     QObject* obj = ScriptObjectV8Proxy::unwrap(v8Val);
                     if (obj) {
                         dest = QVariant::fromValue(obj);
-                        break;
+                        return true;
                     }
                 }
                 {
                     QVariant var = ScriptVariantV8Proxy::unwrap(v8Val);
                     if (var.isValid()) {
                         dest = var;
-                        break;
+                        return true;
                     }
                 }
                 errorMessage = QString() + "Conversion to variant failed: " + QString(*v8::String::Utf8Value(_v8Isolate, val->ToDetailString(getConstContext()).ToLocalChecked()))
-                                       + " Destination type: " + QMetaType::typeName(destTypeId);
+                                       + " Destination type: " + QMetaType::typeName(destTypeId) + " Value details: " + scriptValueDebugDetailsV8(v8Val);
                 qDebug() << errorMessage;
-                break;
+                return false;
             default:
                 // check to see if this is a pointer to a QObject-derived object
                 if (QMetaType::typeFlags(destTypeId) & (QMetaType::PointerToQObject | QMetaType::TrackingPointerToQObject)) {

libraries/script-engine/src/v8/ScriptProgramV8Wrapper.cpp

@@ -32,7 +32,14 @@ ScriptSyntaxCheckResultPointer ScriptProgramV8Wrapper::checkSyntax() {
 }
 
 bool ScriptProgramV8Wrapper::compile() {
+    if (_isCompiled) {
+        // V8TODO is there a case where source code changes later and needs to be compiled again?
+        // V8TODO is same program used from multiple isolates
+        return true;
+    }
     auto isolate = _engine->getIsolate();
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     auto context = _engine->getContext();
     v8::Context::Scope contextScope(context);
@@ -48,6 +55,7 @@ bool ScriptProgramV8Wrapper::compile() {
         qDebug() << "Script compilation succesful: " << _url;
         _compileResult = ScriptSyntaxCheckResultV8Wrapper(ScriptSyntaxCheckResult::Valid);
         _value = V8ScriptProgram(isolate, script);
+        _isCompiled = true;
         return true;
     }
     qDebug() << "Script compilation failed: " << _url;

libraries/script-engine/src/v8/ScriptValueV8Wrapper.cpp

@@ -61,7 +61,8 @@ V8ScriptValue ScriptValueV8Wrapper::fullUnwrap(ScriptEngineV8* engine, const Scr
 
 ScriptValue ScriptValueV8Wrapper::call(const ScriptValue& thisObject, const ScriptValueList& args) {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     V8ScriptValue v8This = fullUnwrap(thisObject);
@@ -89,7 +90,8 @@ ScriptValue ScriptValueV8Wrapper::call(const ScriptValue& thisObject, const Scri
 
 ScriptValue ScriptValueV8Wrapper::call(const ScriptValue& thisObject, const ScriptValue& arguments) {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     V8ScriptValue v8This = fullUnwrap(thisObject);
@@ -113,7 +115,8 @@ ScriptValue ScriptValueV8Wrapper::call(const ScriptValue& thisObject, const Scri
 
 ScriptValue ScriptValueV8Wrapper::construct(const ScriptValueList& args) {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     Q_ASSERT(args.length() <= Q_METAMETHOD_INVOKE_MAX_ARGS);
@@ -139,7 +142,8 @@ ScriptValue ScriptValueV8Wrapper::construct(const ScriptValueList& args) {
 
 ScriptValue ScriptValueV8Wrapper::construct(const ScriptValue& arguments) {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     // V8TODO I'm not sure in what format arguments are yet, backtrace will show how it is used
@@ -170,7 +174,8 @@ ScriptValueIteratorPointer ScriptValueV8Wrapper::newIterator() const {
 
 ScriptValue ScriptValueV8Wrapper::property(const QString& name, const ScriptValue::ResolveFlags &mode) const {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(_engine->getIsolate());
+    v8::Isolate::Scope isolateScope(_engine->getIsolate());
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     if (_value.constGet()->IsObject()) {
@@ -181,6 +186,13 @@ ScriptValue ScriptValueV8Wrapper::property(const QString& name, const ScriptValu
         if (object->Get(_value.constGetContext(), key).ToLocal(&resultLocal)) {
             V8ScriptValue result(_engine->getIsolate(), resultLocal);
             return ScriptValue(new ScriptValueV8Wrapper(_engine, std::move(result)));
+        } else {
+            QString parentValueQString("");
+            v8::Local<v8::String> parentValueString;
+            if (_value.constGet()->ToDetailString(_engine->getContext()).ToLocal(&parentValueString)) {
+                QString(*v8::String::Utf8Value(isolate, parentValueString));
+            }
+            qDebug() << "Failed to get property, parent of value: " << name << ", parent type: " << QString(*v8::String::Utf8Value(isolate, _value.constGet()->TypeOf(isolate))) << " parent value: " << parentValueQString;
         }
     }
     qDebug() << "Failed to get property, parent of value: " << name << " is not a V8 object, reported type: " << QString(*v8::String::Utf8Value(isolate, _value.constGet()->TypeOf(isolate)));
@@ -194,7 +206,8 @@ ScriptValue ScriptValueV8Wrapper::property(const QString& name, const ScriptValu
 
 ScriptValue ScriptValueV8Wrapper::property(quint32 arrayIndex, const ScriptValue::ResolveFlags& mode) const {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     if (_value.constGet()->IsObject()) {
@@ -215,7 +228,8 @@ ScriptValue ScriptValueV8Wrapper::property(quint32 arrayIndex, const ScriptValue
 
 void ScriptValueV8Wrapper::setData(const ScriptValue& value) {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     V8ScriptValue unwrapped = fullUnwrap(value);
@@ -225,7 +239,8 @@ void ScriptValueV8Wrapper::setData(const ScriptValue& value) {
 void ScriptValueV8Wrapper::setProperty(const QString& name, const ScriptValue& value, const ScriptValue::PropertyFlags& flags) {
     Q_ASSERT(flags != ScriptValue::PropertyGetter || flags != ScriptValue::PropertySetter);
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(_engine->getIsolate());
+    v8::Isolate::Scope isolateScope(_engine->getIsolate());
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     V8ScriptValue unwrapped = fullUnwrap(value);
@@ -256,7 +271,8 @@ void ScriptValueV8Wrapper::setProperty(const QString& name, const ScriptValue& v
 
 void ScriptValueV8Wrapper::setProperty(quint32 arrayIndex, const ScriptValue& value, const ScriptValue::PropertyFlags& flags) {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     V8ScriptValue unwrapped = fullUnwrap(value);
@@ -275,7 +291,8 @@ void ScriptValueV8Wrapper::setProperty(quint32 arrayIndex, const ScriptValue& va
 
 void ScriptValueV8Wrapper::setPrototype(const ScriptValue& prototype) {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     ScriptValueV8Wrapper* unwrappedPrototype = unwrap(prototype);
@@ -294,7 +311,8 @@ void ScriptValueV8Wrapper::setPrototype(const ScriptValue& prototype) {
 
 bool ScriptValueV8Wrapper::strictlyEquals(const ScriptValue& other) const {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     ScriptValueV8Wrapper* unwrappedOther = unwrap(other);
@@ -303,7 +321,8 @@ bool ScriptValueV8Wrapper::strictlyEquals(const ScriptValue& other) const {
 
 bool ScriptValueV8Wrapper::toBool() const {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     return _value.constGet()->ToBoolean(_engine->getIsolate())->Value();
@@ -311,7 +330,8 @@ bool ScriptValueV8Wrapper::toBool() const {
 
 qint32 ScriptValueV8Wrapper::toInt32() const {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     v8::Local<v8::Integer> integer;
@@ -323,7 +343,8 @@ qint32 ScriptValueV8Wrapper::toInt32() const {
 
 double ScriptValueV8Wrapper::toInteger() const {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     v8::Local<v8::Integer> integer;
@@ -335,7 +356,8 @@ double ScriptValueV8Wrapper::toInteger() const {
 
 double ScriptValueV8Wrapper::toNumber() const {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     v8::Local<v8::Number> number;
@@ -347,7 +369,8 @@ double ScriptValueV8Wrapper::toNumber() const {
 
 QString ScriptValueV8Wrapper::toString() const {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     v8::String::Utf8Value string(_engine->getIsolate(), _value.constGet());
@@ -357,7 +380,8 @@ QString ScriptValueV8Wrapper::toString() const {
 
 quint16 ScriptValueV8Wrapper::toUInt16() const {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     v8::Local<v8::Uint32> integer;
@@ -369,7 +393,8 @@ quint16 ScriptValueV8Wrapper::toUInt16() const {
 
 quint32 ScriptValueV8Wrapper::toUInt32() const {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     v8::Local<v8::Uint32> integer;
@@ -401,7 +426,8 @@ QObject* ScriptValueV8Wrapper::toQObject() const {
 
 bool ScriptValueV8Wrapper::equals(const ScriptValue& other) const {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     ScriptValueV8Wrapper* unwrappedOther = unwrap(other);
@@ -420,7 +446,8 @@ bool ScriptValueV8Wrapper::equals(const ScriptValue& other) const {
 
 bool ScriptValueV8Wrapper::isArray() const {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     return _value.constGet()->IsArray();
@@ -428,7 +455,8 @@ bool ScriptValueV8Wrapper::isArray() const {
 
 bool ScriptValueV8Wrapper::isBool() const {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     return _value.constGet()->IsBoolean();
@@ -446,7 +474,8 @@ bool ScriptValueV8Wrapper::isError() const {
 
 bool ScriptValueV8Wrapper::isFunction() const {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     return _value.constGet()->IsFunction();
@@ -454,7 +483,8 @@ bool ScriptValueV8Wrapper::isFunction() const {
 
 bool ScriptValueV8Wrapper::isNumber() const {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     return _value.constGet()->IsNumber();
@@ -462,7 +492,8 @@ bool ScriptValueV8Wrapper::isNumber() const {
 
 bool ScriptValueV8Wrapper::isNull() const {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     return _value.constGet()->IsNull();
@@ -470,7 +501,8 @@ bool ScriptValueV8Wrapper::isNull() const {
 
 bool ScriptValueV8Wrapper::isObject() const {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     return _value.constGet()->IsObject();
@@ -478,7 +510,8 @@ bool ScriptValueV8Wrapper::isObject() const {
 
 bool ScriptValueV8Wrapper::isString() const {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     return _value.constGet()->IsString();
@@ -486,26 +519,30 @@ bool ScriptValueV8Wrapper::isString() const {
 
 bool ScriptValueV8Wrapper::isUndefined() const {
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     return _value.constGet()->IsUndefined();
 }
 
 bool ScriptValueV8Wrapper::isValid() const {
-    //V8TODO
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
+    if (_value.constGet()->IsNullOrUndefined()) {
+        return false;
+    }
     return true;
-    //return _value.constGet()->IsValid();
 }
 
 bool ScriptValueV8Wrapper::isVariant() const {
     //V8TODO
     auto isolate = _engine->getIsolate();
-    Q_ASSERT(isolate->IsCurrent());
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolateScope(isolate);
     v8::HandleScope handleScope(isolate);
     v8::Context::Scope contextScope(_engine->getContext());
     return false;