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>

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 = new QmlProfilerFileWriter(this);
-        QmlProfilerFileWriter writer;
+    writer->setTraceTime(traceTime()->startTime(), traceTime()->endTime(),
-        writer.setTraceTime(traceTime()->startTime(), traceTime()->endTime(),
                         traceTime()->duration());
-        writer.setQmlEvents(d->model->getEventTypes(), d->model->getEvents());
+    writer->setQmlEvents(d->model->getEventTypes(), d->model->getEvents());
-        writer.setNotes(d->model->getEventNotes());
+    writer->setNotes(d->model->getEventNotes());
-        writer.setFuture(&future);
+
-        writer.save(file);
+    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);
 
-    QFuture<void> result = Utils::runAsync<void>([this, file] (QFutureInterface<void> &future) {
+    connect(reader, &QmlProfilerFileReader::error, this, [this, reader](const QString &message) {
-        QmlProfilerFileReader reader;
+        delete reader;
-        reader.setFuture(&future);
+        emit error(message);
-        connect(&reader, &QmlProfilerFileReader::error, this, &QmlProfilerModelManager::error);
+    }, Qt::QueuedConnection);
-        reader.setQmlDataModel(d->model);
+
-        reader.load(file);
+    connect(reader, &QmlProfilerFileReader::success, this, [this, reader]() {
-        setRecordedFeatures(reader.loadedFeatures());
+        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>([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

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);
+        emit success();
-        m_qmlModel->setNoteData(m_notes);
         return true;
     }
 }

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;