Mirrors/overte
3
0
Fork 0
mirror of https://github.com/overte-org/overte.git synced 2025-04-08 08:14:48 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

V8 QObject pointer handling changes

Browse source
This commit is contained in:
ksuprynowicz 2023-03-03 00:38:40 +01:00
parent 5e5003ff03
commit cae1e1195d
6 changed files with 72 additions and 34 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
libraries/animation/src/AnimVariant.cpp
View file

@ -124,7 +124,7 @@ void AnimVariantMap::animVariantMapFromScriptValue(const ScriptValue& source) {
}
}
qCWarning(animation) << "Ignoring unrecognized data " << value.toString() << " for animation property " << property->name();
// V8TODO this was spamming to much logs but needs to be fixed later
// V8TODO this was spamming too much logs but needs to be fixed later
//Q_ASSERT(false);
}
}

4
libraries/script-engine/src/v8/ScriptContextV8Wrapper.h
View file

@ -58,7 +58,7 @@ private: // storage
const v8::FunctionCallbackInfo<v8::Value> *_functionCallbackInfo;
const v8::PropertyCallbackInfo<v8::Value> *_propertyCallbackInfo;
ScriptEngineV8* _engine;
// V8TODO: Is custom copy constructor needed for thread safety?
// V8TODO: custom destructor is needed for v8::Persistent
v8::Persistent<v8::Context> _context;
ScriptContextPointer _parentContext;
Q_DISABLE_COPY(ScriptContextV8Wrapper)
@ -77,7 +77,7 @@ public: // ScriptFunctionContext implementation
private: // storage
ScriptEngineV8* _engine;
// V8TODO: Is custom copy constructor needed for thread safety?
// V8TODO: custom destructor is needed for v8::Persistent (check is all other classes using different types of persistent handles have custom destructors too)
v8::Persistent<v8::Context> _context;
//V8ScriptContextInfo _value;
Q_DISABLE_COPY(ScriptFunctionContextV8Wrapper)

9
libraries/script-engine/src/v8/ScriptEngineV8_cast.cpp
View file

