diff --git a/src/app/drivemanager.cpp b/src/app/drivemanager.cpp index 0ad1cb34..3c26545e 100644 --- a/src/app/drivemanager.cpp +++ b/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 diff --git a/src/app/drivemanager.h b/src/app/drivemanager.h index 4e1d396c..959dfa17 100644 --- a/src/app/drivemanager.h +++ b/src/app/drivemanager.h @@ -22,6 +22,8 @@ #include #include +#include +#include #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 m_process; }; #endif // DRIVEMANAGER_H diff --git a/src/app/linuxdrivemanager.cpp b/src/app/linuxdrivemanager.cpp index b67c1ecd..51738118 100644 --- a/src/app/linuxdrivemanager.cpp +++ b/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; } diff --git a/src/app/linuxdrivemanager.h b/src/app/linuxdrivemanager.h index 91b51311..d09ad2ef 100644 --- a/src/app/linuxdrivemanager.h +++ b/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 diff --git a/src/app/macdrivemanager.cpp b/src/app/macdrivemanager.cpp index 79cf8b65..b5102e00 100644 --- a/src/app/macdrivemanager.cpp +++ b/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(&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(&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(&QProcess::finished), this, &MacDrive::onRestoreFinished); - connect(qApp, &QCoreApplication::aboutToQuit, m_child, &QProcess::terminate); + connect(m_process.get(), static_cast(&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); } diff --git a/src/app/macdrivemanager.h b/src/app/macdrivemanager.h index 98fc9661..b263b493 100644 --- a/src/app/macdrivemanager.h +++ b/src/app/macdrivemanager.h @@ -51,7 +51,6 @@ class MacDrive : public Drive MacDrive(DriveProvider *parent, const QString &name, uint64_t size, bool containsLive, const QString &bsdDevice); Q_INVOKABLE virtual bool write(ReleaseVariant *data) override; - Q_INVOKABLE virtual void cancel() override; Q_INVOKABLE virtual void restore() override; private slots: void onFinished(int exitCode, QProcess::ExitStatus exitStatus); @@ -60,7 +59,6 @@ private slots: private: QString m_bsdDevice; - QProcess *m_child{nullptr}; }; #endif // MACDRIVEMANAGER_H diff --git a/src/app/windrivemanager.cpp b/src/app/windrivemanager.cpp index 8bda71c4..3b7d65fb 100644 --- a/src/app/windrivemanager.cpp +++ b/src/app/windrivemanager.cpp @@ -99,7 +99,6 @@ void WinDriveProvider::checkDrives() driveIndexes.removeAll(it.key()); } - // Remove our previously stored drives that were not present in the last check for (int index : driveIndexes) { mDebug() << "Removing old drive with index" << index; emit driveRemoved(m_drives[index]); @@ -117,34 +116,24 @@ WinDrive::WinDrive(WinDriveProvider *parent, const QString &name, uint64_t size, { } -WinDrive::~WinDrive() -{ - if (m_child) - m_child->terminate(); -} - bool WinDrive::write(ReleaseVariant *data) { mDebug() << this->metaObject()->className() << "Preparing to write" << data->fullName() << "to drive" << m_device; if (!Drive::write(data)) return false; - if (m_child) { - // TODO some handling of an already present process - m_child->deleteLater(); - } - m_child = new QProcess(this); - connect(m_child, static_cast(&QProcess::finished), this, &WinDrive::onFinished); - connect(m_child, &QProcess::readyRead, this, &WinDrive::onReadyRead); - connect(qApp, &QCoreApplication::aboutToQuit, m_child, &QProcess::terminate); + m_process.reset(new QProcess(this)); + connect(m_process.get(), static_cast(&QProcess::finished), this, &WinDrive::onFinished); + connect(m_process.get(), &QProcess::readyRead, this, &WinDrive::onReadyRead); + connect(qApp, &QCoreApplication::aboutToQuit, m_process.get(), &QProcess::terminate); if (data->status() != ReleaseVariant::DOWNLOADING) m_image->setStatus(ReleaseVariant::WRITING); if (QFile::exists(qApp->applicationDirPath() + "/helper.exe")) { - m_child->setProgram(qApp->applicationDirPath() + "/helper.exe"); + m_process->setProgram(qApp->applicationDirPath() + "/helper.exe"); } else if (QFile::exists(qApp->applicationDirPath() + "/../helper.exe")) { - m_child->setProgram(qApp->applicationDirPath() + "/../helper.exe"); + m_process->setProgram(qApp->applicationDirPath() + "/../helper.exe"); } else { data->setErrorString(tr("Could not find the helper binary. Check your installation.")); setDelayedWrite(false); @@ -158,38 +147,26 @@ bool WinDrive::write(ReleaseVariant *data) else args << data->temporaryPath(); args << QString("%1").arg(m_device); - m_child->setArguments(args); + m_process->setArguments(args); - mDebug() << this->metaObject()->className() << "Starting" << m_child->program() << args; - m_child->start(); + mDebug() << this->metaObject()->className() << "Starting" << m_process->program() << args; + m_process->start(); return true; } -void WinDrive::cancel() -{ - Drive::cancel(); - if (m_child) { - m_child->terminate(); - m_child->deleteLater(); - m_child = nullptr; - } -} - void WinDrive::restore() { mDebug() << this->metaObject()->className() << "Preparing to restore disk" << m_device; - 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.exe")) { - m_child->setProgram(qApp->applicationDirPath() + "/helper.exe"); + m_process->setProgram(qApp->applicationDirPath() + "/helper.exe"); } else if (QFile::exists(qApp->applicationDirPath() + "/../helper.exe")) { - m_child->setProgram(qApp->applicationDirPath() + "/../helper.exe"); + m_process->setProgram(qApp->applicationDirPath() + "/../helper.exe"); } else { m_restoreStatus = RESTORE_ERROR; return; @@ -198,20 +175,19 @@ void WinDrive::restore() QStringList args; args << "restore"; args << QString("%1").arg(m_device); - m_child->setArguments(args); + m_process->setArguments(args); - // connect(m_process, &QProcess::readyRead, this, &LinuxDrive::onReadyRead); - connect(m_child, SIGNAL(finished(int, QProcess::ExitStatus)), this, SLOT(onRestoreFinished(int, QProcess::ExitStatus))); - connect(qApp, &QCoreApplication::aboutToQuit, m_child, &QProcess::terminate); + connect(m_process.get(), SIGNAL(finished(int, QProcess::ExitStatus)), this, SLOT(onRestoreFinished(int, QProcess::ExitStatus))); + connect(qApp, &QCoreApplication::aboutToQuit, m_process.get(), &QProcess::terminate); - mDebug() << this->metaObject()->className() << "Starting" << m_child->program() << args; + mDebug() << this->metaObject()->className() << "Starting" << m_process->program() << args; - m_child->start(QIODevice::ReadOnly); + m_process->start(QIODevice::ReadOnly); } bool WinDrive::busy() const { - return (m_child && m_child->state() == QProcess::Running); + return (m_process && m_process->state() == QProcess::Running); } QString WinDrive::serialNumber() const @@ -228,35 +204,32 @@ void WinDrive::onFinished(int exitCode, QProcess::ExitStatus exitStatus) { setDelayedWrite(false); - if (!m_child) + if (!m_process) return; mDebug() << "Child finished" << exitCode << exitStatus; - mDebug() << m_child->errorString(); + mDebug() << m_process->errorString(); if (exitCode == 0) { m_image->setStatus(ReleaseVariant::FINISHED); Notifications::notify(tr("Finished!"), tr("Writing %1 was successful").arg(m_image->fullName())); } else if (exitCode == 1) { - m_image->setErrorString(m_child->readAllStandardError().trimmed()); + m_image->setErrorString(m_process->readAllStandardError().trimmed()); m_image->setStatus(ReleaseVariant::FAILED); } else if (exitCode == 2) { m_image->setErrorString(tr("Writing has been cancelled")); m_image->setStatus(ReleaseVariant::FAILED); } - - m_child->deleteLater(); - m_child = nullptr; } void WinDrive::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; @@ -264,14 +237,11 @@ void WinDrive::onRestoreFinished(int exitCode, QProcess::ExitStatus exitStatus) m_restoreStatus = RESTORE_ERROR; } emit restoreStatusChanged(); - - m_child->deleteLater(); - m_child = nullptr; } void WinDrive::onReadyRead() { - if (!m_child) { + if (!m_process) { return; } @@ -282,8 +252,8 @@ void WinDrive::onReadyRead() m_image->setStatus(ReleaseVariant::WRITING); } - while (m_child->bytesAvailable() > 0) { - QString line = m_child->readLine().trimmed(); + while (m_process->bytesAvailable() > 0) { + QString line = m_process->readLine().trimmed(); if (line == "CHECK") { mDebug() << this->metaObject()->className() << "Written media check starting"; m_progress->setValue(0); diff --git a/src/app/windrivemanager.h b/src/app/windrivemanager.h index 54e9d094..46c2aa30 100644 --- a/src/app/windrivemanager.h +++ b/src/app/windrivemanager.h @@ -47,10 +47,8 @@ class WinDrive : public Drive Q_OBJECT public: WinDrive(WinDriveProvider *parent, const QString &name, uint64_t size, bool containsLive, int device, const QString &serialNumber); - ~WinDrive(); Q_INVOKABLE virtual bool write(ReleaseVariant *data) override; - Q_INVOKABLE virtual void cancel() override; Q_INVOKABLE virtual void restore() override; bool busy() const; @@ -66,7 +64,6 @@ private slots: private: int m_device; QString m_serialNo; - QProcess *m_child{nullptr}; }; #endif // WINDRIVEMANAGER_H