History

Properly shut down scripting thread

Zach Pomerantz committed 10 years ago

Commit 284feaf1 parent 42169dd1 file · +26 −24Browse files

Showing the changes to libraries/script-engine/src/ScriptEngine.cpp (1 of 1 file). Show all

Unified · Split

libraries/script-engine/src/ScriptEngine.cpp+26−24
@@ -146,12 +146,15 @@ ScriptEngine::ScriptEngine(const QString& scriptContents, const QString& fileNam · 145 unmodified lines
146146
147147 ScriptEngine::~ScriptEngine() {
148148 qCDebug(scriptengine) << "Script Engine shutting down (destructor) for script:" << getFilename();
149+
149150 auto scriptEngines = DependencyManager::get<ScriptEngines>();
150151 if (scriptEngines) {
151152 scriptEngines->removeScriptEngine(this);
152153 } else {
153154 qCWarning(scriptengine) << "Script destroyed after ScriptEngines!";
154155 }
156+
157+ waitTillDoneRunning();
155158 }
156159
157160 void ScriptEngine::disconnectNonEssentialSignals() {
@@ -172,7 +175,7 @@ void ScriptEngine::runInThread() { · 14 unmodified lines
172175 }
173176
174177 _isThreaded = true;
175− QThread* workerThread = new QThread(this); // thread is not owned, so we need to manage the delete
178+ QThread* workerThread = new QThread();
176179 QString scriptEngineName = QString("Script Thread:") + getFilename();
177180 workerThread->setObjectName(scriptEngineName);
178181
@@ -194,35 +197,34 @@ void ScriptEngine::runInThread() { · 15 unmodified lines
194197
195198 void ScriptEngine::waitTillDoneRunning() {
196199 // If the script never started running or finished running before we got here, we don't need to wait for it
197− if (_isRunning && _isThreaded) {
198−
199− // NOTE: waitTillDoneRunning() will be called on the main Application thread, inside of stopAllScripts()
200− // we want the application thread to continue to process events, because the scripts will likely need to
201− // marshall messages across to the main thread. For example if they access Settings or Menu in any of their
202− // shutdown code.
200+ auto workerThread = thread();
201+ if (_isThreaded && workerThread) {
203202 QString scriptName = getFilename();
204−
205203 auto startedWaiting = usecTimestampNow();
206− while (thread()->isRunning()) {
207− // process events for the main application thread, allowing invokeMethod calls to pass between threads
208− QCoreApplication::processEvents();
209− auto stillWaiting = usecTimestampNow();
210− auto elapsedUsecs = stillWaiting - startedWaiting;
211−
212− // if we've been waiting a second or more, then tell the script engine to stop evaluating
213− static const auto MAX_SCRIPT_EVALUATION_TIME = USECS_PER_SECOND;
214− static const auto WAITING_TOO_LONG = MAX_SCRIPT_EVALUATION_TIME * 5;
215204
216− // if we've been waiting for more than 5 seconds then we should be more aggessive about stopping
217− if (elapsedUsecs > WAITING_TOO_LONG) {
218− qCDebug(scriptengine) << "Script " << scriptName << " has been running too long [" << elapsedUsecs << " usecs] quitting.";
219− thread()->quit();
205+ while (workerThread->isRunning()) {
206+ // NOTE: This will be called on the main application thread from stopAllScripts.
207+ // The application thread will need to continue to process events, because
208+ // the scripts will likely need to marshall messages across to the main thread, e.g.
209+ // if they access Settings or Menu in any of their shutdown code. So:
210+ // Process events for the main application thread, allowing invokeMethod calls to pass between threads.
211+ QCoreApplication::processEvents(); // thread-safe :)
212+
213+ // If we've been waiting a second or more, then tell the script engine to stop evaluating
214+ static const auto MAX_SCRIPT_EVALUATION_TIME = USECS_PER_SECOND;
215+ auto elapsedUsecs = usecTimestampNow() - startedWaiting;
216+ if (elapsedUsecs > MAX_SCRIPT_EVALUATION_TIME) {
217+ qCDebug(scriptengine) <<
218+ "Script " << scriptName << " has been running too long [" << elapsedUsecs << " usecs] quitting.";
219+ workerThread->quit();
220220 break;
221− } else if (elapsedUsecs > MAX_SCRIPT_EVALUATION_TIME) {
222− qCDebug(scriptengine) << "Script " << scriptName << " has been running too long [" << elapsedUsecs << " usecs] aborting evaluation.";
223− QMetaObject::invokeMethod(this, "abortEvaluation");
224221 }
222+
223+ // Avoid a pure busy wait
224+ QThread::yieldCurrentThread();
225225 }
226+
227+ workerThread->deleteLater();
226228 }
227229 }
228230