Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix script-related crashes on exiting a domain #1251

Merged
merged 2 commits into from
Nov 23, 2024
Merged

Fix script-related crashes on exiting a domain #1251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9e26146
Fix script-related crashes on exiting domain
ksuprynowicz Nov 22, 2024
d9c5821
Fixed blocking call
ksuprynowicz Nov 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions libraries/entities-renderer/src/EntityTreeRenderer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,11 @@ void EntityTreeRenderer::setupEntityScriptEngineSignals(const ScriptManagerPoint
void EntityTreeRenderer::resetPersistentEntitiesScriptEngine() {
// This runs script engine shutdown procedure in a separate thread, avoiding a deadlock when script engine is doing
// a blocking call to main thread
if (_persistentEntitiesScriptManager) {
QtConcurrent::run([manager = _persistentEntitiesScriptManager] {
ScriptManagerPointer scriptManager = _persistentEntitiesScriptManager;
// Clear the pointer before lambda is run on another thread.
_persistentEntitiesScriptManager.reset();
if (scriptManager) {
QtConcurrent::run([manager = scriptManager] {
manager->unloadAllEntityScripts(true);
manager->stop();
manager->waitTillDoneRunning();
Expand Down Expand Up @@ -251,8 +254,11 @@ void EntityTreeRenderer::resetPersistentEntitiesScriptEngine() {
void EntityTreeRenderer::resetNonPersistentEntitiesScriptEngine() {
// This runs script engine shutdown procedure in a separate thread, avoiding a deadlock when script engine is doing
// a blocking call to main thread
if (_nonPersistentEntitiesScriptManager) {
QtConcurrent::run([manager = _nonPersistentEntitiesScriptManager] {
ScriptManagerPointer scriptManager = _nonPersistentEntitiesScriptManager;
// Release the pointer as soon as possible.
_nonPersistentEntitiesScriptManager.reset();
if (scriptManager) {
QtConcurrent::run([manager = scriptManager] {
manager->unloadAllEntityScripts(true);
manager->stop();
manager->waitTillDoneRunning();
Expand Down
87 changes: 44 additions & 43 deletions libraries/script-engine/src/ScriptManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,10 @@ void ScriptManager::runInThread() {

void ScriptManager::executeOnScriptThread(std::function<void()> function, const Qt::ConnectionType& type ) {
if (QThread::currentThread() != thread()) {
QMetaObject::invokeMethod(this, "executeOnScriptThread", type, Q_ARG(std::function<void()>, function));
// Lambda is necessary there to keep shared_ptr counter above zero
QMetaObject::invokeMethod(this, [=, manager = shared_from_this()]{
manager->executeOnScriptThread(function, type);
});
return;
}

Expand Down Expand Up @@ -859,10 +862,10 @@ void ScriptManager::removeEventHandler(const EntityItemID& entityID, const QStri
qCDebug(scriptengine) << "*** WARNING *** ScriptManager::removeEventHandler() called on wrong thread [" << QThread::currentThread() << "], invoking on correct thread [" << thread() << "] "
"entityID:" << entityID << " eventName:" << eventName;
#endif
QMetaObject::invokeMethod(this, "removeEventHandler",
Q_ARG(const EntityItemID&, entityID),
Q_ARG(const QString&, eventName),
Q_ARG(const ScriptValue&, handler));
// Lambda is necessary there to keep shared_ptr counter above zero
QMetaObject::invokeMethod(this, [=, manager = shared_from_this()]{
manager->removeEventHandler(entityID, eventName, handler);
});
return;
}
#ifdef THREAD_DEBUGGING
Expand Down Expand Up @@ -906,10 +909,10 @@ void ScriptManager::addEventHandler(const EntityItemID& entityID, const QString&
"entityID:" << entityID << " eventName:" << eventName;
#endif

QMetaObject::invokeMethod(this, "addEventHandler",
Q_ARG(const EntityItemID&, entityID),
Q_ARG(const QString&, eventName),
Q_ARG(const ScriptValue&, handler));
// Lambda is necessary there to keep shared_ptr counter above zero
QMetaObject::invokeMethod(this, [=, manager = shared_from_this()]{
manager->addEventHandler(entityID, eventName, handler);
});
return;
}
#ifdef THREAD_DEBUGGING
Expand Down Expand Up @@ -1168,7 +1171,10 @@ void ScriptManager::stop(bool marshal) {
_isStopping = true; // this can be done on any thread

if (marshal) {
QMetaObject::invokeMethod(this, "stop");
// Lambda is necessary there to keep shared_ptr counter above zero if this gets called from different thread
QMetaObject::invokeMethod(this, [=, manager = shared_from_this()]{
manager->stop(false);
});
return;
}

Expand Down Expand Up @@ -2012,11 +2018,10 @@ bool ScriptManager::hasEntityScriptDetails(const EntityItemID& entityID) const {

void ScriptManager::loadEntityScript(const EntityItemID& entityID, const QString& entityScript, bool forceRedownload) {
if (QThread::currentThread() != thread()) {
QMetaObject::invokeMethod(this, "loadEntityScript",
Q_ARG(const EntityItemID&, entityID),
Q_ARG(const QString&, entityScript),
Q_ARG(bool, forceRedownload)
);
// Lambda is necessary there to keep shared_ptr counter above zero
QMetaObject::invokeMethod(this, [=, manager = shared_from_this()]{
manager->loadEntityScript(entityID, entityScript, forceRedownload);
});
return;
}
PROFILE_RANGE(script, __FUNCTION__);
Expand Down Expand Up @@ -2093,13 +2098,10 @@ void ScriptManager::entityScriptContentAvailable(const EntityItemID& entityID, c
<< contents << "isURL:" << isURL << "success:" << success;
#endif

QMetaObject::invokeMethod(this, "entityScriptContentAvailable",
Q_ARG(const EntityItemID&, entityID),
Q_ARG(const QString&, scriptOrURL),
Q_ARG(const QString&, contents),
Q_ARG(bool, isURL),
Q_ARG(bool, success),
Q_ARG(const QString&, status));
// Lambda is necessary there to keep shared_ptr counter above zero
QMetaObject::invokeMethod(this, [=, manager = shared_from_this()]{
manager->entityScriptContentAvailable(entityID, scriptOrURL, contents, isURL, success, status);
});
return;
}

Expand Down Expand Up @@ -2443,9 +2445,10 @@ void ScriptManager::unloadEntityScript(const EntityItemID& entityID, bool should
"entityID:" << entityID;
#endif

QMetaObject::invokeMethod(this, "unloadEntityScript",
Q_ARG(const EntityItemID&, entityID),
Q_ARG(bool, shouldRemoveFromMap));
// Lambda is necessary there to keep shared_ptr counter above zero
QMetaObject::invokeMethod(this, [=, manager = shared_from_this()]{
manager->unloadEntityScript(entityID, shouldRemoveFromMap);
});
return;
}
#ifdef THREAD_DEBUGGING
Expand Down Expand Up @@ -2495,9 +2498,10 @@ void ScriptManager::unloadAllEntityScripts(bool blockingCall) {
#ifdef THREAD_DEBUGGING
qCDebug(scriptengine) << "*** WARNING *** ScriptManager::unloadAllEntityScripts() called on wrong thread [" << QThread::currentThread() << "], invoking on correct thread [" << thread() << "]";
#endif

QMetaObject::invokeMethod(this, "unloadAllEntityScripts",
blockingCall ? Qt::BlockingQueuedConnection : Qt::QueuedConnection);
// Lambda is necessary there to keep shared_ptr counter above zero
QMetaObject::invokeMethod(this, [=, manager = shared_from_this()] {
manager->unloadAllEntityScripts(blockingCall);
}, blockingCall ? Qt::BlockingQueuedConnection : Qt::QueuedConnection);
return;
}
#ifdef THREAD_DEBUGGING
Expand Down Expand Up @@ -2590,12 +2594,10 @@ void ScriptManager::callEntityScriptMethod(const EntityItemID& entityID, const Q
qCDebug(scriptengine) << "*** WARNING *** ScriptManager::callEntityScriptMethod() called on wrong thread [" << QThread::currentThread() << "], invoking on correct thread [" << thread() << "] "
"entityID:" << entityID << "methodName:" << methodName;
#endif

QMetaObject::invokeMethod(this, "callEntityScriptMethod",
Q_ARG(const EntityItemID&, entityID),
Q_ARG(const QString&, methodName),
Q_ARG(const QStringList&, params),
Q_ARG(const QUuid&, remoteCallerID));
// Lambda is necessary there to keep shared_ptr counter above zero
QMetaObject::invokeMethod(this, [=, manager = shared_from_this()]{
manager->callEntityScriptMethod(entityID, methodName, params, remoteCallerID);
});
return;
}
#ifdef THREAD_DEBUGGING
Expand Down Expand Up @@ -2660,10 +2662,10 @@ void ScriptManager::callEntityScriptMethod(const EntityItemID& entityID, const Q
"entityID:" << entityID << "methodName:" << methodName << "event: mouseEvent";
#endif

QMetaObject::invokeMethod(this, "callEntityScriptMethod",
Q_ARG(const EntityItemID&, entityID),
Q_ARG(const QString&, methodName),
Q_ARG(const PointerEvent&, event));
// Lambda is necessary there to keep shared_ptr counter above zero
QMetaObject::invokeMethod(this, [=, manager = shared_from_this()]{
manager->callEntityScriptMethod(entityID, methodName, event);
});
return;
}
#ifdef THREAD_DEBUGGING
Expand Down Expand Up @@ -2699,11 +2701,10 @@ void ScriptManager::callEntityScriptMethod(const EntityItemID& entityID, const Q
"entityID:" << entityID << "methodName:" << methodName << "otherID:" << otherID << "collision: collision";
#endif

QMetaObject::invokeMethod(this, "callEntityScriptMethod",
Q_ARG(const EntityItemID&, entityID),
Q_ARG(const QString&, methodName),
Q_ARG(const EntityItemID&, otherID),
Q_ARG(const Collision&, collision));
// Lambda is necessary there to keep shared_ptr counter above zero
QMetaObject::invokeMethod(this, [=, manager = shared_from_this()]{
manager->callEntityScriptMethod(entityID, methodName, otherID, collision);
});
return;
}
#ifdef THREAD_DEBUGGING
Expand Down
11 changes: 7 additions & 4 deletions libraries/script-engine/src/ScriptManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,24 +378,27 @@ class ScriptManager : public QObject, public EntitiesScriptEngineProvider, publi
Q_ENUM(Type);

static int processLevelMaxRetries;
ScriptManager(Context context, const QString& scriptContents = NO_SCRIPT, const QString& fileNameString = QString("about:ScriptEngine"));
~ScriptManager();
private:
// Constructor is private so that only properly generated shared pointer can be used
explicit ScriptManager(Context context, const QString& scriptContents = NO_SCRIPT, const QString& fileNameString = QString("about:ScriptEngine"));
public:
~ScriptManager() override;

// static initialization support
typedef void (*ScriptManagerInitializer)(ScriptManager*);
class StaticInitializerNode {
public:
ScriptManagerInitializer init;
StaticInitializerNode* prev;
inline StaticInitializerNode(ScriptManagerInitializer&& pInit) : init(std::move(pInit)),prev(nullptr) { registerNewStaticInitializer(this); }
inline explicit StaticInitializerNode(ScriptManagerInitializer&& pInit) : init(std::move(pInit)),prev(nullptr) { registerNewStaticInitializer(this); }
};
static void registerNewStaticInitializer(StaticInitializerNode* dest);

class StaticTypesInitializerNode {
public:
ScriptManagerInitializer init;
StaticTypesInitializerNode* prev;
inline StaticTypesInitializerNode(ScriptManagerInitializer&& pInit) : init(std::move(pInit)),prev(nullptr) { registerNewStaticTypesInitializer(this); }
inline explicit StaticTypesInitializerNode(ScriptManagerInitializer&& pInit) : init(std::move(pInit)),prev(nullptr) { registerNewStaticTypesInitializer(this); }
};
static void registerNewStaticTypesInitializer(StaticTypesInitializerNode* dest);

Expand Down
Loading