History

Clean scripting thread deletion

Zach Pomerantz committed 10 years ago

Commit edf82c51 parent 13d60241 file · +30 −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+30−24
@@ -162,16 +162,15 @@ ScriptEngine::~ScriptEngine() { · 161 unmodified lines
162162 } else {
163163 qCWarning(scriptengine) << "Script destroyed after ScriptEngines!";
164164 }
165−
166− waitTillDoneRunning();
167165 }
168166
169167 void ScriptEngine::disconnectNonEssentialSignals() {
170168 disconnect();
171− QThread* receiver;
169+ QThread* workerThread;
172170 // Ensure the thread should be running, and does exist
173− if (_isRunning && _isThreaded && (receiver = thread())) {
174− connect(this, &ScriptEngine::doneRunning, receiver, &QThread::quit);
171+ if (_isRunning && _isThreaded && (workerThread = thread())) {
172+ connect(this, &ScriptEngine::doneRunning, workerThread, &QThread::quit);
173+ connect(workerThread, &QThread::finished, workerThread, &QObject::deleteLater);
175174 }
176175 }
177176
@@ -268,30 +267,33 @@ void ScriptEngine::runInThread() { · 90 unmodified lines
268267 }
269268
270269 _isThreaded = true;
271− QThread* workerThread = new QThread();
272− QString scriptEngineName = QString("Script Thread:") + getFilename();
273− workerThread->setObjectName(scriptEngineName);
274270
271+ // The thread interface cannot live on itself, and we want to move this into the thread, so
272+ // the thread cannot have this as a parent.
273+ QThread* workerThread = new QThread();
274+ workerThread->setObjectName(QString("Script Thread:") + getFilename());
275+ moveToThread(workerThread);
276+
275277 // NOTE: If you connect any essential signals for proper shutdown or cleanup of
276278 // the script engine, make sure to add code to "reconnect" them to the
277279 // disconnectNonEssentialSignals() method
278−
279− // when the worker thread is started, call our engine's run..
280280 connect(workerThread, &QThread::started, this, &ScriptEngine::run);
281−
282− // tell the thread to stop when the script engine is done
283281 connect(this, &ScriptEngine::doneRunning, workerThread, &QThread::quit);
282+ connect(workerThread, &QThread::finished, workerThread, &QObject::deleteLater);
284283
285− moveToThread(workerThread);
286−
287− // Starts an event loop, and emits workerThread->started()
288284 workerThread->start();
289285 }
290286
291287 void ScriptEngine::waitTillDoneRunning() {
292288 // If the script never started running or finished running before we got here, we don't need to wait for it
293289 auto workerThread = thread();
290+
294291 if (_isThreaded && workerThread) {
292+ // We should never be waiting (blocking) on our own thread
293+ assert(workerThread != QThread::currentThread());
294+
295+ stop();
296+
295297 QString scriptName = getFilename();
296298 auto startedWaiting = usecTimestampNow();
297299
@@ -301,24 +303,28 @@ void ScriptEngine::waitTillDoneRunning() { · 3 unmodified lines
301303 // the scripts will likely need to marshall messages across to the main thread, e.g.
302304 // if they access Settings or Menu in any of their shutdown code. So:
303305 // Process events for the main application thread, allowing invokeMethod calls to pass between threads.
304− QCoreApplication::processEvents(); // thread-safe :)
306+ QCoreApplication::processEvents();
305307
306− // If we've been waiting a second or more, then tell the script engine to stop evaluating
308+ // If the final evaluation takes too long, then tell the script engine to stop evaluating
307309 static const auto MAX_SCRIPT_EVALUATION_TIME = USECS_PER_SECOND;
310+ static const auto MAX_SCRIPT_QUITTING_TIME = 0.5 * MAX_SCRIPT_EVALUATION_TIME;
308311 auto elapsedUsecs = usecTimestampNow() - startedWaiting;
309312 if (elapsedUsecs > MAX_SCRIPT_EVALUATION_TIME) {
310− qCDebug(scriptengine) <<
311− "Script " << scriptName << " has been running too long [" << elapsedUsecs << " usecs] quitting.";
312− abortEvaluation(); // to allow the thread to quit
313− workerThread->quit();
314− break;
313+ qCDebug(scriptengine).nospace() <<
314+ "Script " << scriptName << " has been running too long [" << elapsedUsecs << "usecs] quitting.";
315+ abortEvaluation(); // break out of current evaluation
316+
317+ // Wait for the scripting thread to stop running
318+ if (!workerThread->wait(MAX_SCRIPT_QUITTING_TIME)) {
319+ qCWarning(scriptengine).nospace() <<
320+ "Script " << scriptName << " has been quitting too long [" << elapsedUsecs << "usecs] terminating.";
321+ workerThread->terminate();
322+ }
315323 }
316324
317325 // Avoid a pure busy wait
318326 QThread::yieldCurrentThread();
319327 }
320−
321− workerThread->deleteLater();
322328 }
323329 }
324330