Work on fixing V8 memory leaks, memory leak test

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);
 }
 

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);
+}

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

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

@@ -37,6 +37,8 @@ public: // construction
                            const v8::Local<v8::Context> context, ScriptContextPointer parent);
     ScriptContextV8Wrapper(ScriptEngineV8* engine, const v8::PropertyCallbackInfo<v8::Value> *propertyCallbackInfo,
                            const v8::Local<v8::Context> 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<v8::Context> context);
+    virtual ~ScriptFunctionContextV8Wrapper() {_context.Reset();};
 
 public:  // ScriptFunctionContext implementation
     virtual QString fileName() const override;

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<QObject*>();
             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<QObject*>();
                 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);

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<v8::Object> 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<ScriptObjectV8Proxy>& 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);
         }

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

@@ -108,6 +108,8 @@ public:
     static void v8Get(v8::Local<v8::Name> name, const v8::PropertyCallbackInfo<v8::Value>& info);
     static void v8Set(v8::Local<v8::Name> name, v8::Local<v8::Value> value_obj, const v8::PropertyCallbackInfo<v8::Value>& info);
     static void v8GetPropertyNames(const v8::PropertyCallbackInfo<v8::Array>& info);
+    // This gets called when script-owned object is being garbage-collected
+    static void weakHandleCallback(const v8::WeakCallbackInfo<ScriptObjectV8Proxy> &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<v8::Object> _v8Object;
+    v8::Persistent<v8::Object> _v8Object;
     int pointerCorruptionTest = 12345678;
 
     Q_DISABLE_COPY(ScriptObjectV8Proxy)

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",

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() {