QmlProfiler: Avoid race conditions when loading or saving data

We cannot assume that the model manager is still available when the
operation finishes. Also, accessing the QML model or the trace time
from the thread is dangerous.

Change-Id: I673c57c09490a0e3e2647f3197929eff1ce4ceb3
Reviewed-by: Eike Ziller <eike.ziller@theqtcompany.com>

3 files changed, 46 insertions, 32 deletions

diff --git a/src/plugins/qmlprofiler/qmlprofilermodelmanager.cpp b/src/plugins/qmlprofiler/qmlprofilermodelmanager.cpp
index 0c3914b083..84ec468140 100644
--- a/src/plugins/qmlprofiler/qmlprofilermodelmanager.cpp
+++ b/src/plugins/qmlprofiler/qmlprofilermodelmanager.cpp
@@ -338,16 +338,20 @@ void QmlProfilerModelManager::save(const QString &filename)
 
     d->notesModel->saveData();
 
-    QFuture<void> result = Utils::runAsync<void>([this, file] (QFutureInterface<void> &future) {
-        QmlProfilerFileWriter writer;
-        writer.setTraceTime(traceTime()->startTime(), traceTime()->endTime(),
-                            traceTime()->duration());
-        writer.setQmlEvents(d->model->getEventTypes(), d->model->getEvents());
-        writer.setNotes(d->model->getEventNotes());
-        writer.setFuture(&future);
-        writer.save(file);
+    QmlProfilerFileWriter *writer = new QmlProfilerFileWriter(this);
+    writer->setTraceTime(traceTime()->startTime(), traceTime()->endTime(),
+                        traceTime()->duration());
+    writer->setQmlEvents(d->model->getEventTypes(), d->model->getEvents());
+    writer->setNotes(d->model->getEventNotes());
+
+    connect(writer, &QObject::destroyed, this, &QmlProfilerModelManager::saveFinished,
+            Qt::QueuedConnection);
+
+    QFuture<void> result = Utils::runAsync<void>([file, writer] (QFutureInterface<void> &future) {
+        writer->setFuture(&future);
+        writer->save(file);
+        delete writer;
         file->deleteLater();
-        QMetaObject::invokeMethod(this, "saveFinished", Qt::QueuedConnection);
     });
 
     Core::ProgressManager::addTask(result, tr("Saving Trace Data"), Constants::TASK_SAVE,
@@ -366,24 +370,33 @@ void QmlProfilerModelManager::load(const QString &filename)
 
     clear();
     setState(AcquiringData);
+    QmlProfilerFileReader *reader = new QmlProfilerFileReader(this);
+
+    connect(reader, &QmlProfilerFileReader::error, this, [this, reader](const QString &message) {
+        delete reader;
+        emit error(message);
+    }, Qt::QueuedConnection);
+
+    connect(reader, &QmlProfilerFileReader::success, this, [this, reader]() {
+        d->model->setData(reader->traceStart(), qMax(reader->traceStart(), reader->traceEnd()),
+                          reader->qmlEvents(), reader->ranges());
+        d->model->setNoteData(reader->notes());
+        setRecordedFeatures(reader->loadedFeatures());
+        d->traceTime->increaseEndTime(d->model->lastTimeMark());
+        delete reader;
+        acquiringDone();
+    }, Qt::QueuedConnection);
 
-    QFuture<void> result = Utils::runAsync<void>([this, file] (QFutureInterface<void> &future) {
-        QmlProfilerFileReader reader;
-        reader.setFuture(&future);
-        connect(&reader, &QmlProfilerFileReader::error, this, &QmlProfilerModelManager::error);
-        reader.setQmlDataModel(d->model);
-        reader.load(file);
-        setRecordedFeatures(reader.loadedFeatures());
+    QFuture<void> result = Utils::runAsync<void>([file, reader] (QFutureInterface<void> &future) {
+        reader->setFuture(&future);
+        reader->load(file);
         file->close();
         file->deleteLater();
-        d->traceTime->increaseEndTime(d->model->lastTimeMark());
-        acquiringDone();
     });
 
     Core::ProgressManager::addTask(result, tr("Loading Trace Data"), Constants::TASK_LOAD);
 }
 
-
 void QmlProfilerModelManager::setState(QmlProfilerModelManager::State state)
 {
     // It's not an error, we are continuously calling "AcquiringData" for example
diff --git a/src/plugins/qmlprofiler/qmlprofilertracefile.cpp b/src/plugins/qmlprofiler/qmlprofilertracefile.cpp
index 4c58cf17e7..afe6186967 100644
--- a/src/plugins/qmlprofiler/qmlprofilertracefile.cpp
+++ b/src/plugins/qmlprofiler/qmlprofilertracefile.cpp
@@ -117,16 +117,13 @@ static QString qmlTypeAsString(Message message, RangeType rangeType)
 
 QmlProfilerFileReader::QmlProfilerFileReader(QObject *parent) :
     QObject(parent),
+    m_traceStart(-1),
+    m_traceEnd(-1),
     m_future(0),
     m_loadedFeatures(0)
 {
 }
 
-void QmlProfilerFileReader::setQmlDataModel(QmlProfilerDataModel *dataModel)
-{
-    m_qmlModel = dataModel;
-}
-
 void QmlProfilerFileReader::setFuture(QFutureInterface<void> *future)
 {
     m_future = future;
@@ -142,8 +139,6 @@ bool QmlProfilerFileReader::load(QIODevice *device)
     QXmlStreamReader stream(device);
 
     bool validVersion = true;
-    qint64 traceStart = -1;
-    qint64 traceEnd = -1;
 
     while (validVersion && !stream.atEnd() && !stream.hasError()) {
         if (isCanceled())
@@ -160,9 +155,9 @@ bool QmlProfilerFileReader::load(QIODevice *device)
                 else
                     validVersion = false;
                 if (attributes.hasAttribute(_("traceStart")))
-                    traceStart = attributes.value(_("traceStart")).toLongLong();
+                    m_traceStart = attributes.value(_("traceStart")).toLongLong();
                 if (attributes.hasAttribute(_("traceEnd")))
-                    traceEnd = attributes.value(_("traceEnd")).toLongLong();
+                    m_traceEnd = attributes.value(_("traceEnd")).toLongLong();
             }
 
             if (elementName == _("eventData")) {
@@ -190,8 +185,7 @@ bool QmlProfilerFileReader::load(QIODevice *device)
         emit error(tr("Error while parsing trace data file: %1").arg(stream.errorString()));
         return false;
     } else {
-        m_qmlModel->setData(traceStart, qMax(traceStart, traceEnd), m_qmlEvents, m_ranges);
-        m_qmlModel->setNoteData(m_notes);
+        emit success();
         return true;
     }
 }
diff --git a/src/plugins/qmlprofiler/qmlprofilertracefile.h b/src/plugins/qmlprofiler/qmlprofilertracefile.h
index 5a5080c1be..f29b04634d 100644
--- a/src/plugins/qmlprofiler/qmlprofilertracefile.h
+++ b/src/plugins/qmlprofiler/qmlprofilertracefile.h
@@ -50,14 +50,21 @@ class QmlProfilerFileReader : public QObject
 public:
     explicit QmlProfilerFileReader(QObject *parent = 0);
 
-    void setQmlDataModel(QmlProfilerDataModel *dataModel);
     void setFuture(QFutureInterface<void> *future);
 
     bool load(QIODevice *device);
     quint64 loadedFeatures() const;
 
+    qint64 traceStart() const { return m_traceStart; }
+    qint64 traceEnd() const { return m_traceEnd; }
+
+    const QVector<QmlProfilerDataModel::QmlEventTypeData> &qmlEvents() const { return m_qmlEvents; }
+    const QVector<QmlProfilerDataModel::QmlEventData> &ranges() const { return m_ranges; }
+    const QVector<QmlProfilerDataModel::QmlEventNoteData> &notes() const { return m_notes; }
+
 signals:
     void error(const QString &error);
+    void success();
 
 private:
     void loadEventData(QXmlStreamReader &reader);
@@ -66,7 +73,7 @@ private:
     void progress(QIODevice *device);
     bool isCanceled() const;
 
-    QmlProfilerDataModel *m_qmlModel;
+    qint64 m_traceStart, m_traceEnd;
     QFutureInterface<void> *m_future;
     QVector<QmlProfilerDataModel::QmlEventTypeData> m_qmlEvents;
     QVector<QmlProfilerDataModel::QmlEventData> m_ranges;