diff --git a/apps/arstack_studio/src/DeterministicSessionHarness.cpp b/apps/arstack_studio/src/DeterministicSessionHarness.cpp index d9d956417..f039e96b1 100644 --- a/apps/arstack_studio/src/DeterministicSessionHarness.cpp +++ b/apps/arstack_studio/src/DeterministicSessionHarness.cpp @@ -4,13 +4,19 @@ #include "DeviceIoWorker.hpp" #include "FirmwareManager.hpp" +#include "FirmwareWorker.hpp" #include "SclProfileModel.hpp" #include "SmartSessionController.hpp" #include "StudioDeviceController.hpp" #include #include +#include +#include +#include #include +#include +#include #ifndef ARSTACK_STUDIO_VERSION #define ARSTACK_STUDIO_VERSION "0.1.0" @@ -20,6 +26,14 @@ class DeterministicSessionHarness final { public: static int run(int argc, char* argv[]) { QCoreApplication app(argc, argv); + // A real OS child ensures timeout/cancel tests exercise QProcess + // termination rather than a synthetic boolean-only worker model. + if (app.arguments().contains(QStringLiteral("--firmware-worker-hang-child"))) { + QThread::msleep(8000); + return 0; + } + if (app.arguments().contains(QStringLiteral("--firmware-worker-fast-child"))) + return 0; struct CaseResult { const char* name; @@ -29,6 +43,7 @@ class DeterministicSessionHarness final { const CaseResult cases[] = { {"dual-revision catalog selects only qualified silicon", revisionCatalogFailClosed()}, {"firmware timeout retains port until flasher exits", firmwarePortReleaseWaitsForWorker()}, + {"real timed-out firmware subprocess retires before COM release", firmwareWorkerTimeoutAwaitsProcessExit()}, {"current identity -> READY without recovery", currentIdentityNeverRecovers()}, {"legacy identity -> firmware update", legacyIdentityOffersUpdate()}, {"same version missing build ID -> READY with update suggestion", sameVersionMissingBuildOffersUpdate()}, @@ -130,6 +145,75 @@ class DeterministicSessionHarness final { } private: + static bool firmwareWorkerTimeoutAwaitsProcessExit() { + FirmwareWorker worker; + worker.initialize(); + if (worker.process_ == nullptr) return false; + + QEventLoop loop; + QTimer deadline; + deadline.setSingleShot(true); + QObject::connect(&deadline, &QTimer::timeout, &loop, &QEventLoop::quit); + + bool started = false; + bool timedOutAfterExit = false; + bool restartedCleanly = false; + bool unexpectedTerminal = false; + int terminalCount = 0; + QObject::connect(&worker, &FirmwareWorker::operationStarted, &loop, + [&](const quint64 generation) { + if (generation == 41) started = true; + }); + QObject::connect(&worker, &FirmwareWorker::operationTimedOut, &loop, + [&](const quint64 generation) { + ++terminalCount; + timedOutAfterExit = generation == 41 && started && + worker.process_->state() == QProcess::NotRunning && + !worker.operationActive_ && worker.generation_ == 0; + loop.quit(); + }); + QObject::connect(&worker, &FirmwareWorker::operationFinished, &loop, + [&](const quint64 generation, const int code, const bool normal) { + ++terminalCount; + if (generation == 42) { + restartedCleanly = code == 0 && normal && + worker.process_->state() == QProcess::NotRunning && + !worker.operationActive_; + } else { + unexpectedTerminal = true; + } + loop.quit(); + }); + QObject::connect(&worker, &FirmwareWorker::operationLaunchFailed, &loop, + [&](quint64, const QString&) { unexpectedTerminal = true; loop.quit(); }); + QObject::connect(&worker, &FirmwareWorker::operationLaunchTimedOut, &loop, + [&](quint64) { unexpectedTerminal = true; loop.quit(); }); + QObject::connect(&worker, &FirmwareWorker::operationCancelled, &loop, + [&](quint64) { unexpectedTerminal = true; loop.quit(); }); + + const QString executable = QCoreApplication::applicationFilePath(); + const QProcessEnvironment env = QProcessEnvironment::systemEnvironment(); + worker.startOperation(41, executable, {QStringLiteral("--firmware-worker-hang-child")}, + env, 3500, 150); + deadline.start(5500); + loop.exec(); + deadline.stop(); + if (!timedOutAfterExit || unexpectedTerminal || terminalCount != 1) { + worker.shutdown(); + return false; + } + + // The worker must accept a second operation only after the first + // process has exited and its generation has been fully retired. + worker.startOperation(42, executable, {QStringLiteral("--firmware-worker-fast-child")}, + env, 3500, 3500); + deadline.start(5500); + loop.exec(); + deadline.stop(); + worker.shutdown(); + return restartedCleanly && !unexpectedTerminal && terminalCount == 2; + } + static bool revisionCatalogFailClosed() { FirmwareManager firmware; firmware.revisionCatalogAvailable_ = true; diff --git a/apps/arstack_studio/src/FirmwareWorker.cpp b/apps/arstack_studio/src/FirmwareWorker.cpp index c440110fd..e81581dfb 100644 --- a/apps/arstack_studio/src/FirmwareWorker.cpp +++ b/apps/arstack_studio/src/FirmwareWorker.cpp @@ -27,28 +27,27 @@ void FirmwareWorker::initialize() { connect(launchTimer_, &QTimer::timeout, this, [this] { if (!operationActive_ || process_ == nullptr || process_->state() != QProcess::Starting) return; - const quint64 generation = generation_; - operationActive_ = false; - cancelRequested_ = false; + // QProcess::kill() is asynchronous. Retain the operation/COM lease + // until QProcess has confirmed NotRunning via finished/errorOccurred. + killCause_ = KillCause::launchTimeout; if (operationTimer_ != nullptr) operationTimer_->stop(); process_->kill(); - emit operationLaunchTimedOut(generation); - clearOperation(); + if (operationActive_ && process_->state() == QProcess::NotRunning) + finishProcess(-1, false); }); connect(operationTimer_, &QTimer::timeout, this, [this] { if (!operationActive_ || process_ == nullptr) return; - const quint64 generation = generation_; - operationActive_ = false; - cancelRequested_ = false; + killCause_ = KillCause::operationTimeout; if (launchTimer_ != nullptr) launchTimer_->stop(); process_->kill(); - emit operationTimedOut(generation); - clearOperation(); + if (operationActive_ && process_->state() == QProcess::NotRunning) + finishProcess(-1, false); }); connect(process_, &QProcess::started, this, [this] { if (!operationActive_ || shuttingDown_) return; + if (killCause_ != KillCause::none) { process_->kill(); return; } if (launchTimer_ != nullptr) launchTimer_->stop(); if (operationTimer_ != nullptr) operationTimer_->start(); emit operationStarted(generation_); @@ -67,6 +66,13 @@ void FirmwareWorker::initialize() { connect(process_, &QProcess::errorOccurred, this, [this](const QProcess::ProcessError error) { if (!operationActive_ || shuttingDown_ || process_ == nullptr || error != QProcess::FailedToStart) return; + // A timeout during Starting can finish with FailedToStart rather than + // finished(). Treat it as termination acknowledgement, not a new + // operation result while the timed-out process still owns COM. + if (killCause_ != KillCause::none) { + if (process_->state() == QProcess::NotRunning) finishProcess(-1, false); + return; + } const quint64 generation = generation_; const QString message = process_->errorString(); operationActive_ = false; @@ -108,6 +114,7 @@ void FirmwareWorker::startOperation( generation_ = generation; operationActive_ = true; cancelRequested_ = false; + killCause_ = KillCause::none; process_->setProcessEnvironment(environment); process_->setProgram(program); process_->setArguments(arguments); @@ -125,8 +132,8 @@ void FirmwareWorker::cancel(const quint64 generation) { const quint64 cancelledGeneration = generation_; operationActive_ = false; cancelRequested_ = false; - emit operationCancelled(cancelledGeneration); clearOperation(); + emit operationCancelled(cancelledGeneration); return; } process_->kill(); @@ -149,19 +156,26 @@ void FirmwareWorker::finishProcess(const int exitCode, const bool normalExit) { } operationActive_ = false; - if (cancelRequested_) { - cancelRequested_ = false; + const KillCause retiredCause = killCause_; + const bool wasCancelled = cancelRequested_; + cancelRequested_ = false; + clearOperation(); + // Timeout is published only after the OS process has *actually* exited. + // The facade may now drop busy/COM ownership without racing a live flasher. + if (wasCancelled) { emit operationCancelled(completedGeneration); - clearOperation(); - return; + } else if (retiredCause == KillCause::launchTimeout) { + emit operationLaunchTimedOut(completedGeneration); + } else if (retiredCause == KillCause::operationTimeout) { + emit operationTimedOut(completedGeneration); + } else { + emit operationFinished(completedGeneration, exitCode, normalExit); } - - emit operationFinished(completedGeneration, exitCode, normalExit); - clearOperation(); } void FirmwareWorker::clearOperation() { generation_ = 0; + killCause_ = KillCause::none; } void FirmwareWorker::shutdown() { @@ -171,7 +185,7 @@ void FirmwareWorker::shutdown() { if (operationTimer_ != nullptr) operationTimer_->stop(); operationActive_ = false; cancelRequested_ = false; - generation_ = 0; + clearOperation(); if (process_ != nullptr && process_->state() != QProcess::NotRunning) { QObject::disconnect(process_, nullptr, this, nullptr); diff --git a/apps/arstack_studio/src/FirmwareWorker.hpp b/apps/arstack_studio/src/FirmwareWorker.hpp index a96cf0956..838f85f2a 100644 --- a/apps/arstack_studio/src/FirmwareWorker.hpp +++ b/apps/arstack_studio/src/FirmwareWorker.hpp @@ -44,6 +44,9 @@ public slots: void operationCancelled(quint64 generation); private: + friend class DeterministicSessionHarness; + + enum class KillCause { none, launchTimeout, operationTimeout }; void finishProcess(int exitCode, bool normalExit); void clearOperation(); @@ -55,4 +58,5 @@ public slots: bool shuttingDown_{false}; bool operationActive_{false}; bool cancelRequested_{false}; + KillCause killCause_{KillCause::none}; };