diff --git a/interface/src/ui/InteractiveWindow.cpp b/interface/src/ui/InteractiveWindow.cpp index f5f07fe918..258ba6784b 100644 --- a/interface/src/ui/InteractiveWindow.cpp +++ b/interface/src/ui/InteractiveWindow.cpp @@ -106,6 +106,7 @@ void registerInteractiveWindowMetaType(ScriptEngine* engine) { } ScriptValue interactiveWindowPointerToScriptValue(ScriptEngine* engine, const InteractiveWindowPointer& in) { + // V8TODO: is ScriptOwnership safe here? return engine->newQObject(in, ScriptEngine::ScriptOwnership); } diff --git a/libraries/script-engine/src/ScriptManagerScriptingInterface.cpp b/libraries/script-engine/src/ScriptManagerScriptingInterface.cpp index 04dc992b5e..3c4994d1a5 100644 --- a/libraries/script-engine/src/ScriptManagerScriptingInterface.cpp +++ b/libraries/script-engine/src/ScriptManagerScriptingInterface.cpp @@ -63,3 +63,7 @@ QVariantMap ScriptManagerScriptingInterface::getMemoryUsageStatistics() { map.insert("usedGlobalHandlesSize", QVariant((qulonglong)(statistics.usedGlobalHandlesSize))); return map; } + +ScriptValue ScriptManagerScriptingInterface::createGarbageCollectorDebuggingObject() { + return _manager->engine()->newQObject(new TestQObject, ScriptEngine::ScriptOwnership); +} \ No newline at end of file diff --git a/libraries/script-engine/src/ScriptManagerScriptingInterface.h b/libraries/script-engine/src/ScriptManagerScriptingInterface.h index 507feaec0f..8d37b2e7a3 100644 --- a/libraries/script-engine/src/ScriptManagerScriptingInterface.h +++ b/libraries/script-engine/src/ScriptManagerScriptingInterface.h @@ -49,6 +49,13 @@ * @property {Script.ResourceBuckets} ExternalPaths - External resource buckets. */ +// V8TODO: this should be moved to somewhere test-related +class TestQObject : public QObject { + Q_OBJECT +public: + Q_INVOKABLE virtual void testMethod() { qDebug() << "TestQObject::testMethod"; }; +}; + class ScriptManagerScriptingInterface : public QObject { Q_OBJECT public: @@ -503,6 +510,13 @@ public: */ Q_INVOKABLE QVariantMap getMemoryUsageStatistics(); + /**jsdoc + * Create test object for garbage collector debugging. + * @function Script.createGarbageCollectorDebuggingObject() + * @Returns Test object. + */ + Q_INVOKABLE ScriptValue createGarbageCollectorDebuggingObject(); + signals: /**jsdoc diff --git a/libraries/script-engine/src/v8/ScriptContextV8Wrapper.h b/libraries/script-engine/src/v8/ScriptContextV8Wrapper.h index ddb14f1d4c..255ec88be9 100644 --- a/libraries/script-engine/src/v8/ScriptContextV8Wrapper.h +++ b/libraries/script-engine/src/v8/ScriptContextV8Wrapper.h @@ -37,6 +37,8 @@ public: // construction const v8::Local context, ScriptContextPointer parent); ScriptContextV8Wrapper(ScriptEngineV8* engine, const v8::PropertyCallbackInfo *propertyCallbackInfo, const v8::Local context, ScriptContextPointer parent); + virtual ~ScriptContextV8Wrapper() {_context.Reset();} + static ScriptContextV8Wrapper* unwrap(ScriptContext* val); public: // ScriptContext implementation @@ -68,6 +70,7 @@ class ScriptFunctionContextV8Wrapper final : public ScriptFunctionContext { public: // construction //V8TODO ScriptFunctionContextV8Wrapper(ScriptEngineV8* engine, const v8::Local context); + virtual ~ScriptFunctionContextV8Wrapper() {_context.Reset();}; public: // ScriptFunctionContext implementation virtual QString fileName() const override; diff --git a/libraries/script-engine/src/v8/ScriptEngineV8_cast.cpp b/libraries/script-engine/src/v8/ScriptEngineV8_cast.cpp index 1420fb067f..fd62047b25 100644 --- a/libraries/script-engine/src/v8/ScriptEngineV8_cast.cpp +++ b/libraries/script-engine/src/v8/ScriptEngineV8_cast.cpp @@ -750,6 +750,7 @@ V8ScriptValue ScriptEngineV8::castVariantToValue(const QVariant& val) { case QMetaType::QObjectStar: { QObject* obj = val.value(); if (obj == nullptr) return V8ScriptValue(this, v8::Null(_v8Isolate)); + //V8TODO: what should be the ownership in this case? return ScriptObjectV8Proxy::newQObject(this, obj); } case QMetaType::QDateTime: @@ -768,9 +769,10 @@ V8ScriptValue ScriptEngineV8::castVariantToValue(const QVariant& val) { if (QMetaType::typeFlags(valTypeId) & (QMetaType::PointerToQObject | QMetaType::TrackingPointerToQObject)) { QObject* obj = val.value(); if (obj == nullptr) return V8ScriptValue(this, v8::Null(_v8Isolate)); + //V8TODO: what should be the ownership in this case? return ScriptObjectV8Proxy::newQObject(this, obj); } - // have we set a prototype'd variant? + // have we set a prototyped variant? { _customTypeProtect.lockForRead(); CustomPrototypeMap::const_iterator lookup = _customPrototypes.find(valTypeId); diff --git a/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp b/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp index 9ab11b99a5..005bc3e7b3 100644 --- a/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp +++ b/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp @@ -146,7 +146,9 @@ V8ScriptValue ScriptObjectV8Proxy::newQObject(ScriptEngineV8* engine, QObject* o if (lookupV8 != enginePtr->_qobjectWrapperMapV8.end()) { enginePtr->_qobjectWrapperMapV8.erase(lookupV8); } + qDebug() << "ScriptObjectV8Proxy::newQObject object deleted, object count: " << enginePtr->_qobjectWrapperMapV8.size(); }); + //qDebug() << "ScriptObjectV8Proxy::newQObject object count: " << engine->_qobjectWrapperMapV8.size(); } return V8ScriptValue(engine, proxy.get()->toV8Value()); @@ -373,6 +375,10 @@ void ScriptObjectV8Proxy::investigate() { v8::Local propertiesObject = v8::Object::New(_engine->getIsolate()); v8Object->SetInternalField(2, propertiesObject); _v8Object.Reset(_engine->getIsolate(), v8Object); + if (_ownsObject) { + qDebug(scriptengine_v8) << "ScriptObjectV8Proxy::investigate SetWeak"; + _v8Object.SetWeak(this, weakHandleCallback, v8::WeakCallbackType::kParameter); + } // Add all the methods objects as properties - this allows adding properties to a given method later. Is used by Script.request. // V8TODO: Should these be deleted when the script-owned object is destroyed? It needs checking if script-owned objects will be garbage-collected, or will self-referencing prevent it. @@ -383,7 +389,13 @@ void ScriptObjectV8Proxy::investigate() { Q_ASSERT(false); } } +} +void ScriptObjectV8Proxy::weakHandleCallback(const v8::WeakCallbackInfo& info) { + //V8TODO: does the object need to be moved to script engine thread? + //V8TODO: why does this never get called? + qDebug(scriptengine_v8) << "ScriptObjectV8Proxy::weakHandleCallback"; + info.GetParameter()->_object->deleteLater(); } QString ScriptObjectV8Proxy::name() const { @@ -690,6 +702,7 @@ V8ScriptValue ScriptObjectV8Proxy::property(const V8ScriptValue& object, const V //V8TODO ScriptEngine::ExcludeDeleteLater | ScriptEngine::PreferExistingWrapperObject; // It's not necessarily new, newQObject looks for it first in object wrapper map + // V8TODO: won't ScriptEngine::ScriptOwnership cause trouble here? return ScriptObjectV8Proxy::newQObject(_engine, proxy, ScriptEngine::ScriptOwnership, options); //return _engine->newQObject(proxy, ScriptEngine::ScriptOwnership, options); } diff --git a/libraries/script-engine/src/v8/ScriptObjectV8Proxy.h b/libraries/script-engine/src/v8/ScriptObjectV8Proxy.h index 0513401f02..7151e5010a 100644 --- a/libraries/script-engine/src/v8/ScriptObjectV8Proxy.h +++ b/libraries/script-engine/src/v8/ScriptObjectV8Proxy.h @@ -108,6 +108,8 @@ public: static void v8Get(v8::Local name, const v8::PropertyCallbackInfo& info); static void v8Set(v8::Local name, v8::Local value_obj, const v8::PropertyCallbackInfo& info); static void v8GetPropertyNames(const v8::PropertyCallbackInfo& info); + // This gets called when script-owned object is being garbage-collected + static void weakHandleCallback(const v8::WeakCallbackInfo &info); private: // implementation void investigate(); @@ -129,7 +131,7 @@ private: // storage // V8TODO Maybe depending on object ownership it should be different type of handles? For example weak persistent would allow // script engine-owned objects to be garbage collected. This will also need adding a garbage collector callback from V8 // to let proxy know that it is not valid anymore - v8::UniquePersistent _v8Object; + v8::Persistent _v8Object; int pointerCorruptionTest = 12345678; Q_DISABLE_COPY(ScriptObjectV8Proxy) diff --git a/scripts/defaultScripts.js b/scripts/defaultScripts.js index dec95dd936..ac047ecf78 100644 --- a/scripts/defaultScripts.js +++ b/scripts/defaultScripts.js @@ -40,7 +40,8 @@ var DEFAULT_SCRIPTS_COMBINED = [ "system/onEscape.js", "system/onFirstRun.js", "system/appreciate/appreciate_app.js", - "system/places/places.js" + "system/places/places.js", + "developer/debugging/scriptMemoryReport.js" ]; var DEFAULT_SCRIPTS_SEPARATE = [ "system/controllers/controllerScripts.js", diff --git a/scripts/system/controllers/controllerScripts.js b/scripts/system/controllers/controllerScripts.js index c9d0555c89..6eabce512a 100644 --- a/scripts/system/controllers/controllerScripts.js +++ b/scripts/system/controllers/controllerScripts.js @@ -41,6 +41,9 @@ var CONTOLLER_SCRIPTS = [ "controllerModules/trackedHandTablet.js" ]; +Script.include("../../developer/debugging/scriptMemoryReport.js"); +//Script.include("developer/debugging/scriptMemoryReport.js"); + var DEBUG_MENU_ITEM = "Debug defaultScripts.js"; function runDefaultsTogether() {