Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Prachi194agrawal wants to merge 8 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6d76a24
Fix: Prevent UI reset when new device inserted during write (#825)
Prachi194agrawal Dec 16, 2025
f8cfe68
Refactor: Use state-based isBusy() instead of progress checking (#825)
Prachi194agrawal Jan 13, 2026
6d63fbc
Refactor: Move QProcess to base Drive class and remove m_isBusy flag
Prachi194agrawal Jan 14, 2026
9f5aa75
Refactor: Use std::unique_ptr for QProcess management
Prachi194agrawal Jan 15, 2026
3169c70
Address code review feedback: clean up RAII implementation
Prachi194agrawal Jan 15, 2026
f1972ee
Refactor QProcess management to use std::make_unique and remove platf…
Prachi194agrawal Jan 16, 2026
56c55a7
Fix: Revert to .reset() because make_unique is incompatible with cust…
Prachi194agrawal Jan 16, 2026
de28ba0
Fix: Apply clang-format to DriveOperationDeleter
Prachi194agrawal Jan 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/app/drivemanager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,12 @@ void DriveManager::onDriveConnected(Drive *d)
endInsertRows();
emit drivesChanged();

// Fix for issue #825: Only change selection if the currently selected drive is not busy
if (selected() && selected()->isBusy()) {
return;
}

// Standard logic: Select the new drive if not busy
Copy link
Collaborator

@grulja grulja Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this comment can be removed

if (m_provider->initialized()) {
m_selectedIndex = position;
emit selectedChanged();
Expand Down Expand Up @@ -289,6 +295,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) {
Expand Down
3 changes: 3 additions & 0 deletions src/app/drivemanager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#include <QAbstractListModel>
#include <QDebug>
#include <QProcess>

#include "releasemanager.h"

Expand Down Expand Up @@ -169,6 +170,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);

Expand All @@ -194,6 +196,7 @@ public slots:
RestoreStatus m_restoreStatus{CLEAN};
QString m_error{};
bool m_delayedWrite{false};
QProcess *m_process{nullptr};
Copy link
Collaborator

@grulja grulja Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do here something like:

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;
    }
};

std::unique_ptr<QProcess, DriveOperationDeleter> m_process;

Then you can remove the m_process cleaning that is currently done separately in each [Linux,Mac,Windows]DriveManager destructor. Also you can simplify the process removal in each cancel() method and make the DriveManager::cancel() to simply do m_process.reset() that does exactly what we want thanks to the DriveOperationDeleter. With the use of std::unique_ptr you don't have to do:

if (m_process) {
    m_process->deleteLater();
}
m_process = new QProcess(this);

but simply:

m_process = std::make_unique<QProcess>(this);

};

#endif // DRIVEMANAGER_H
2 changes: 0 additions & 2 deletions src/app/linuxdrivemanager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,6 @@ private slots:

private:
QString m_device;

QProcess *m_process{nullptr};
};

#endif // LINUXDRIVEMANAGER_H
70 changes: 35 additions & 35 deletions src/app/macdrivemanager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,19 +87,19 @@ 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) {
if (m_process) {
// TODO some handling of an already present process
m_child->deleteLater();
m_process->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 = new QProcess(this);
connect(m_process, static_cast<void (QProcess::*)(int, QProcess::ExitStatus)>(&QProcess::finished), this, &MacDrive::onFinished);
connect(m_process, &QProcess::readyRead, this, &MacDrive::onReadyRead);
connect(qApp, &QCoreApplication::aboutToQuit, m_process, &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);
Expand All @@ -114,39 +114,39 @@ 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()
Copy link
Collaborator

@grulja grulja Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove MacDrive::cancel() completely. If we don't implement it ourself the base DriveManager::cancel() will be called.

{
Drive::cancel();
if (m_child) {
m_child->kill();
m_child->deleteLater();
m_child = nullptr;
if (m_process) {
m_process->kill();
m_process->deleteLater();
m_process = nullptr;
}
}

void MacDrive::restore()
{
mCritical() << "starting to restore";
if (m_child)
m_child->deleteLater();
if (m_process)
m_process->deleteLater();

m_child = new QProcess(this);
m_process = 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;
Expand All @@ -155,16 +155,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, static_cast<void (QProcess::*)(int, QProcess::ExitStatus)>(&QProcess::finished), this, &MacDrive::onRestoreFinished);
connect(qApp, &QCoreApplication::aboutToQuit, m_process, &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)
Expand All @@ -173,11 +173,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) {
Expand All @@ -194,25 +194,25 @@ 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;
m_process->deleteLater();
m_process = nullptr;
}

void MacDrive::onReadyRead()
{
if (!m_child)
if (!m_process)
return;

if (m_image->status() == ReleaseVariant::WRITING) {
Expand All @@ -223,9 +223,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);
}
Expand Down
1 change: 0 additions & 1 deletion src/app/macdrivemanager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ private slots:

private:
QString m_bsdDevice;
QProcess *m_child{nullptr};
};

#endif // MACDRIVEMANAGER_H
Loading