4
0
mirror of https://github.com/QuasarApp/installer-framework.git synced 2025-05-06 18:19:33 +00:00

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>
This commit is contained in:
Arttu Tarkiainen 2019-10-16 16:17:43 +03:00 committed by Katja Marttila
parent 7151a0b087
commit 1cb9feb9a2
3 changed files with 11 additions and 21 deletions

@ -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

@ -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;

@ -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