Commit b1a251eb authored by Elvis Angelaccio's avatar Elvis Angelaccio

Rework kill logic

The libzip plugin sometimes cannot react to `QThread::requestInterruption()`
because there is a blocking `zip_close()` which writes to disk (e.g. with AddJobs).

This means we have to manually abort the thread (by passing a timeout to
`QThread::wait()` in `Job::doKill()`. We cannot do this uconditionally
because we would end up with crashes in libarchive. Since the libarchive
plugin is not affected by this problem, we rework the logic in
`Job::doKill()` by assuming that the interface will tell us whether it
needs to be brutally killed. This way we can distinguish between the
libarchive and the libzip plugins (the ones that use a worker thread).

The mutex in this patch is needed because in theory `m_operationMode` could be
read and written by different threads at the same time, even though that's unlikely.

BUG: 389290
FIXED-IN: 18.03.80
Task: T7824
parent 0fba00d0
...@@ -151,6 +151,10 @@ public: ...@@ -151,6 +151,10 @@ public:
*/ */
static QStringList entryPathsFromDestination(QStringList entries, const Archive::Entry *destination, int entriesWithoutChildren); static QStringList entryPathsFromDestination(QStringList entries, const Archive::Entry *destination, int entriesWithoutChildren);
/**
* @return true if the interface has killed the job or if it will stop it as soon as possible.
* Otherwise returns false if the interface is not able to kill the operation.
*/
virtual bool doKill(); virtual bool doKill();
bool isHeaderEncryptionEnabled() const; bool isHeaderEncryptionEnabled() const;
......
...@@ -223,16 +223,22 @@ void Job::onUserQuery(Query *query) ...@@ -223,16 +223,22 @@ void Job::onUserQuery(Query *query)
bool Job::doKill() bool Job::doKill()
{ {
if (d->isRunning()) { const bool canKillJob = archiveInterface()->doKill();
d->requestInterruption();
d->wait(); if (!d->isRunning()) {
return canKillJob;
} }
bool ret = archiveInterface()->doKill(); d->requestInterruption();
if (!ret) {
qCWarning(ARK) << "Killing does not seem to be supported here."; if (!canKillJob) {
qCDebug(ARK) << "Forcing thread exit in one second...";
d->wait(1000);
return true;
} }
return ret;
d->wait();
return canKillJob;
} }
LoadJob::LoadJob(Archive *archive, ReadOnlyArchiveInterface *interface) LoadJob::LoadJob(Archive *archive, ReadOnlyArchiveInterface *interface)
......
...@@ -78,7 +78,7 @@ LibzipPlugin::~LibzipPlugin() ...@@ -78,7 +78,7 @@ LibzipPlugin::~LibzipPlugin()
bool LibzipPlugin::list() bool LibzipPlugin::list()
{ {
qCDebug(ARK) << "Listing archive contents for:" << QFile::encodeName(filename()); qCDebug(ARK) << "Listing archive contents for:" << QFile::encodeName(filename());
setOperationMode(List);
m_numberOfEntries = 0; m_numberOfEntries = 0;
int errcode = 0; int errcode = 0;
...@@ -124,7 +124,7 @@ bool LibzipPlugin::list() ...@@ -124,7 +124,7 @@ bool LibzipPlugin::list()
bool LibzipPlugin::addFiles(const QVector<Archive::Entry*> &files, const Archive::Entry *destination, const CompressionOptions& options, uint numberOfEntriesToAdd) bool LibzipPlugin::addFiles(const QVector<Archive::Entry*> &files, const Archive::Entry *destination, const CompressionOptions& options, uint numberOfEntriesToAdd)
{ {
Q_UNUSED(numberOfEntriesToAdd) Q_UNUSED(numberOfEntriesToAdd)
setOperationMode(Add);
int errcode = 0; int errcode = 0;
zip_error_t err; zip_error_t err;
...@@ -391,6 +391,7 @@ bool LibzipPlugin::emitEntryForIndex(zip_t *archive, qlonglong index) ...@@ -391,6 +391,7 @@ bool LibzipPlugin::emitEntryForIndex(zip_t *archive, qlonglong index)
bool LibzipPlugin::deleteFiles(const QVector<Archive::Entry*> &files) bool LibzipPlugin::deleteFiles(const QVector<Archive::Entry*> &files)
{ {
setOperationMode(Delete);
int errcode = 0; int errcode = 0;
zip_error_t err; zip_error_t err;
...@@ -436,6 +437,7 @@ bool LibzipPlugin::deleteFiles(const QVector<Archive::Entry*> &files) ...@@ -436,6 +437,7 @@ bool LibzipPlugin::deleteFiles(const QVector<Archive::Entry*> &files)
bool LibzipPlugin::addComment(const QString& comment) bool LibzipPlugin::addComment(const QString& comment)
{ {
setOperationMode(Comment);
int errcode = 0; int errcode = 0;
zip_error_t err; zip_error_t err;
...@@ -465,7 +467,7 @@ bool LibzipPlugin::addComment(const QString& comment) ...@@ -465,7 +467,7 @@ bool LibzipPlugin::addComment(const QString& comment)
bool LibzipPlugin::testArchive() bool LibzipPlugin::testArchive()
{ {
qCDebug(ARK) << "Testing archive"; qCDebug(ARK) << "Testing archive";
setOperationMode(Test);
int errcode = 0; int errcode = 0;
zip_error_t err; zip_error_t err;
...@@ -515,14 +517,24 @@ bool LibzipPlugin::testArchive() ...@@ -515,14 +517,24 @@ bool LibzipPlugin::testArchive()
bool LibzipPlugin::doKill() bool LibzipPlugin::doKill()
{ {
qCDebug(ARK) << "Killing operation..."; QMutexLocker mutexLocker(&m_mutex);
switch (m_operationMode) {
case Add:
case Copy:
case Delete:
case Move:
return false;
default:
break;
}
return true; return true;
} }
bool LibzipPlugin::extractFiles(const QVector<Archive::Entry*> &files, const QString& destinationDirectory, const ExtractionOptions& options) bool LibzipPlugin::extractFiles(const QVector<Archive::Entry*> &files, const QString& destinationDirectory, const ExtractionOptions& options)
{ {
qCDebug(ARK) << "Extracting files to:" << destinationDirectory; qCDebug(ARK) << "Extracting files to:" << destinationDirectory;
setOperationMode(Extract);
const bool extractAll = files.isEmpty(); const bool extractAll = files.isEmpty();
const bool removeRootNode = options.isDragAndDropEnabled(); const bool removeRootNode = options.isDragAndDropEnabled();
...@@ -790,7 +802,7 @@ bool LibzipPlugin::extractEntry(zip_t *archive, const QString &entry, const QStr ...@@ -790,7 +802,7 @@ bool LibzipPlugin::extractEntry(zip_t *archive, const QString &entry, const QStr
bool LibzipPlugin::moveFiles(const QVector<Archive::Entry*> &files, Archive::Entry *destination, const CompressionOptions &options) bool LibzipPlugin::moveFiles(const QVector<Archive::Entry*> &files, Archive::Entry *destination, const CompressionOptions &options)
{ {
Q_UNUSED(options) Q_UNUSED(options)
setOperationMode(Move);
int errcode = 0; int errcode = 0;
zip_error_t err; zip_error_t err;
...@@ -840,7 +852,7 @@ bool LibzipPlugin::moveFiles(const QVector<Archive::Entry*> &files, Archive::Ent ...@@ -840,7 +852,7 @@ bool LibzipPlugin::moveFiles(const QVector<Archive::Entry*> &files, Archive::Ent
bool LibzipPlugin::copyFiles(const QVector<Archive::Entry*> &files, Archive::Entry *destination, const CompressionOptions &options) bool LibzipPlugin::copyFiles(const QVector<Archive::Entry*> &files, Archive::Entry *destination, const CompressionOptions &options)
{ {
Q_UNUSED(options) Q_UNUSED(options)
setOperationMode(Copy);
int errcode = 0; int errcode = 0;
zip_error_t err; zip_error_t err;
...@@ -973,4 +985,10 @@ QString LibzipPlugin::permissionsToString(const mode_t &perm) ...@@ -973,4 +985,10 @@ QString LibzipPlugin::permissionsToString(const mode_t &perm)
return modeval; return modeval;
} }
void LibzipPlugin::setOperationMode(ReadWriteArchiveInterface::OperationMode operationMode)
{
QMutexLocker mutexLocker(&m_mutex);
m_operationMode = operationMode;
}
#include "libzipplugin.moc" #include "libzipplugin.moc"
...@@ -28,6 +28,8 @@ ...@@ -28,6 +28,8 @@
#include "archiveinterface.h" #include "archiveinterface.h"
#include <QMutex>
#include <zip.h> #include <zip.h>
using namespace Kerfuffle; using namespace Kerfuffle;
...@@ -57,11 +59,14 @@ private: ...@@ -57,11 +59,14 @@ private:
bool emitEntryForIndex(zip_t *archive, qlonglong index); bool emitEntryForIndex(zip_t *archive, qlonglong index);
void progressEmitted(double pct); void progressEmitted(double pct);
QString permissionsToString(const mode_t &perm); QString permissionsToString(const mode_t &perm);
void setOperationMode(OperationMode operationMode);
QVector<Archive::Entry*> m_emittedEntries; QVector<Archive::Entry*> m_emittedEntries;
bool m_overwriteAll; bool m_overwriteAll;
bool m_skipAll; bool m_skipAll;
bool m_listAfterAdd; bool m_listAfterAdd;
QMutex m_mutex;
OperationMode m_operationMode = List;
}; };
#endif // LIBZIPPLUGIN_H #endif // LIBZIPPLUGIN_H
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