Commit 8ad610bf authored by Elvis Angelaccio's avatar Elvis Angelaccio
Browse files

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
parent 5927ecbd
......@@ -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.";
......
......@@ -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;
......
......@@ -87,7 +87,6 @@ protected:
ArchiveRead m_archiveReader;
ArchiveRead m_archiveReadDisk;
bool m_abortOperation;
private:
int extractionFlags() const;
......
......@@ -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));
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment