Better implementation of persistent values

The old code never reset values when the engine got deleted.
This is however required if we want to offer a public API
that is safe to use (with QJSValue).

Change-Id: I9754b07b35922a824733dfb8488f3d4998905da7
Reviewed-by: Simon Hausmann <simon.hausmann@digia.com>

diff --git a/src/qml/qml/v4vm/qv4mm.cpp b/src/qml/qml/v4vm/qv4mm.cpp
index 6fe3eab..d756cf5 100644 (file)
--- a/src/qml/qml/v4vm/qv4mm.cpp
+++ b/src/qml/qml/v4vm/qv4mm.cpp
@@ -130,6 +130,7 @@ bool operator<(const MemoryManager::Data::Chunk &a, const MemoryManager::Data::C
 MemoryManager::MemoryManager()
     : m_d(new Data(true))
     , m_contextList(0)
+    , m_persistentValues(0)
 {
     setEnableGC(true);
 #ifdef V4_USE_VALGRIND
@@ -240,6 +241,22 @@ void MemoryManager::mark()
     for (QHash<Managed *, uint>::const_iterator it = m_d->protectedObject.begin(); it != m_d->protectedObject.constEnd(); ++it)
         it.key()->mark();
 
+    PersistentValuePrivate *persistent = m_persistentValues;
+    PersistentValuePrivate **last = &m_persistentValues;
+    while (persistent) {
+        if (!persistent->refcount) {
+            *last = persistent->next;
+            PersistentValuePrivate *n = persistent->next;
+            delete persistent;
+            persistent = n;
+            continue;
+        }
+        if (Managed *m = persistent->value.asManaged())
+            m->mark();
+        last = &persistent->next;
+        persistent = persistent->next;
+    }
+
     // push all caller saved registers to the stack, so we can find the objects living in these registers
 #if COMPILER(MSVC)
 #  if CPU(X86_64)
@@ -380,6 +397,16 @@ void MemoryManager::setEnableGC(bool enableGC)
 
 MemoryManager::~MemoryManager()
 {
+    PersistentValuePrivate *persistent = m_persistentValues;
+    while (persistent) {
+        if (Managed *m = persistent->value.asManaged())
+            persistent->value = Value::undefinedValue();
+        persistent->engine = 0;
+        PersistentValuePrivate *n = persistent->next;
+        persistent->next = 0;
+        persistent = n;
+    }
+
     sweep();
 }
 

diff --git a/src/qml/qml/v4vm/qv4mm_p.h b/src/qml/qml/v4vm/qv4mm_p.h
index 0848c99..f51331a 100644 (file)
--- a/src/qml/qml/v4vm/qv4mm_p.h
+++ b/src/qml/qml/v4vm/qv4mm_p.h
@@ -136,6 +136,8 @@ private:
 protected:
     QScopedPointer<Data> m_d;
     ExecutionContext *m_contextList;
+public:
+    PersistentValuePrivate *m_persistentValues;
 };
 
 inline ExecutionContext *MemoryManager::allocContext(uint size)

diff --git a/src/qml/qml/v4vm/qv4runtime.cpp b/src/qml/qml/v4vm/qv4runtime.cpp
index 6c587fd..6f1df36 100644 (file)
--- a/src/qml/qml/v4vm/qv4runtime.cpp
+++ b/src/qml/qml/v4vm/qv4runtime.cpp
@@ -119,9 +119,9 @@ QString numberToString(double num, int radix = 10)
 }
 
 Exception::Exception(ExecutionContext *throwingContext, const Value &exceptionValue)
+    : exception(PersistentValue(throwingContext->engine, exceptionValue))
 {
     this->throwingContext = throwingContext->engine->current;
-    this->exception = PersistentValue(throwingContext->engine->memoryManager, exceptionValue);
     accepted = false;
 }
 

diff --git a/src/qml/qml/v4vm/qv4value.cpp b/src/qml/qml/v4vm/qv4value.cpp
index c7a2285..6112c37 100644 (file)
--- a/src/qml/qml/v4vm/qv4value.cpp
+++ b/src/qml/qml/v4vm/qv4value.cpp
@@ -167,45 +167,50 @@ Value Value::property(ExecutionContext *ctx, String *name) const
     return isObject() ? objectValue()->get(ctx, name) : undefinedValue();
 }
 
