From 09c98b3ac3983a165d11b98f7d8fefe0c9f7e401 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Mon, 22 May 2017 15:31:11 -0700 Subject: [PATCH 1/2] Fix ScriptEngine thread being deleted too early The thread appeared to be deleted before waitTillDoneRunning was finished. This is a speculative fix. This ensures that the QThread isn't deleted until after the ScriptEngine is deleted. --- libraries/script-engine/src/ScriptEngine.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 8bbb9a3e2c..9c7dd6f230 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -231,7 +231,7 @@ void ScriptEngine::disconnectNonEssentialSignals() { // Ensure the thread should be running, and does exist if (_isRunning && _isThreaded && (workerThread = thread())) { connect(this, &ScriptEngine::doneRunning, workerThread, &QThread::quit); - connect(workerThread, &QThread::finished, workerThread, &QObject::deleteLater); + connect(this, &QObject::destroyed, workerThread, &QObject::deleteLater); } } @@ -346,7 +346,7 @@ void ScriptEngine::runInThread() { // disconnectNonEssentialSignals() method connect(workerThread, &QThread::started, this, &ScriptEngine::run); connect(this, &ScriptEngine::doneRunning, workerThread, &QThread::quit); - connect(workerThread, &QThread::finished, workerThread, &QObject::deleteLater); + connect(this, &QObject::destroyed, workerThread, &QObject::deleteLater); workerThread->start(); } From 3c9205d0b238609c8a4a35f7952e9c5e7c939be5 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Mon, 22 May 2017 15:35:38 -0700 Subject: [PATCH 2/2] Remove invalid comment and unnecessary check from ScriptEngine This removes an invalid comment - waitTillDoneRunning is not only called from the main thread - and remove a check to make sure thread() is not null because it should no longer be deleted before ScriptEngine. --- libraries/script-engine/src/ScriptEngine.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 9c7dd6f230..03d1eb5868 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -397,16 +397,12 @@ void ScriptEngine::waitTillDoneRunning() { } } - // NOTE: This will be called on the main application thread from stopAllScripts. - // The application thread will need to continue to process events, because + // NOTE: This will be called on the main application thread (among other threads) from stopAllScripts. + // The thread will need to continue to process events, because // the scripts will likely need to marshall messages across to the main thread, e.g. // if they access Settings or Menu in any of their shutdown code. So: - // Process events for the main application thread, allowing invokeMethod calls to pass between threads. + // Process events for this thread, allowing invokeMethod calls to pass between threads. QCoreApplication::processEvents(); - // In some cases (debugging), processEvents may give the thread enough time to shut down, so recheck it. - if (!thread()) { - break; - } // Avoid a pure busy wait QThread::yieldCurrentThread();