Fix: Prevent UI reset when new device inserted during write (#825)

src/app/drivemanager.cpp

@@ -157,6 +157,10 @@ void DriveManager::onDriveConnected(Drive *d)
     endInsertRows();
     emit drivesChanged();
 
+    if (selected() && selected()->isBusy()) {
+        return;
+    }
+
     if (m_provider->initialized()) {
         m_selectedIndex = position;
         emit selectedChanged();
@@ -289,6 +293,11 @@ bool Drive::delayedWrite() const
     return m_delayedWrite;
 }
 
+bool Drive::isBusy() const
+{
+    return m_process && m_process->state() != QProcess::NotRunning;
+}
+
 void Drive::setDelayedWrite(const bool &o)
 {
     if (m_delayedWrite != o) {
@@ -329,6 +338,7 @@ void Drive::cancel()
     m_error = QString();
     m_restoreStatus = CLEAN;
     emit restoreStatusChanged();
+    m_process.reset();
 }
 
 bool Drive::operator==(const Drive &o) const

src/app/drivemanager.h

@@ -22,6 +22,8 @@
 
 #include <QAbstractListModel>
 #include <QDebug>
+#include <QProcess>
+#include <memory>
 
 #include "releasemanager.h"
 
@@ -30,6 +32,35 @@ class DriveProvider;
 class Drive;
 class UdisksDrive;
 
+/**
+ * @brief Custom deleter for QProcess to ensure proper cleanup
+ *
+ * Handles platform-specific process termination before deletion
+ */
+struct DriveOperationDeleter {
+    void operator()(QProcess *process)
+    {
+        if (!process) {
+            return;
+        }
+
+        if (process->state() != QProcess::NotRunning) {
+#ifdef Q_OS_WIN
+            process->kill();
+            process->waitForFinished();
+#else
+            process->terminate();
+            if (!process->waitForFinished(3000)) {
+                process->kill();
+                process->waitForFinished();
+            }
+#endif
+        }
+
+        delete process;
+    }
+};
+
 /**
  * @brief The DriveManager class
  *
@@ -169,6 +200,7 @@ class Drive : public QObject
     virtual qreal size() const;
     virtual RestoreStatus restoreStatus();
     virtual bool delayedWrite() const;
+    virtual bool isBusy() const;
 
     virtual void setDelayedWrite(const bool &o);
 
@@ -194,6 +226,7 @@ public slots:
     RestoreStatus m_restoreStatus{CLEAN};
     QString m_error{};
     bool m_delayedWrite{false};
+    std::unique_ptr<QProcess, DriveOperationDeleter> m_process;
 };
 
 #endif // DRIVEMANAGER_H

src/app/linuxdrivemanager.cpp

@@ -193,14 +193,6 @@ LinuxDrive::LinuxDrive(LinuxDriveProvider *parent, QString device, QString name,
 {
 }
 
-LinuxDrive::~LinuxDrive()
-{
-    if (m_image && m_image->status() == ReleaseVariant::WRITING) {
-        m_image->setErrorString(tr("The drive was removed while it was written to."));
-        m_image->setStatus(ReleaseVariant::FAILED);
-    }
-}
-
 bool LinuxDrive::write(ReleaseVariant *data)
 {
     mDebug() << this->metaObject()->className() << "Will now write" << data->iso() << "to" << this->m_device;
@@ -210,8 +202,7 @@ bool LinuxDrive::write(ReleaseVariant *data)
     if (m_image->status() == ReleaseVariant::READY || m_image->status() == ReleaseVariant::FAILED || m_image->status() == ReleaseVariant::FAILED_VERIFICATION || m_image->status() == ReleaseVariant::FINISHED)
         m_image->setStatus(ReleaseVariant::WRITING);
 
-    if (!m_process)
-        m_process = new QProcess(this);
+    m_process.reset(new QProcess(this));
 
     QStringList args;
     if (QFile::exists(qApp->applicationDirPath() + "/../helper/linux/helper")) {
@@ -235,44 +226,21 @@ bool LinuxDrive::write(ReleaseVariant *data)
     mDebug() << this->metaObject()->className() << "Helper command will be" << args;
     m_process->setArguments(args);
 
-    connect(m_process, &QProcess::readyRead, this, &LinuxDrive::onReadyRead);
-    connect(m_process, SIGNAL(finished(int, QProcess::ExitStatus)), this, SLOT(onFinished(int, QProcess::ExitStatus)));
-    // TODO check if this is actually necessary - it should work just fine even without it
-    connect(m_process, &QProcess::errorOccurred, this, &LinuxDrive::onErrorOccurred);
-    connect(qApp, &QCoreApplication::aboutToQuit, m_process, &QProcess::terminate);
+    connect(m_process.get(), &QProcess::readyRead, this, &LinuxDrive::onReadyRead);
+    connect(m_process.get(), SIGNAL(finished(int, QProcess::ExitStatus)), this, SLOT(onFinished(int, QProcess::ExitStatus)));
+    connect(m_process.get(), &QProcess::errorOccurred, this, &LinuxDrive::onErrorOccurred);
+    connect(qApp, &QCoreApplication::aboutToQuit, m_process.get(), &QProcess::terminate);
 
     m_process->start(QIODevice::ReadOnly);
 
     return true;
 }
 
-void LinuxDrive::cancel()
-{
-    Drive::cancel();
-    static bool beingCancelled = false;
-    if (m_process != nullptr && !beingCancelled) {
-        beingCancelled = true;
-        if (m_image) {
-            if (m_image->status() == ReleaseVariant::WRITE_VERIFYING) {
-                m_image->setStatus(ReleaseVariant::FINISHED);
-            } else if (m_image->status() != ReleaseVariant::DOWNLOADING && m_image->status() != ReleaseVariant::DOWNLOAD_VERIFYING) {
-                m_image->setErrorString(tr("Stopped before writing has finished."));
-                m_image->setStatus(ReleaseVariant::FAILED);
-            }
-        }
-        m_process->kill();
-        m_process->deleteLater();
-        m_process = nullptr;
-        beingCancelled = false;
-    }
-}
-
 void LinuxDrive::restore()
 {
     mDebug() << this->metaObject()->className() << "Will now restore" << this->m_device;
 
-    if (!m_process)
-        m_process = new QProcess(this);
+    m_process.reset(new QProcess(this));
 
     m_restoreStatus = RESTORING;
     emit restoreStatusChanged();
@@ -294,9 +262,9 @@ void LinuxDrive::restore()
     mDebug() << this->metaObject()->className() << "Helper command will be" << args;
     m_process->setArguments(args);
 
-    connect(m_process, &QProcess::readyRead, this, &LinuxDrive::onReadyRead);
-    connect(m_process, SIGNAL(finished(int, QProcess::ExitStatus)), this, SLOT(onRestoreFinished(int, QProcess::ExitStatus)));
-    connect(qApp, &QCoreApplication::aboutToQuit, m_process, &QProcess::terminate);
+    connect(m_process.get(), &QProcess::readyRead, this, &LinuxDrive::onReadyRead);
+    connect(m_process.get(), SIGNAL(finished(int, QProcess::ExitStatus)), this, SLOT(onRestoreFinished(int, QProcess::ExitStatus)));
+    connect(qApp, &QCoreApplication::aboutToQuit, m_process.get(), &QProcess::terminate);
 
     m_process->start(QIODevice::ReadOnly);
 }
@@ -353,11 +321,8 @@ void LinuxDrive::onFinished(int exitCode, QProcess::ExitStatus status)
         }
     }
 
-    if (m_process) {
-        m_process->deleteLater();
-        m_process = nullptr;
-        m_image = nullptr;
-    }
+    m_process.reset();
+    m_image = nullptr;
 }
 
 void LinuxDrive::onRestoreFinished(int exitCode, QProcess::ExitStatus status)
@@ -373,10 +338,7 @@ void LinuxDrive::onRestoreFinished(int exitCode, QProcess::ExitStatus status)
     } else {
         m_restoreStatus = RESTORED;
     }
-    if (m_process) {
-        m_process->deleteLater();
-        m_process = nullptr;
-    }
+    m_process.reset();
     emit restoreStatusChanged();
 }
 
@@ -389,8 +351,7 @@ void LinuxDrive::onErrorOccurred(QProcess::ProcessError e)
     QString errorMessage = m_process->errorString();
     mWarning() << "Restoring failed:" << errorMessage;
     m_image->setErrorString(errorMessage);
-    m_process->deleteLater();
-    m_process = nullptr;
+    m_process.reset();
     m_image->setStatus(ReleaseVariant::FAILED);
     m_image = nullptr;
 }

src/app/linuxdrivemanager.h

@@ -62,10 +62,8 @@ class LinuxDrive : public Drive
     Q_OBJECT
 public:
     LinuxDrive(LinuxDriveProvider *parent, QString device, QString name, uint64_t size, bool isoLayout);
-    ~LinuxDrive();
 
     Q_INVOKABLE virtual bool write(ReleaseVariant *data) override;
-    Q_INVOKABLE virtual void cancel() override;
     Q_INVOKABLE virtual void restore() override;
 
 private slots:
@@ -76,8 +74,6 @@ private slots:
 
 private:
     QString m_device;
-
-    QProcess *m_process{nullptr};
 };
 
 #endif // LINUXDRIVEMANAGER_H

src/app/macdrivemanager.cpp

@@ -87,19 +87,15 @@ bool MacDrive::write(ReleaseVariant *data)
     if (m_image->status() == ReleaseVariant::READY || m_image->status() == ReleaseVariant::FAILED || m_image->status() == ReleaseVariant::FAILED_VERIFICATION || m_image->status() == ReleaseVariant::FINISHED)
         m_image->setStatus(ReleaseVariant::WRITING);
 
-    if (m_child) {
-        // TODO some handling of an already present process
-        m_child->deleteLater();
-    }
-    m_child = new QProcess(this);
-    connect(m_child, static_cast<void (QProcess::*)(int, QProcess::ExitStatus)>(&QProcess::finished), this, &MacDrive::onFinished);
-    connect(m_child, &QProcess::readyRead, this, &MacDrive::onReadyRead);
-    connect(qApp, &QCoreApplication::aboutToQuit, m_child, &QProcess::terminate);
+    m_process.reset(new QProcess(this));
+    connect(m_process.get(), static_cast<void (QProcess::*)(int, QProcess::ExitStatus)>(&QProcess::finished), this, &MacDrive::onFinished);
+    connect(m_process.get(), &QProcess::readyRead, this, &MacDrive::onReadyRead);
+    connect(qApp, &QCoreApplication::aboutToQuit, m_process.get(), &QProcess::terminate);
 
     if (QFile::exists(qApp->applicationDirPath() + "/../../../../helper/mac/helper.app/Contents/MacOS/helper")) {
-        m_child->setProgram(QString("%1/../../../../helper/mac/helper.app/Contents/MacOS/helper").arg(qApp->applicationDirPath()));
+        m_process->setProgram(QString("%1/../../../../helper/mac/helper.app/Contents/MacOS/helper").arg(qApp->applicationDirPath()));
     } else if (QFile::exists(qApp->applicationDirPath() + "/helper")) {
-        m_child->setProgram(QString("%1/helper").arg(qApp->applicationDirPath()));
+        m_process->setProgram(QString("%1/helper").arg(qApp->applicationDirPath()));
     } else {
         data->setErrorString(tr("Could not find the helper binary. Check your installation."));
         setDelayedWrite(false);
@@ -114,39 +110,27 @@ bool MacDrive::write(ReleaseVariant *data)
     }
     args.append(m_bsdDevice);
 
-    mCritical() << "The command is" << m_child->program() << args;
-    m_child->setArguments(args);
+    mCritical() << "The command is" << m_process->program() << args;
+    m_process->setArguments(args);
 
-    m_child->start();
+    m_process->start();
 
     return true;
 }
 
-void MacDrive::cancel()
-{
-    Drive::cancel();
-    if (m_child) {
-        m_child->kill();
-        m_child->deleteLater();
-        m_child = nullptr;
-    }
-}
-
 void MacDrive::restore()
 {
     mCritical() << "starting to restore";
-    if (m_child)
-        m_child->deleteLater();
 
-    m_child = new QProcess(this);
+    m_process.reset(new QProcess(this));
 
     m_restoreStatus = RESTORING;
     emit restoreStatusChanged();
 
     if (QFile::exists(qApp->applicationDirPath() + "/../../../../helper/mac/helper.app/Contents/MacOS/helper")) {
-        m_child->setProgram(QString("%1/../../../../helper/mac/helper.app/Contents/MacOS/helper").arg(qApp->applicationDirPath()));
+        m_process->setProgram(QString("%1/../../../../helper/mac/helper.app/Contents/MacOS/helper").arg(qApp->applicationDirPath()));
     } else if (QFile::exists(qApp->applicationDirPath() + "/helper")) {
-        m_child->setProgram(QString("%1/helper").arg(qApp->applicationDirPath()));
+        m_process->setProgram(QString("%1/helper").arg(qApp->applicationDirPath()));
     } else {
         m_restoreStatus = RESTORE_ERROR;
         return;
@@ -155,16 +139,16 @@ void MacDrive::restore()
     QStringList args;
     args.append("restore");
     args.append(m_bsdDevice);
-    m_child->setArguments(args);
+    m_process->setArguments(args);
 
-    m_child->setProcessChannelMode(QProcess::ForwardedChannels);
+    m_process->setProcessChannelMode(QProcess::ForwardedChannels);
 
-    connect(m_child, static_cast<void (QProcess::*)(int, QProcess::ExitStatus)>(&QProcess::finished), this, &MacDrive::onRestoreFinished);
-    connect(qApp, &QCoreApplication::aboutToQuit, m_child, &QProcess::terminate);
+    connect(m_process.get(), static_cast<void (QProcess::*)(int, QProcess::ExitStatus)>(&QProcess::finished), this, &MacDrive::onRestoreFinished);
+    connect(qApp, &QCoreApplication::aboutToQuit, m_process.get(), &QProcess::terminate);
 
-    mCritical() << "The command is" << m_child->program() << args;
+    mCritical() << "The command is" << m_process->program() << args;
 
-    m_child->start();
+    m_process->start();
 }
 
 void MacDrive::onFinished(int exitCode, QProcess::ExitStatus exitStatus)
@@ -173,11 +157,11 @@ void MacDrive::onFinished(int exitCode, QProcess::ExitStatus exitStatus)
 
     setDelayedWrite(false);
 
-    if (!m_child)
+    if (!m_process)
         return;
 
     if (exitCode != 0) {
-        QString output = m_child->readAllStandardError();
+        QString output = m_process->readAllStandardError();
         QRegularExpression re("^.+:.+: ");
         QStringList lines = output.split('\n');
         if (lines.length() > 0) {
@@ -194,25 +178,22 @@ void MacDrive::onFinished(int exitCode, QProcess::ExitStatus exitStatus)
 
 void MacDrive::onRestoreFinished(int exitCode, QProcess::ExitStatus exitStatus)
 {
-    if (!m_child)
+    if (!m_process)
         return;
 
     mCritical() << "Process finished" << exitCode << exitStatus;
-    mCritical() << m_child->readAllStandardError();
+    mCritical() << m_process->readAllStandardError();
 
     if (exitCode == 0)
         m_restoreStatus = RESTORED;
     else
         m_restoreStatus = RESTORE_ERROR;
     emit restoreStatusChanged();
-
-    m_child->deleteLater();
-    m_child = nullptr;
 }
 
 void MacDrive::onReadyRead()
 {
-    if (!m_child)
+    if (!m_process)
         return;
 
     if (m_image->status() == ReleaseVariant::WRITING) {
@@ -223,9 +204,9 @@ void MacDrive::onReadyRead()
     if (m_image->status() != ReleaseVariant::WRITE_VERIFYING && m_image->status() != ReleaseVariant::WRITING)
         m_image->setStatus(ReleaseVariant::WRITING);
 
-    while (m_child->bytesAvailable() > 0) {
+    while (m_process->bytesAvailable() > 0) {
         bool ok;
-        int64_t bytes = m_child->readLine().trimmed().toULongLong(&ok);
+        int64_t bytes = m_process->readLine().trimmed().toULongLong(&ok);
         if (ok)
             m_progress->setValue(bytes);
     }