-PersistentValue::PersistentValue()
-    : m_memoryManager(0)
-    , m_value(Value::undefinedValue())
-{
-}
 
-PersistentValue::PersistentValue(MemoryManager *mm, const Value &val)
-    : m_memoryManager(mm)
-    , m_value(val)
+PersistentValue::PersistentValue(ExecutionEngine *e, const Value &val)
+    : d(PersistentValuePrivate::create(e, val))
 {
-    assert(mm);
-    if (Managed *m = asManaged())
-        m_memoryManager->protect(m);
 }
 
 PersistentValue::PersistentValue(const PersistentValue &other)
-    : m_memoryManager(other.m_memoryManager)
-    , m_value(other.m_value)
+    : d(other.d)
 {
-    if (Managed *m = asManaged())
-        m_memoryManager->protect(m);
+    d->ref();
 }
 
 PersistentValue &PersistentValue::operator=(const PersistentValue &other)
 {
-    if (this == &other)
+    if (d == other.d)
         return *this;
-    if (Managed *m = asManaged())
-        m_memoryManager->unprotect(m);
-    m_memoryManager = other.m_memoryManager;
-    m_value = other.m_value;
-    if (Managed *m = asManaged())
-        m_memoryManager->protect(m);
+
+    // the memory manager cleans up those with a refcount of 0
+    d->deref();
+    d = other.d;
+    d->ref();
 }
 
 PersistentValue::~PersistentValue()
 {
-    if (Managed *m = asManaged())
-        m_memoryManager->unprotect(m);
+    d->deref();
+}
+
+PersistentValuePrivate *PersistentValuePrivate::create(ExecutionEngine *e, const Value &v)
+{
+    PersistentValuePrivate *d = new PersistentValuePrivate;
+    d->engine = e;
+    d->next = e->memoryManager->m_persistentValues;
+    e->memoryManager->m_persistentValues = d;
+    d->value = v;
+    d->refcount = 1;
+}
+
+void PersistentValuePrivate::deref()
+{
+    // if engine is not 0, they are registered with the memory manager
+    // and will get cleaned up in the next gc run
+    if (!--refcount && !engine)
+        delete this;
 }
 
 

diff --git a/src/qml/qml/v4vm/qv4value_p.h b/src/qml/qml/v4vm/qv4value_p.h
index 09539cc..281e6a7 100644 (file)
--- a/src/qml/qml/v4vm/qv4value_p.h
+++ b/src/qml/qml/v4vm/qv4value_p.h
@@ -544,24 +544,33 @@ inline Value Managed::call(ExecutionContext *context, const Value &thisObject, V
     return vtbl->call(this, context, thisObject, args, argc);
 }
 
+struct PersistentValuePrivate
+{
+    Value value;
+    int refcount;
+    ExecutionEngine *engine;
+    PersistentValuePrivate *next;
+
+    static PersistentValuePrivate *create(ExecutionEngine *e, const Value &v);
+    void ref() { ++refcount; }
+    void deref();
+};
+
 class PersistentValue
 {
 public:
-    PersistentValue();
-    PersistentValue(MemoryManager *mm, const Value &val);
+    PersistentValue(ExecutionEngine *e, const Value &val);
     PersistentValue(const PersistentValue &other);
     PersistentValue &operator=(const PersistentValue &other);
     ~PersistentValue();
 
-    Value *operator->() { return &m_value; }
-    Value *operator*() { return &m_value; }
+    Value *operator->() { return &d->value; }
+    Value *operator*() { return &d->value; }
 
-    operator Value() const { return m_value; }
+    operator Value() const { return d->value; }
 
 private:
-    Managed *asManaged() { return m_memoryManager ? m_value.asManaged() : 0; }
-    MemoryManager *m_memoryManager;
-    Value m_value;
+    PersistentValuePrivate *d;
 };
 
 } // namespace VM