yukijoou/kwin
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

[scripting] Avoid threading issues when loading from a file

Browse source 
Summary:
KWin::Script::loadScriptFromFile ran in it's own thread and accessed
member variables of KWin::Script without any guards.

Potentially script could be destroyed whilst the file is loading.

Rather than adding mutexes everywhere, this patch scopes the QFile
object to be local to the threaded function making it independent.

BUG: 403038

Test Plan: Ran a script from a file

Reviewers: #kwin, graesslin

Reviewed By: #kwin, graesslin

Subscribers: kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D18126
This commit is contained in:
David Edmundson 2019-01-09 19:40:09 +00:00
parent fbe219172a
commit ba7aecfe53
2 changed files with 11 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
scripting/scripting.cpp
View file

@ -231,9 +231,9 @@ KWin::AbstractScript::AbstractScript(int id, QString scriptName, QString pluginN
: QObject(parent) : QObject(parent)
, m_scriptId(id) , m_scriptId(id)
, m_pluginName(pluginName) , m_pluginName(pluginName)
, m_fileName(scriptName)
, m_running(false) , m_running(false)
{ {
m_scriptFile.setFileName(scriptName);
if (m_pluginName.isNull()) { if (m_pluginName.isNull()) {
m_pluginName = scriptName; m_pluginName = scriptName;
} }
@ -255,7 +255,7 @@ void KWin::AbstractScript::stop()
void KWin::AbstractScript::printMessage(const QString &message) void KWin::AbstractScript::printMessage(const QString &message)
{ {
qCDebug(KWIN_SCRIPTING) << scriptFile().fileName() << ":" << message; qCDebug(KWIN_SCRIPTING) << fileName() << ":" << message;
emit print(message); emit print(message);
} }
@ -468,16 +468,16 @@ void KWin::Script::run()
m_starting = true; m_starting = true;
QFutureWatcher<QByteArray> *watcher = new QFutureWatcher<QByteArray>(this); QFutureWatcher<QByteArray> *watcher = new QFutureWatcher<QByteArray>(this);
connect(watcher, SIGNAL(finished()), SLOT(slotScriptLoadedFromFile())); connect(watcher, SIGNAL(finished()), SLOT(slotScriptLoadedFromFile()));
watcher->setFuture(QtConcurrent::run(this, &KWin::Script::loadScriptFromFile)); watcher->setFuture(QtConcurrent::run(this, &KWin::Script::loadScriptFromFile, fileName()));
} }
QByteArray KWin::Script::loadScriptFromFile() QByteArray KWin::Script::loadScriptFromFile(const QString &fileName)
{ {
if (!scriptFile().open(QIODevice::ReadOnly)) { QFile file(fileName);
if (!file.open(QIODevice::ReadOnly)) {
return QByteArray(); return QByteArray();
} }
QByteArray result(scriptFile().readAll()); QByteArray result(file.readAll());
scriptFile().close();
return result; return result;
} }
@ -591,7 +591,7 @@ void KWin::DeclarativeScript::run()
return; return;
} }
m_component->loadUrl(QUrl::fromLocalFile(scriptFile().fileName())); m_component->loadUrl(QUrl::fromLocalFile(fileName()));
if (m_component->isLoading()) { if (m_component->isLoading()) {
connect(m_component, &QQmlComponent::statusChanged, this, &DeclarativeScript::createComponent); connect(m_component, &QQmlComponent::statusChanged, this, &DeclarativeScript::createComponent);
} else { } else {

9
scripting/scripting.h
View file

@ -60,7 +60,7 @@ public:
AbstractScript(int id, QString scriptName, QString pluginName, QObject *parent = nullptr); AbstractScript(int id, QString scriptName, QString pluginName, QObject *parent = nullptr);
~AbstractScript(); ~AbstractScript();
QString fileName() const { QString fileName() const {
return m_scriptFile.fileName(); return m_fileName;
} }
const QString &pluginName() { const QString &pluginName() {
return m_pluginName; return m_pluginName;
@ -153,9 +153,6 @@ Q_SIGNALS:
void runningChanged(bool); void runningChanged(bool);
protected: protected:
QFile &scriptFile() {
return m_scriptFile;
}
bool running() const { bool running() const {
return m_running; return m_running;
} }
@ -204,7 +201,7 @@ private:
**/ **/
QAction *createMenu(const QString &title, QScriptValue &items, QMenu *parent); QAction *createMenu(const QString &title, QScriptValue &items, QMenu *parent);
int m_scriptId; int m_scriptId;
QFile m_scriptFile; QString m_fileName;
QString m_pluginName; QString m_pluginName;
bool m_running; bool m_running;
QHash<QAction*, QScriptValue> m_shortcutCallbacks; QHash<QAction*, QScriptValue> m_shortcutCallbacks;
@ -255,7 +252,7 @@ private:
* Read the script from file into a byte array. * Read the script from file into a byte array.
* If file cannot be read an empty byte array is returned. * If file cannot be read an empty byte array is returned.
**/ **/
QByteArray loadScriptFromFile(); QByteArray loadScriptFromFile(const QString &fileName);
QScriptEngine *m_engine; QScriptEngine *m_engine;
bool m_starting; bool m_starting;
QScopedPointer<ScriptUnloaderAgent> m_agent; QScopedPointer<ScriptUnloaderAgent> m_agent;