@ -471,8 +471,9 @@ bool ScriptEngineV8::castValueToVariant(const V8ScriptValue& v8Val, QVariant& de
// V8TODO: this is to diagnose a really weird segfault where it looks like only half of the QPointer is set to null upon object deletion
uint64_t ptrVal = (uint64_t)(ScriptObjectV8Proxy::unwrap(v8Val));
if ((uint32_t)(ptrVal) == 0 && ptrVal != 0) {
printf("break");
Q_ASSERT(false);
qDebug() << "ScriptEngineV8::castValueToVariant pointer bug happened";
//Q_ASSERT(false);
return false;
}
}
dest = QVariant::fromValue(ScriptObjectV8Proxy::unwrap(v8Val));
@ -754,7 +755,9 @@ V8ScriptValue ScriptEngineV8::castVariantToValue(const QVariant& val) {
}
default:
// check to see if this is a pointer to a QObject-derived object
if (QMetaType::typeFlags(valTypeId) & (QMetaType::PointerToQObject | QMetaType::TrackingPointerToQObject)) {
// V8TODO: I added WeakPointerToQObject and SharedPointerToQObject here, but maybe they need different object ownership?
if (QMetaType::typeFlags(valTypeId) & (QMetaType::PointerToQObject | QMetaType::TrackingPointerToQObject
| QMetaType::WeakPointerToQObject | QMetaType::SharedPointerToQObject)) {
QObject* obj = val.value<QObject*>();
if (obj == nullptr) return V8ScriptValue(this, v8::Null(_v8Isolate));
return ScriptObjectV8Proxy::newQObject(this, obj);

85
libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp
View file

@ -66,6 +66,7 @@ private: // storage
ScriptObjectV8Proxy::ScriptObjectV8Proxy(ScriptEngineV8* engine, QObject* object, bool ownsObject, const ScriptEngine::QObjectWrapOptions& options) :
_engine(engine), _wrapOptions(options), _ownsObject(ownsObject), _object(object) {
//_v8ObjectTemplate(engine->getIsolate(), v8::ObjectTemplate::New(engine->getIsolate())
Q_ASSERT(_engine != nullptr);
investigate();
}
@ -451,21 +452,40 @@ void ScriptObjectV8Proxy::v8Get(v8::Local<v8::Name> name, const v8::PropertyCall
return;
}
V8ScriptValue object(proxy->_engine, objectV8);
Q_ASSERT(name->IsString());
V8ScriptString nameString(proxy->_engine, v8::Local<v8::String>::Cast(name));
uint id;
QueryFlags flags = proxy->queryProperty(object, nameString, HandlesReadAccess, &id);
if (flags) {
V8ScriptValue value = proxy->property(object, nameString, id);
info.GetReturnValue().Set(value.get());
if (!name->IsString() && !name->IsSymbol()) {
QString notStringMessage("ScriptObjectV8Proxy::v8Get: " + proxy->_engine->scriptValueDebugDetailsV8(V8ScriptValue(proxy->_engine, name)));
qDebug(scriptengine) << notStringMessage;
Q_ASSERT(false);
}
v8::Local<v8::String> v8NameString;
/*if (name->IsString()) {
v8NameString = v8::Local<v8::String>::Cast(name);
} else {
v8::Local<v8::Value> property;
if(info.This()->GetInternalField(2).As<v8::Object>()->Get(proxy->_engine->getContext(), name).ToLocal(&property)) {
info.GetReturnValue().Set(property);
} else {
qDebug(scriptengine) << "Value not found: " << *utf8Value;
if (!name->ToString(info.GetIsolate()->GetCurrentContext()).ToLocal(&v8NameString)) {
Q_ASSERT(false);
}
}
if (name->IsSymbol()) {
qDebug(scriptengine) << "ScriptObjectV8Proxy::v8Set: symbol: " + nameString.toQString();
}*/
if (name->IsString()) {
V8ScriptString nameString(proxy->_engine, v8::Local<v8::String>::Cast(name));
uint id;
QueryFlags flags = proxy->queryProperty(object, nameString, HandlesReadAccess, &id);
if (flags) {
V8ScriptValue value = proxy->property(object, nameString, id);
info.GetReturnValue().Set(value.get());
return;
}
}
v8::Local<v8::Value> property;
if(info.This()->GetInternalField(2).As<v8::Object>()->Get(proxy->_engine->getContext(), name).ToLocal(&property)) {
info.GetReturnValue().Set(property);
} else {
qDebug(scriptengine) << "Value not found: " << *utf8Value;
}
}
void ScriptObjectV8Proxy::v8Set(v8::Local<v8::Name> name, v8::Local<v8::Value> value, const v8::PropertyCallbackInfo<v8::Value>& info) {
@ -480,21 +500,38 @@ void ScriptObjectV8Proxy::v8Set(v8::Local<v8::Name> name, v8::Local<v8::Value> v
return;
}
V8ScriptValue object(proxy->_engine, objectV8);
Q_ASSERT(name->IsString());
V8ScriptString nameString(proxy->_engine, v8::Local<v8::String>::Cast(name));
if (!name->IsString() && !name->IsSymbol()) {
QString notStringMessage("ScriptObjectV8Proxy::v8Set: " + proxy->_engine->scriptValueDebugDetailsV8(V8ScriptValue(proxy->_engine, name)));
qDebug(scriptengine) << notStringMessage;
Q_ASSERT(false);
}
/*v8::Local<v8::String> v8NameString;
if (name->IsString()) {
v8NameString = v8::Local<v8::String>::Cast(name);
} else {
if (!name->ToString(info.GetIsolate()->GetCurrentContext()).ToLocal(&v8NameString)) {
Q_ASSERT(false);
}
}
if (name->IsSymbol()) {
qDebug(scriptengine) << "ScriptObjectV8Proxy::v8Set: symbol: " + nameString.toQString();
}*/
//V8ScriptString nameString(info.GetIsolate(), name->ToString(proxy->_engine->getContext()).ToLocalChecked());
uint id;
QueryFlags flags = proxy->queryProperty(object, nameString, HandlesWriteAccess, &id);
if (flags) {
proxy->setProperty(object, nameString, id, V8ScriptValue(proxy->_engine, value));
if (name->IsString()) {
V8ScriptString nameString(proxy->_engine, v8::Local<v8::String>::Cast(name));
uint id;
QueryFlags flags = proxy->queryProperty(object, nameString, HandlesWriteAccess, &id);
if (flags) {
proxy->setProperty(object, nameString, id, V8ScriptValue(proxy->_engine, value));
info.GetReturnValue().Set(value);
return;
}
}
// V8TODO: Should it be v8::Object or v8::Local<v8::Object>?
if (info.This()->GetInternalField(2).As<v8::Object>()->Set(proxy->_engine->getContext(), name, value).FromMaybe(false)) {
info.GetReturnValue().Set(value);
} else {
// V8TODO: Should it be v8::Object or v8::Local<v8::Object>?
if (info.This()->GetInternalField(2).As<v8::Object>()->Set(proxy->_engine->getContext(), name, value).FromMaybe(false)) {
info.GetReturnValue().Set(value);
} else {
qDebug(scriptengine) << "Set failed: " << *utf8Value;
}
qDebug(scriptengine) << "Set failed: " << *utf8Value;
}
}

4
libraries/script-engine/src/v8/ScriptValueV8Wrapper.cpp
View file

@ -554,11 +554,11 @@ QObject* ScriptValueV8Wrapper::toQObject() const {
if (dest.canConvert<QObject*>()) {
return dest.value<QObject*>();
} else {
Q_ASSERT(false);
//Q_ASSERT(false);
return nullptr;
}
} else {
Q_ASSERT(false);
//Q_ASSERT(false);
return nullptr;
}
}

2
libraries/script-engine/src/v8/V8Types.h
View file

@ -131,8 +131,6 @@ public:
}
private:
// std::shared_ptr<v8::UniquePersistent<T>> _value;
// V8TODO: Persistent needs reset to release object? It may cause memory leaks here
std::shared_ptr<v8::UniquePersistent<T>> _value;
// V8TODO: maybe make it weak
// does it need , CopyablePersistentTraits<Value>?