Move QJSEngine into the thread which executes the JavaScript

This fixes https://github.com/Martchus/tageditor/issues/50.

When keeping the QJSEngine tied to the main thread the garbage
collector will run on the main thread's event loop. This leads to
crashes when trying to allocate memory within the engine from
another thread (`QV4::PersistentValueStorage::allocate()`).

The Qt documentation does not mention that the garbage collector
might run on the event loop of the thread tied to the JSEngine.
I expected it only to run after or before allocations/deletions
within the thread calling the engine's methods. There is already
an issue regarding the lack of documentation:
https://bugreports.qt.io/browse/QTBUG-57227

I found no way to obtain the QThread object for a thread started
with Qt Concurrent. The possibility I found was calling
`QThread::currentThread()` from the concurrent thread once it has
already been started. However, when the concurrent thread has
been started it might already be too late to move the engine.
Adding further synchronization to solve this is an overkill so
I resorted to using QThread directly.
This commit is contained in:
Martchus 2019-10-11 19:47:30 +02:00
parent bf4687d0b6
commit 4531d10a81
2 changed files with 87 additions and 17 deletions

View File

@ -5,7 +5,6 @@
#include <QDir>
#include <QStringBuilder>
#include <QtConcurrent>
#include <memory>
@ -77,13 +76,8 @@ bool RenamingEngine::generatePreview(const QDir &rootDirectory, bool includeSubd
setRootItem();
m_includeSubdirs = includeSubdirs;
m_dir = rootDirectory;
QtConcurrent::run([this]() {
m_aborted.store(false);
m_itemsProcessed = 0;
m_errorsOccured = 0;
m_newlyGeneratedRootItem = generatePreview(m_dir);
emit previewGenerated();
});
(new PreviewGenerator(this))->start();
return m_isBusy = true;
#else
return false;
@ -95,13 +89,7 @@ bool RenamingEngine::applyChangings()
if (!m_rootItem || m_isBusy) {
return false;
}
QtConcurrent::run([this]() {
m_aborted.store(false);
m_itemsProcessed = 0;
m_errorsOccured = 0;
applyChangings(m_rootItem.get());
emit changingsApplied();
});
(new RenamingThing(this))->start();
return m_isBusy = true;
}
@ -144,17 +132,30 @@ FilteredFileSystemItemModel *RenamingEngine::previewModel()
void RenamingEngine::processPreviewGenerated()
{
m_isBusy = false;
finalizeTaskCompletion();
setRootItem(move(m_newlyGeneratedRootItem));
}
void RenamingEngine::processChangingsApplied()
{
m_isBusy = false;
finalizeTaskCompletion();
updateModel(nullptr);
updateModel(m_rootItem.get());
}
void RenamingEngine::resetStatus()
{
m_aborted.store(false);
m_itemsProcessed = 0;
m_errorsOccured = 0;
}
void RenamingEngine::finalizeTaskCompletion()
{
m_engine.moveToThread(thread());
m_isBusy = false;
}
inline void RenamingEngine::setRootItem(unique_ptr<FileSystemItem> &&rootItem)
{
updateModel(rootItem.get());
@ -288,6 +289,7 @@ void RenamingEngine::executeScriptForItem(const QFileInfo &fileInfo, FileSystemI
{
// make file info for the specified item available in the script
m_tagEditorQObj->setFileInfo(fileInfo, item);
// execute script
const auto scriptResult(m_program.call());
if (scriptResult.isError()) {
@ -347,6 +349,42 @@ void RenamingEngine::executeScriptForItem(const QFileInfo &fileInfo, FileSystemI
item->setNote(m_tagEditorQObj->note().isEmpty() ? tr("skipped") : m_tagEditorQObj->note());
}
}
PreviewGenerator::PreviewGenerator(RenamingEngine *engine)
: QThread(engine)
, m_engine(engine)
{
m_engine->m_engine.moveToThread(this);
connect(this, &PreviewGenerator::finished, m_engine, &RenamingEngine::previewGenerated, Qt::QueuedConnection);
connect(this, &PreviewGenerator::finished, this, &PreviewGenerator::deleteLater);
}
PreviewGenerator::~PreviewGenerator()
{
printf("preview gone\n");
}
void PreviewGenerator::run()
{
m_engine->resetStatus();
m_engine->m_newlyGeneratedRootItem = m_engine->generatePreview(m_engine->m_dir);
}
RenamingThing::RenamingThing(RenamingEngine *engine)
: QThread(engine)
, m_engine(engine)
{
m_engine->m_engine.moveToThread(this);
connect(this, &RenamingThing::finished, m_engine, &RenamingEngine::changingsApplied, Qt::QueuedConnection);
connect(this, &RenamingThing::finished, this, &RenamingThing::deleteLater);
}
void RenamingThing::run()
{
m_engine->resetStatus();
m_engine->applyChangings(m_engine->m_rootItem.get());
}
#endif
} // namespace RenamingUtility

View File

@ -9,6 +9,7 @@
#include <QDir>
#include <QList>
#include <QObject>
#include <QThread>
#include <memory>
@ -19,10 +20,39 @@ namespace RenamingUtility {
class FileSystemItemModel;
class FilteredFileSystemItemModel;
class TagEditorObject;
class RenamingEngine;
class PreviewGenerator final : public QThread {
Q_OBJECT
public:
explicit PreviewGenerator(RenamingEngine *engine);
~PreviewGenerator() override;
protected:
void run() final;
private:
RenamingEngine *m_engine;
};
class RenamingThing final : public QThread {
Q_OBJECT
public:
explicit RenamingThing(RenamingEngine *engine);
protected:
void run() final;
private:
RenamingEngine *m_engine;
};
class RenamingEngine : public QObject {
Q_OBJECT
friend class PreviewGenerator;
friend class RenamingThing;
public:
explicit RenamingEngine(QObject *parent = nullptr);
@ -58,6 +88,8 @@ private slots:
void processChangingsApplied();
private:
void resetStatus();
void finalizeTaskCompletion();
void setRootItem(std::unique_ptr<FileSystemItem> &&rootItem = std::unique_ptr<FileSystemItem>());
void updateModel(FileSystemItem *rootItem);
#ifndef TAGEDITOR_NO_JSENGINE