Fix for potential crash in JS console

This commit is contained in:
Brad Davis 2017-07-29 12:27:12 -07:00
parent 52bebf621d
commit bea91be521
4 changed files with 45 additions and 44 deletions

View file

@ -9,6 +9,8 @@
// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html // See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
// //
#include "JSConsole.h"
#include <QFuture> #include <QFuture>
#include <QKeyEvent> #include <QKeyEvent>
#include <QLabel> #include <QLabel>
@ -20,7 +22,6 @@
#include <PathUtils.h> #include <PathUtils.h>
#include "Application.h" #include "Application.h"
#include "JSConsole.h"
#include "ScriptHighlighting.h" #include "ScriptHighlighting.h"
const int NO_CURRENT_HISTORY_COMMAND = -1; const int NO_CURRENT_HISTORY_COMMAND = -1;
@ -60,14 +61,12 @@ void _writeLines(const QString& filename, const QList<QString>& lines) {
QTextStream(&file) << json; QTextStream(&file) << json;
} }
JSConsole::JSConsole(QWidget* parent, ScriptEngine* scriptEngine) : JSConsole::JSConsole(QWidget* parent, const QSharedPointer<ScriptEngine>& scriptEngine) :
QWidget(parent), QWidget(parent),
_ui(new Ui::Console), _ui(new Ui::Console),
_currentCommandInHistory(NO_CURRENT_HISTORY_COMMAND), _currentCommandInHistory(NO_CURRENT_HISTORY_COMMAND),
_savedHistoryFilename(QStandardPaths::writableLocation(QStandardPaths::DataLocation) + "/" + HISTORY_FILENAME), _savedHistoryFilename(QStandardPaths::writableLocation(QStandardPaths::DataLocation) + "/" + HISTORY_FILENAME),
_commandHistory(_readLines(_savedHistoryFilename)), _commandHistory(_readLines(_savedHistoryFilename)) {
_ownScriptEngine(scriptEngine == NULL),
_scriptEngine(NULL) {
_ui->setupUi(this); _ui->setupUi(this);
_ui->promptTextEdit->setLineWrapMode(QTextEdit::NoWrap); _ui->promptTextEdit->setLineWrapMode(QTextEdit::NoWrap);
_ui->promptTextEdit->setWordWrapMode(QTextOption::NoWrap); _ui->promptTextEdit->setWordWrapMode(QTextOption::NoWrap);
@ -90,36 +89,37 @@ JSConsole::JSConsole(QWidget* parent, ScriptEngine* scriptEngine) :
} }
JSConsole::~JSConsole() { JSConsole::~JSConsole() {
disconnect(_scriptEngine, SIGNAL(printedMessage(const QString&)), this, SLOT(handlePrint(const QString&))); if (_scriptEngine) {
disconnect(_scriptEngine, SIGNAL(errorMessage(const QString&)), this, SLOT(handleError(const QString&))); disconnect(_scriptEngine.data(), SIGNAL(printedMessage(const QString&)), this, SLOT(handlePrint(const QString&)));
if (_ownScriptEngine) { disconnect(_scriptEngine.data(), SIGNAL(errorMessage(const QString&)), this, SLOT(handleError(const QString&)));
_scriptEngine->deleteLater(); _scriptEngine.reset();
} }
delete _ui; delete _ui;
} }
void JSConsole::setScriptEngine(ScriptEngine* scriptEngine) { void JSConsole::setScriptEngine(const QSharedPointer<ScriptEngine>& scriptEngine) {
if (_scriptEngine == scriptEngine && scriptEngine != NULL) { if (_scriptEngine == scriptEngine && scriptEngine != NULL) {
return; return;
} }
if (_scriptEngine != NULL) { if (_scriptEngine != NULL) {
disconnect(_scriptEngine, &ScriptEngine::printedMessage, this, &JSConsole::handlePrint); disconnect(_scriptEngine.data(), &ScriptEngine::printedMessage, this, &JSConsole::handlePrint);
disconnect(_scriptEngine, &ScriptEngine::infoMessage, this, &JSConsole::handleInfo); disconnect(_scriptEngine.data(), &ScriptEngine::infoMessage, this, &JSConsole::handleInfo);
disconnect(_scriptEngine, &ScriptEngine::warningMessage, this, &JSConsole::handleWarning); disconnect(_scriptEngine.data(), &ScriptEngine::warningMessage, this, &JSConsole::handleWarning);
disconnect(_scriptEngine, &ScriptEngine::errorMessage, this, &JSConsole::handleError); disconnect(_scriptEngine.data(), &ScriptEngine::errorMessage, this, &JSConsole::handleError);
if (_ownScriptEngine) { _scriptEngine.reset();
_scriptEngine->deleteLater();
}
} }
// if scriptEngine is NULL then create one and keep track of it using _ownScriptEngine // if scriptEngine is NULL then create one and keep track of it using _ownScriptEngine
_ownScriptEngine = (scriptEngine == NULL); if (scriptEngine.isNull()) {
_scriptEngine = _ownScriptEngine ? DependencyManager::get<ScriptEngines>()->loadScript(_consoleFileName, false) : scriptEngine; _scriptEngine = QSharedPointer<ScriptEngine>(DependencyManager::get<ScriptEngines>()->loadScript(_consoleFileName, false), &QObject::deleteLater);
} else {
_scriptEngine = scriptEngine;
}
connect(_scriptEngine, &ScriptEngine::printedMessage, this, &JSConsole::handlePrint); connect(_scriptEngine.data(), &ScriptEngine::printedMessage, this, &JSConsole::handlePrint);
connect(_scriptEngine, &ScriptEngine::infoMessage, this, &JSConsole::handleInfo); connect(_scriptEngine.data(), &ScriptEngine::infoMessage, this, &JSConsole::handleInfo);
connect(_scriptEngine, &ScriptEngine::warningMessage, this, &JSConsole::handleWarning); connect(_scriptEngine.data(), &ScriptEngine::warningMessage, this, &JSConsole::handleWarning);
connect(_scriptEngine, &ScriptEngine::errorMessage, this, &JSConsole::handleError); connect(_scriptEngine.data(), &ScriptEngine::errorMessage, this, &JSConsole::handleError);
} }
void JSConsole::executeCommand(const QString& command) { void JSConsole::executeCommand(const QString& command) {
@ -135,19 +135,22 @@ void JSConsole::executeCommand(const QString& command) {
appendMessage(">", "<span style='" + COMMAND_STYLE + "'>" + command.toHtmlEscaped() + "</span>"); appendMessage(">", "<span style='" + COMMAND_STYLE + "'>" + command.toHtmlEscaped() + "</span>");
QFuture<QScriptValue> future = QtConcurrent::run(this, &JSConsole::executeCommandInWatcher, command); QWeakPointer<ScriptEngine> weakScriptEngine = _scriptEngine;
auto consoleFileName = _consoleFileName;
QFuture<QScriptValue> future = QtConcurrent::run([weakScriptEngine, consoleFileName, command]()->QScriptValue{
QScriptValue result;
auto scriptEngine = weakScriptEngine.lock();
if (scriptEngine) {
BLOCKING_INVOKE_METHOD(scriptEngine.data(), "evaluate",
Q_RETURN_ARG(QScriptValue, result),
Q_ARG(const QString&, command),
Q_ARG(const QString&, consoleFileName));
}
return result;
});
_executeWatcher.setFuture(future); _executeWatcher.setFuture(future);
} }
QScriptValue JSConsole::executeCommandInWatcher(const QString& command) {
QScriptValue result;
BLOCKING_INVOKE_METHOD(_scriptEngine, "evaluate",
Q_RETURN_ARG(QScriptValue, result),
Q_ARG(const QString&, command),
Q_ARG(const QString&, _consoleFileName));
return result;
}
void JSConsole::commandFinished() { void JSConsole::commandFinished() {
QScriptValue result = _executeWatcher.result(); QScriptValue result = _executeWatcher.result();

View file

@ -17,6 +17,7 @@
#include <QFutureWatcher> #include <QFutureWatcher>
#include <QObject> #include <QObject>
#include <QWidget> #include <QWidget>
#include <QSharedPointer>
#include "ui_console.h" #include "ui_console.h"
#include "ScriptEngine.h" #include "ScriptEngine.h"
@ -29,10 +30,10 @@ const int CONSOLE_HEIGHT = 200;
class JSConsole : public QWidget { class JSConsole : public QWidget {
Q_OBJECT Q_OBJECT
public: public:
JSConsole(QWidget* parent, ScriptEngine* scriptEngine = NULL); JSConsole(QWidget* parent, const QSharedPointer<ScriptEngine>& scriptEngine = QSharedPointer<ScriptEngine>());
~JSConsole(); ~JSConsole();
void setScriptEngine(ScriptEngine* scriptEngine = NULL); void setScriptEngine(const QSharedPointer<ScriptEngine>& scriptEngine = QSharedPointer<ScriptEngine>());
void clear(); void clear();
public slots: public slots:
@ -58,17 +59,14 @@ private:
void setToNextCommandInHistory(); void setToNextCommandInHistory();
void setToPreviousCommandInHistory(); void setToPreviousCommandInHistory();
void resetCurrentCommandHistory(); void resetCurrentCommandHistory();
QScriptValue executeCommandInWatcher(const QString& command);
QFutureWatcher<QScriptValue> _executeWatcher; QFutureWatcher<QScriptValue> _executeWatcher;
Ui::Console* _ui; Ui::Console* _ui;
int _currentCommandInHistory; int _currentCommandInHistory;
QString _savedHistoryFilename; QString _savedHistoryFilename;
QList<QString> _commandHistory; QList<QString> _commandHistory;
// Keeps track if the script engine is created inside the JSConsole
bool _ownScriptEngine;
QString _rootCommand; QString _rootCommand;
ScriptEngine* _scriptEngine; QSharedPointer<ScriptEngine> _scriptEngine;
static const QString _consoleFileName; static const QString _consoleFileName;
}; };

View file

@ -24,12 +24,12 @@ TestingDialog::TestingDialog(QWidget* parent) :
_console->setFixedHeight(TESTING_CONSOLE_HEIGHT); _console->setFixedHeight(TESTING_CONSOLE_HEIGHT);
auto _engines = DependencyManager::get<ScriptEngines>(); auto _engines = DependencyManager::get<ScriptEngines>();
_engine = _engines->loadScript(qApp->applicationDirPath() + testRunnerRelativePath); _engine.reset(_engines->loadScript(qApp->applicationDirPath() + testRunnerRelativePath));
_console->setScriptEngine(_engine); _console->setScriptEngine(_engine);
connect(_engine, &ScriptEngine::finished, this, &TestingDialog::onTestingFinished); connect(_engine.data(), &ScriptEngine::finished, this, &TestingDialog::onTestingFinished);
} }
void TestingDialog::onTestingFinished(const QString& scriptPath) { void TestingDialog::onTestingFinished(const QString& scriptPath) {
_engine = nullptr; _engine.reset();
_console->setScriptEngine(nullptr); _console->setScriptEngine();
} }

View file

@ -29,7 +29,7 @@ public:
private: private:
std::unique_ptr<JSConsole> _console; std::unique_ptr<JSConsole> _console;
ScriptEngine* _engine; QSharedPointer<ScriptEngine> _engine;
}; };
#endif #endif