diff --git a/libraries/entities/src/EntityScriptingInterface.cpp b/libraries/entities/src/EntityScriptingInterface.cpp index 2c8b223053..8873fd5917 100644 --- a/libraries/entities/src/EntityScriptingInterface.cpp +++ b/libraries/entities/src/EntityScriptingInterface.cpp @@ -1645,8 +1645,11 @@ bool EntityScriptingInterface::getServerScriptStatus(const QUuid& entityID, cons } connect(request, &GetScriptStatusRequest::finished, manager, [callback](GetScriptStatusRequest* request) mutable { - QString statusString = EntityScriptStatus_::valueToKey(request->getStatus()); auto engine = callback.engine(); + // V8TODO: it seems to sometimes be called on a wrong thread, reading to script engine crashes. I added an assert for now + Q_ASSERT(QThread::currentThread() == engine->thread()); + Q_ASSERT(QThread::currentThread() == engine->manager()->thread()); + QString statusString = EntityScriptStatus_::valueToKey(request->getStatus()); ScriptValueList args { engine->newValue(request->getResponseReceived()), engine->newValue(request->getIsRunning()), engine->newValue(statusString.toLower()), engine->newValue(request->getErrorInfo()) }; callback.call(ScriptValue(), args); request->deleteLater(); diff --git a/libraries/script-engine/src/v8/ScriptEngineV8.cpp b/libraries/script-engine/src/v8/ScriptEngineV8.cpp index 94592d3997..3206cde1f1 100644 --- a/libraries/script-engine/src/v8/ScriptEngineV8.cpp +++ b/libraries/script-engine/src/v8/ScriptEngineV8.cpp @@ -846,7 +846,7 @@ ScriptValue ScriptEngineV8::evaluateInClosure(const ScriptValue& _closure, v8::Local closureGlobal; ScriptValueV8Wrapper* unwrappedClosure; ScriptProgramV8Wrapper* unwrappedProgram; - v8::Local oldContext = getContext(); + //v8::Local oldContext = getContext(); { v8::Context::Scope contextScope(getContext()); @@ -1180,7 +1180,9 @@ ScriptContextV8Pointer ScriptEngineV8::pushContext(v8::Local contex _contexts.append(std::make_shared(this, context, ScriptContextPointer())); v8::Context::Scope contextScope(context); static volatile int debug_context_id = 1; - context->Global()->Set(context, v8::String::NewFromUtf8(_v8Isolate, "debug_context_id").ToLocalChecked(), v8::Integer::New(_v8Isolate, debug_context_id)); + if (!context->Global()->Set(context, v8::String::NewFromUtf8(_v8Isolate, "debug_context_id").ToLocalChecked(), v8::Integer::New(_v8Isolate, debug_context_id)).FromMaybe(false)) { + Q_ASSERT(false); + } debug_context_id++; return _contexts.last(); } diff --git a/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp b/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp index c2df964dab..a2f217c4b6 100644 --- a/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp +++ b/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp @@ -504,12 +504,12 @@ void ScriptObjectV8Proxy::v8GetPropertyNames(const v8::PropertyCallbackInfo_engine, objectV8); - uint id; + //uint id; v8::Local properties = proxy->getPropertyNames(); v8::Local objectProperties; uint32_t propertiesLength = properties->Length(); if (info.This()->GetInternalField(2).As()->GetPropertyNames(context).ToLocal(&objectProperties)) { - for (int n = 0; n < objectProperties->Length(); n++) { + for (uint32_t n = 0; n < objectProperties->Length(); n++) { if(!properties->Set(context, propertiesLength+n, objectProperties->Get(context, n).ToLocalChecked()).FromMaybe(false)) { qDebug(scriptengine) << "ScriptObjectV8Proxy::v8GetPropertyNames: Cannot add member name"; } @@ -1262,7 +1262,7 @@ int ScriptSignalV8Proxy::qt_metacall(QMetaObject::Call call, int id, void** argu } //}); - _totalCallTime_s += callTimer.elapsed() / 1000.0; + _totalCallTime_s += callTimer.elapsed() / 1000.0f; return -1; } diff --git a/libraries/script-engine/src/v8/ScriptValueV8Wrapper.cpp b/libraries/script-engine/src/v8/ScriptValueV8Wrapper.cpp index 135b44bbdd..24be68dd9e 100644 --- a/libraries/script-engine/src/v8/ScriptValueV8Wrapper.cpp +++ b/libraries/script-engine/src/v8/ScriptValueV8Wrapper.cpp @@ -550,6 +550,7 @@ QObject* ScriptValueV8Wrapper::toQObject() const { return dest.value(); } else { Q_ASSERT(false); + return nullptr; } } else { Q_ASSERT(false); diff --git a/libraries/script-engine/src/v8/V8Types.h b/libraries/script-engine/src/v8/V8Types.h index 952fbcefee..018f59f8c9 100644 --- a/libraries/script-engine/src/v8/V8Types.h +++ b/libraries/script-engine/src/v8/V8Types.h @@ -23,16 +23,30 @@ template class V8ScriptValueTemplate { public: V8ScriptValueTemplate() = delete; + + /*enum class Ownership { + Script = 0, + Value = 1 + };*/ //V8ScriptValueTemplate(v8::Isolate *isolate, v8::Local value) : _isolate(isolate) { //_value.reset(v8::UniquePersistent::New(_isolate, value)); //}; - V8ScriptValueTemplate(ScriptEngineV8 *engine, const v8::Local value) : _engine(engine) { + // V8TODO: Persistent handles need to be constructed and destructed in the same thread + // Adding asserts might be a good idea + V8ScriptValueTemplate(ScriptEngineV8 *engine, const v8::Local value/*, V8ScriptValueTemplate::Ownership ownership*/) : _engine(engine) { v8::Locker locker(_engine->getIsolate()); v8::Isolate::Scope isolateScope(_engine->getIsolate()); v8::HandleScope handleScope(_engine->getIsolate()); v8::Context::Scope(_engine->getContext()); - _value.reset(new v8::Persistent(_engine->getIsolate(), value)); - //_value.reset(new v8::UniquePersistent(_engine->getIsolate(), value)); + /*if (ownership == V8ScriptValueTemplate::Ownership::Script) { + _value.reset(new v8::WeakPersistent(_engine->getIsolate(), value)); + return; + } else if (ownership == V8ScriptValueTemplate::Ownership::Value) { + _value.reset(new v8::UniquePersistent(_engine->getIsolate(), value)); + return; + } + Q_ASSERT(false);*/ + _value.reset(new v8::UniquePersistent(_engine->getIsolate(), value)); }; V8ScriptValueTemplate& operator= (const V8ScriptValueTemplate &source) { @@ -41,7 +55,7 @@ public: v8::HandleScope handleScope(_engine->getIsolate()); v8::Context::Scope(_engine->getContext()); _engine = source.getEngine(); - _value.reset(new v8::Persistent(_engine->getIsolate(), source.constGet())); + _value.reset(new v8::UniquePersistent(_engine->getIsolate(), source.constGet())); //_value.reset(new v8::UniquePersistent(_engine->getIsolate(), source.constGet())); return *this; }; @@ -52,7 +66,7 @@ public: v8::HandleScope handleScope(_engine->getIsolate()); v8::Context::Scope(_engine->getContext()); //_value.reset(new v8::UniquePersistent(_engine->getIsolate(), v8::Local())); - _value.reset(new v8::Persistent(_engine->getIsolate(), v8::Local())); + _value.reset(new v8::UniquePersistent(_engine->getIsolate(), v8::Local())); }; V8ScriptValueTemplate(const V8ScriptValueTemplate &copied) : _engine(copied.getEngine()) { @@ -61,7 +75,7 @@ public: v8::HandleScope handleScope(_engine->getIsolate()); v8::Context::Scope(_engine->getContext()); //_value.reset(new v8::UniquePersistent(_engine->getIsolate(), copied.constGet())); - _value.reset(new v8::Persistent(_engine->getIsolate(), copied.constGet())); + _value.reset(new v8::UniquePersistent(_engine->getIsolate(), copied.constGet())); } v8::Local get() { @@ -106,11 +120,19 @@ public: void reset(v8::Isolate *isolate, v8::Local) { Q_ASSERT(false); }; + // V8TODO: add thread safe destructors to all objects that have persistent handles + ~V8ScriptValueTemplate() { + v8::Locker locker(_engine->getIsolate()); + v8::Isolate::Scope isolateScope(_engine->getIsolate()); + v8::HandleScope handleScope(_engine->getIsolate()); + //v8::Context::Scope(_engine->getContext()); + _value->Reset(); + } private: // std::shared_ptr> _value; // V8TODO: Persistent needs reset to release object? It may cause memory leaks here - std::shared_ptr> _value; + std::shared_ptr> _value; // V8TODO: maybe make it weak // does it need , CopyablePersistentTraits? // V8TODO: is context needed at all? diff --git a/scripts/system/controllers/controllerModules/inEditMode.js b/scripts/system/controllers/controllerModules/inEditMode.js index 5150e84c72..d553bf5714 100644 --- a/scripts/system/controllers/controllerModules/inEditMode.js +++ b/scripts/system/controllers/controllerModules/inEditMode.js @@ -203,12 +203,13 @@ Script.include("/~/system/libraries/utils.js"); if ((controllerData.triggerClicks[this.hand] === 0 && controllerData.secondaryValues[this.hand] === 0)) { var stopRunning = false; - controllerData.nearbyOverlayIDs[this.hand].forEach(function(overlayID) { + // V8TODO: check if this doesn't break anything + /*controllerData.nearbyOverlayIDs[this.hand].forEach(function(overlayID) { var overlayName = Overlays.getProperty(overlayID, "name"); if (overlayName === "KeyboardAnchor") { stopRunning = true; } - }); + });*/ if (stopRunning) { return this.exitModule();