diff options
author | Arttu Tarkiainen <arttu.tarkiainen@qt.io> | 2019-10-16 16:17:43 +0300 |
---|---|---|
committer | Katja Marttila <katja.marttila@qt.io> | 2019-11-01 06:43:51 +0000 |
commit | 1cb9feb9a20df8954e7e281d053e13d81f5c46ee (patch) | |
tree | 4683024d02fe479fc0b4dd6bd4c743f5a99c6035 /src | |
parent | 7151a0b087e6788c01f26c25e727297f2601be8c (diff) |
Fix extracted files list formation in ExtractArchive operation
In ExtractArchive, the ConnectionType used to connect signal
'currentFileChanged' to slot 'fileFinished' was changed to
Qt::AutoConnection in c8de51cadbc5855ca1e77d038d7f09bf60d059ee,
rather than using Qt::DirectConnection explicitly.
Now the connection type defaults to queued as the sender and
receiver live on a different thread. This seems to cause an issue
where the slot function is not invoked for every emitted
'currentFileChanged' signal before returning from
ExtractArchiveOperation::performOperation(). This can happen
randomly on some installer runs, leaving entries out from
ExtractArchive's "files" list for some components.
The list formation works fine with Qt::DirectConnection, however
reverting to this behavior might be a bad practice as it leaves
'fileFinished' vulnerable to accidental calls from another thread
if we change the behavior in the future. Rather move the extracted
files list formation to ExtractArchiveOperation::Callback like we
already do for backup files.
Task-number: QTIFW-1239
Change-Id: I7dbe73abefe00814e8652432d8abdbcaa83e0bac
Reviewed-by: Iikka Eklund <iikka.eklund@qt.io>
Reviewed-by: Katja Marttila <katja.marttila@qt.io>
Diffstat (limited to 'src')
-rw-r--r-- | src/libs/installer/extractarchiveoperation.cpp | 20 | ||||
-rw-r--r-- | src/libs/installer/extractarchiveoperation.h | 4 | ||||
-rw-r--r-- | src/libs/installer/extractarchiveoperation_p.h | 8 |
3 files changed, 11 insertions, 21 deletions
diff --git a/src/libs/installer/extractarchiveoperation.cpp b/src/libs/installer/extractarchiveoperation.cpp index 12067a7c6..1c3e1b78a 100644 --- a/src/libs/installer/extractarchiveoperation.cpp +++ b/src/libs/installer/extractarchiveoperation.cpp @@ -59,7 +59,6 @@ bool ExtractArchiveOperation::performOperation() Receiver receiver; Callback callback; - connect(&callback, &Callback::currentFileChanged, this, &ExtractArchiveOperation::fileFinished); connect(&callback, &Callback::progressChanged, this, &ExtractArchiveOperation::progressChanged); if (PackageManagerCore *core = packageManager()) { @@ -70,8 +69,6 @@ bool ExtractArchiveOperation::performOperation() connect(runnable, &Runnable::finished, &receiver, &Receiver::runnableFinished, Qt::QueuedConnection); - m_files.clear(); - QFileInfo fileInfo(archivePath); emit outputTextChanged(tr("Extracting \"%1\"").arg(fileInfo.fileName())); @@ -96,6 +93,8 @@ bool ExtractArchiveOperation::performOperation() // -<component_name> (dir) // -<filename>.txt (file) + QStringList files = callback.extractedFiles(); + QString fileDirectory = targetDir + QLatin1String("/installerResources/") + archivePath.section(QLatin1Char('/'), 1, 1, QString::SectionSkipEmpty) + QLatin1Char('/'); QString archiveFileName = archivePath.section(QLatin1Char('/'), 2, 2, QString::SectionSkipEmpty); @@ -114,10 +113,10 @@ bool ExtractArchiveOperation::performOperation() if (file.open(QIODevice::WriteOnly)) { setDefaultFilePermissions(file.fileName(), DefaultFilePermissions::NonExecutable); QDataStream out (&file); - for (int i = 0; i < m_files.count(); ++i) { - m_files[i] = replacePath(m_files.at(i), targetDir, QLatin1String(scRelocatable)); + for (int i = 0; i < files.count(); ++i) { + files[i] = replacePath(files.at(i), targetDir, QLatin1String(scRelocatable)); } - out << m_files; + out << files; setValue(QLatin1String("files"), file.fileName()); file.close(); } else { @@ -199,13 +198,4 @@ bool ExtractArchiveOperation::testOperation() return true; } -/*! - This slot is direct connected to the caller so please don't call it from another thread in the - same time. -*/ -void ExtractArchiveOperation::fileFinished(const QString &filename) -{ - m_files.prepend(filename); -} - } // namespace QInstaller diff --git a/src/libs/installer/extractarchiveoperation.h b/src/libs/installer/extractarchiveoperation.h index 0f584dc0a..e706159ed 100644 --- a/src/libs/installer/extractarchiveoperation.h +++ b/src/libs/installer/extractarchiveoperation.h @@ -55,11 +55,7 @@ Q_SIGNALS: private: void startUndoProcess(const QStringList &files); -private Q_SLOTS: - void fileFinished(const QString &progress); - private: - QStringList m_files; class Callback; class Runnable; class Receiver; diff --git a/src/libs/installer/extractarchiveoperation_p.h b/src/libs/installer/extractarchiveoperation_p.h index f333da366..9cc07246f 100644 --- a/src/libs/installer/extractarchiveoperation_p.h +++ b/src/libs/installer/extractarchiveoperation_p.h @@ -97,6 +97,10 @@ public: return m_backupFiles; } + QStringList extractedFiles() const { + return m_extractedFiles; + } + public slots: void statusChanged(QInstaller::PackageManagerCore::Status status) { @@ -113,13 +117,12 @@ public slots: } signals: - void currentFileChanged(const QString &filename); void progressChanged(double progress); private: void setCurrentFile(const QString &filename) Q_DECL_OVERRIDE { - emit currentFileChanged(QDir::toNativeSeparators(filename)); + m_extractedFiles.prepend(QDir::toNativeSeparators(filename)); } static QString generateBackupName(const QString &fn) @@ -157,6 +160,7 @@ private: private: HRESULT m_state = S_OK; BackupFiles m_backupFiles; + QStringList m_extractedFiles; }; class ExtractArchiveOperation::Runnable : public QObject, public QRunnable |