summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorElvis Angelaccio <elvis.angelaccio@kde.org>2016-09-14 14:05:02 (GMT)
committerElvis Angelaccio <elvis.angelaccio@kde.org>2016-09-14 14:13:38 (GMT)
commit8ad610bf1a5e08e68d89080bb81f036788867718 (patch)
tree55e1fd4f2df0a834f1a26b48d06ede74daf5c669
parent5927ecbd19f7b6c3b9873278158c5daa3666ee2a (diff)
Fix race condition when killing jobs
The `m_abortOperation` global variable was set by the main thread and read by the secondary thread, i.e. undefined behavior. QThread::requestInterruption() and isInterruptionRequest() are instead safe because they use a QMutexLocker. Tested only with ListJobs, are other type of jobs are currently broken (see e.g. bugs #365869 and #365870). Closes T3598
-rw-r--r--kerfuffle/jobs.cpp5
-rw-r--r--plugins/libarchive/libarchiveplugin.cpp10
-rw-r--r--plugins/libarchive/libarchiveplugin.h1
-rw-r--r--plugins/libarchive/readwritelibarchiveplugin.cpp9
4 files changed, 12 insertions, 13 deletions
diff --git a/kerfuffle/jobs.cpp b/kerfuffle/jobs.cpp
index 1e396f1..d280937 100644
--- a/kerfuffle/jobs.cpp
+++ b/kerfuffle/jobs.cpp
@@ -167,6 +167,11 @@ void Job::onUserQuery(Query *query)
bool Job::doKill()
{
+ if (d->isRunning()) {
+ d->requestInterruption();
+ d->wait();
+ }
+
bool ret = archiveInterface()->doKill();
if (!ret) {
qCWarning(ARK) << "Killing does not seem to be supported here.";
diff --git a/plugins/libarchive/libarchiveplugin.cpp b/plugins/libarchive/libarchiveplugin.cpp
index ed81152..9f6baae 100644
--- a/plugins/libarchive/libarchiveplugin.cpp
+++ b/plugins/libarchive/libarchiveplugin.cpp
@@ -32,11 +32,11 @@
#include <KLocalizedString>
#include <QDirIterator>
+#include <QThread>
LibarchivePlugin::LibarchivePlugin(QObject *parent, const QVariantList &args)
: ReadWriteArchiveInterface(parent, args)
, m_archiveReadDisk(archive_read_disk_new())
- , m_abortOperation(false)
, m_cachedArchiveEntryCount(0)
, m_emitNoEntries(false)
, m_extractedFilesSize(0)
@@ -66,7 +66,7 @@ bool LibarchivePlugin::list()
int result = ARCHIVE_RETRY;
bool firstEntry = true;
- while (!m_abortOperation && (result = archive_read_next_header(m_archiveReader.data(), &aentry)) == ARCHIVE_OK) {
+ while (!QThread::currentThread()->isInterruptionRequested() && (result = archive_read_next_header(m_archiveReader.data(), &aentry)) == ARCHIVE_OK) {
if (firstEntry) {
qDebug(ARK) << "Detected format for first entry:" << archive_format_name(m_archiveReader.data());
@@ -82,7 +82,6 @@ bool LibarchivePlugin::list()
m_cachedArchiveEntryCount++;
archive_read_data_skip(m_archiveReader.data());
}
- m_abortOperation = false;
if (result != ARCHIVE_EOF) {
const QString errorString = QLatin1String(archive_error_string(m_archiveReader.data()));
@@ -137,7 +136,6 @@ bool LibarchivePlugin::testArchive()
bool LibarchivePlugin::doKill()
{
- m_abortOperation = true;
return true;
}
@@ -199,7 +197,7 @@ bool LibarchivePlugin::extractFiles(const QList<Archive::Entry*> &files, const Q
QString fileBeingRenamed;
// Iterate through all entries in archive.
- while (!m_abortOperation && (archive_read_next_header(m_archiveReader.data(), &entry) == ARCHIVE_OK)) {
+ while (!QThread::currentThread()->isInterruptionRequested() && (archive_read_next_header(m_archiveReader.data(), &entry) == ARCHIVE_OK)) {
if (!extractAll && remainingFiles.isEmpty()) {
break;
@@ -384,8 +382,6 @@ bool LibarchivePlugin::extractFiles(const QList<Archive::Entry*> &files, const Q
} // While entries left to read in archive.
- m_abortOperation = false;
-
qCDebug(ARK) << "Extracted" << no_entries << "entries";
return archive_read_close(m_archiveReader.data()) == ARCHIVE_OK;
diff --git a/plugins/libarchive/libarchiveplugin.h b/plugins/libarchive/libarchiveplugin.h
index 56dda50..93a3a12 100644
--- a/plugins/libarchive/libarchiveplugin.h
+++ b/plugins/libarchive/libarchiveplugin.h
@@ -87,7 +87,6 @@ protected:
ArchiveRead m_archiveReader;
ArchiveRead m_archiveReadDisk;
- bool m_abortOperation;
private:
int extractionFlags() const;
diff --git a/plugins/libarchive/readwritelibarchiveplugin.cpp b/plugins/libarchive/readwritelibarchiveplugin.cpp
index 6aa961d..6ed3eb9 100644
--- a/plugins/libarchive/readwritelibarchiveplugin.cpp
+++ b/plugins/libarchive/readwritelibarchiveplugin.cpp
@@ -33,6 +33,7 @@
#include <QDirIterator>
#include <QSaveFile>
+#include <QThread>
K_PLUGIN_FACTORY_WITH_JSON(ReadWriteLibarchivePluginFactory, "kerfuffle_libarchive.json", registerPlugin<ReadWriteLibarchivePlugin>();)
@@ -71,7 +72,7 @@ bool ReadWriteLibarchivePlugin::addFiles(const QList<Archive::Entry*> &files, co
: destination->fullPath();
foreach(Archive::Entry *selectedFile, files) {
- if (m_abortOperation) {
+ if (QThread::currentThread()->isInterruptionRequested()) {
break;
}
@@ -89,7 +90,7 @@ bool ReadWriteLibarchivePlugin::addFiles(const QList<Archive::Entry*> &files, co
QDir::Hidden | QDir::NoDotAndDotDot,
QDirIterator::Subdirectories);
- while (!m_abortOperation && it.hasNext()) {
+ while (!QThread::currentThread()->isInterruptionRequested() && it.hasNext()) {
QString path = it.next();
if ((it.fileName() == QLatin1String("..")) ||
@@ -127,8 +128,6 @@ bool ReadWriteLibarchivePlugin::addFiles(const QList<Archive::Entry*> &files, co
}
}
- m_abortOperation = false;
-
finish(isSuccessful);
return isSuccessful;
}
@@ -409,7 +408,7 @@ bool ReadWriteLibarchivePlugin::processOldEntries(int &entriesCounter, Operation
}
}
- while ((mode != Add || !m_abortOperation) && archive_read_next_header(m_archiveReader.data(), &entry) == ARCHIVE_OK) {
+ while ((mode != Add || !QThread::currentThread()->isInterruptionRequested()) && archive_read_next_header(m_archiveReader.data(), &entry) == ARCHIVE_OK) {
const QString file = QFile::decodeName(archive_entry_pathname(entry));