Fix memory corruption when multiple QML engines have JavaScript wrappers for the same QObject

It's possible that the same QObject is exposed to multiple JavaScript
environments, for which we have this "extra" hack in the form of a QMap.  The
common case is that QQmlData has a QV4::WeakValue that points to the JS wrapper
for the object. However in the rare case of multiple exposure, a map in the
other engines stores those references. That map was erroneously storing
pointers to temporary values on the JS stack instead of heap pointers.

Change-Id: I8587f9921a9b4f9efd288326d00cebc25ad0bc12
Task-number: QTBUG-45051
Reviewed-by: Lars Knoll <lars.knoll@digia.com>
This commit is contained in:
Simon Hausmann 2015-04-28 15:38:09 +02:00
parent d0dc7cec78
commit 3b5d37ce38
4 changed files with 25 additions and 21 deletions

View File

@ -390,7 +390,7 @@ void MemoryManager::sweep(bool lastSweep)
if (MultiplyWrappedQObjectMap *multiplyWrappedQObjects = m_d->engine->m_multiplyWrappedQObjects) {
for (MultiplyWrappedQObjectMap::Iterator it = multiplyWrappedQObjects->begin(); it != multiplyWrappedQObjects->end();) {
if (!it.value()->markBit())
if (!it.value().isNullOrUndefined())
it = multiplyWrappedQObjects->erase(it);
else
++it;

View File

@ -589,7 +589,7 @@ ReturnedValue QObjectWrapper::wrap(ExecutionEngine *engine, QObject *object)
} else if (ddata->jsWrapper.isUndefined() &&
(ddata->jsEngineId == engine->m_engineId || // We own the QObject
ddata->jsEngineId == 0 || // No one owns the QObject
!ddata->hasTaintedV8Object)) { // Someone else has used the QObject, but it isn't tainted
!ddata->hasTaintedV4Object)) { // Someone else has used the QObject, but it isn't tainted
QV4::ScopedValue rv(scope, create(engine, object));
ddata->jsWrapper.set(scope.engine, rv);
@ -600,11 +600,11 @@ ReturnedValue QObjectWrapper::wrap(ExecutionEngine *engine, QObject *object)
// If this object is tainted, we have to check to see if it is in our
// tainted object list
ScopedObject alternateWrapper(scope, (Object *)0);
if (engine->m_multiplyWrappedQObjects && ddata->hasTaintedV8Object)
if (engine->m_multiplyWrappedQObjects && ddata->hasTaintedV4Object)
alternateWrapper = engine->m_multiplyWrappedQObjects->value(object);
// If our tainted handle doesn't exist or has been collected, and there isn't
// a handle in the ddata, we can assume ownership of the ddata->v8object
// a handle in the ddata, we can assume ownership of the ddata->jsWrapper
if (ddata->jsWrapper.isUndefined() && !alternateWrapper) {
QV4::ScopedValue result(scope, create(engine, object));
ddata->jsWrapper.set(scope.engine, result);
@ -616,8 +616,8 @@ ReturnedValue QObjectWrapper::wrap(ExecutionEngine *engine, QObject *object)
alternateWrapper = create(engine, object);
if (!engine->m_multiplyWrappedQObjects)
engine->m_multiplyWrappedQObjects = new MultiplyWrappedQObjectMap;
engine->m_multiplyWrappedQObjects->insert(object, alternateWrapper);
ddata->hasTaintedV8Object = true;
engine->m_multiplyWrappedQObjects->insert(object, alternateWrapper->d());
ddata->hasTaintedV4Object = true;
}
return alternateWrapper.asReturnedValue();
@ -1908,16 +1908,20 @@ Heap::QmlSignalHandler::QmlSignalHandler(QV4::ExecutionEngine *engine, QObject *
DEFINE_OBJECT_VTABLE(QmlSignalHandler);
void MultiplyWrappedQObjectMap::insert(QObject *key, Object *value)
void MultiplyWrappedQObjectMap::insert(QObject *key, Heap::Object *value)
{
QHash<QObject*, Object*>::insert(key, value);
QV4::WeakValue v;
v.set(value->internalClass->engine, value);
QHash<QObject*, QV4::WeakValue>::insert(key, v);
connect(key, SIGNAL(destroyed(QObject*)), this, SLOT(removeDestroyedObject(QObject*)));
}
MultiplyWrappedQObjectMap::Iterator MultiplyWrappedQObjectMap::erase(MultiplyWrappedQObjectMap::Iterator it)
{
disconnect(it.key(), SIGNAL(destroyed(QObject*)), this, SLOT(removeDestroyedObject(QObject*)));
return QHash<QObject*, Object*>::erase(it);
return QHash<QObject*, QV4::WeakValue>::erase(it);
}
void MultiplyWrappedQObjectMap::remove(QObject *key)
@ -1930,7 +1934,7 @@ void MultiplyWrappedQObjectMap::remove(QObject *key)
void MultiplyWrappedQObjectMap::removeDestroyedObject(QObject *object)
{
QHash<QObject*, Object*>::remove(object);
QHash<QObject*, QV4::WeakValue>::remove(object);
}
QT_END_NAMESPACE

View File

@ -173,20 +173,20 @@ struct QmlSignalHandler : public QV4::Object
};
class MultiplyWrappedQObjectMap : public QObject,
private QHash<QObject*, Object*>
private QHash<QObject*, QV4::WeakValue>
{
Q_OBJECT
public:
typedef QHash<QObject*, Object*>::ConstIterator ConstIterator;
typedef QHash<QObject*, Object*>::Iterator Iterator;
typedef QHash<QObject*, QV4::WeakValue>::ConstIterator ConstIterator;
typedef QHash<QObject*, QV4::WeakValue>::Iterator Iterator;
ConstIterator begin() const { return QHash<QObject*, Object*>::constBegin(); }
Iterator begin() { return QHash<QObject*, Object*>::begin(); }
ConstIterator end() const { return QHash<QObject*, Object*>::constEnd(); }
Iterator end() { return QHash<QObject*, Object*>::end(); }
ConstIterator begin() const { return QHash<QObject*, QV4::WeakValue>::constBegin(); }
Iterator begin() { return QHash<QObject*, QV4::WeakValue>::begin(); }
ConstIterator end() const { return QHash<QObject*, QV4::WeakValue>::constEnd(); }
Iterator end() { return QHash<QObject*, QV4::WeakValue>::end(); }
void insert(QObject *key, Object *value);
Object *value(QObject *key) const { return QHash<QObject*, Object*>::value(key, 0); }
void insert(QObject *key, Heap::Object *value);
ReturnedValue value(QObject *key) const { return QHash<QObject*, QV4::WeakValue>::value(key).value(); }
Iterator erase(Iterator it);
void remove(QObject *key);

View File

@ -74,7 +74,7 @@ class Q_QML_PRIVATE_EXPORT QQmlData : public QAbstractDeclarativeData
public:
QQmlData()
: ownedByQml1(false), ownMemory(true), ownContext(false), indestructible(true), explicitIndestructibleSet(false),
hasTaintedV8Object(false), isQueuedForDeletion(false), rootObjectInCreation(false),
hasTaintedV4Object(false), isQueuedForDeletion(false), rootObjectInCreation(false),
hasVMEMetaObject(false), parentFrozen(false), bindingBitsSize(0), bindingBits(0), notifyList(0), context(0), outerContext(0),
bindings(0), signalHandlers(0), nextContextObject(0), prevContextObject(0),
lineNumber(0), columnNumber(0), jsEngineId(0), compiledData(0), deferredData(0),
@ -112,7 +112,7 @@ public:
quint32 ownContext:1;
quint32 indestructible:1;
quint32 explicitIndestructibleSet:1;
quint32 hasTaintedV8Object:1;
quint32 hasTaintedV4Object:1;
quint32 isQueuedForDeletion:1;
/*
* rootObjectInCreation should be true only when creating top level CPP and QML objects,