Skip to content

Commit 5e89b95

Browse files
committed
Fix adapter process restart race conditions
- Replace fire-and-forget QTimer::singleShot in AdapterProcess::stop() with a cancellable QTimer that is stopped on start, stop, and process exit, preventing the kill timer from killing a newly started adapter. - Suppress sessionError emission from onProcessError during STOPPING state to avoid spurious error signals during intentional shutdown. - Disconnect sessionStopped from initAdapter on sessionError in ModbusPoll to prevent auto-restart after an unexpected crash. - Add regression tests for processError during STOPPING state.
1 parent 3dfc900 commit 5e89b95

6 files changed

Lines changed: 68 additions & 8 deletions

File tree

src/ProtocolAdapter/adapterclient.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,11 @@ void AdapterClient::onErrorReceived(int id, const QString& method, const QJsonOb
140140
void AdapterClient::onProcessError(const QString& message)
141141
{
142142
_handshakeTimer.stop();
143-
_state = State::IDLE;
144-
emit sessionError(message);
143+
if (_state != State::STOPPING)
144+
{
145+
_state = State::IDLE;
146+
emit sessionError(message);
147+
}
145148
}
146149

147150
void AdapterClient::onProcessFinished()

src/ProtocolAdapter/adapterprocess.cpp

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,15 @@ AdapterProcess::AdapterProcess(QObject* parent) : QObject(parent)
1313
_pProcess = new QProcess(this);
1414
_pFramingReader = new FramingReader(this);
1515

16+
_pKillTimer = new QTimer(this);
17+
_pKillTimer->setSingleShot(true);
18+
connect(_pKillTimer, &QTimer::timeout, this, [this]() {
19+
if (_pProcess->state() != QProcess::NotRunning)
20+
{
21+
_pProcess->kill();
22+
}
23+
});
24+
1625
connect(_pProcess, &QProcess::readyReadStandardOutput, this, &AdapterProcess::onReadyReadStdout);
1726
connect(_pProcess, &QProcess::readyReadStandardError, this, &AdapterProcess::onReadyReadStderr);
1827
connect(_pProcess, &QProcess::finished, this, &AdapterProcess::onProcessFinished);
@@ -27,6 +36,7 @@ bool AdapterProcess::start(const QString& path)
2736
return true;
2837
}
2938

39+
_pKillTimer->stop();
3040
_pendingMethods.clear();
3141
_nextRequestId = 1;
3242

@@ -51,12 +61,8 @@ void AdapterProcess::stop()
5161
if (_pProcess->state() != QProcess::NotRunning)
5262
{
5363
_pProcess->closeWriteChannel();
54-
QTimer::singleShot(cStopTimeoutMs, this, [this]() {
55-
if (_pProcess->state() != QProcess::NotRunning)
56-
{
57-
_pProcess->kill();
58-
}
59-
});
64+
_pKillTimer->stop();
65+
_pKillTimer->start(cStopTimeoutMs);
6066
}
6167
}
6268

@@ -173,5 +179,6 @@ void AdapterProcess::onProcessFinished(int exitCode, QProcess::ExitStatus exitSt
173179
{
174180
qCInfo(scopeComm) << "AdapterProcess: process finished, exit code:" << exitCode << "status:" << exitStatus;
175181
_pendingMethods.clear();
182+
_pKillTimer->stop();
176183
emit processFinished();
177184
}

src/ProtocolAdapter/adapterprocess.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <QObject>
1010
#include <QProcess>
1111
#include <QString>
12+
#include <QTimer>
1213

1314
/*!
1415
* \brief Transport layer for an external adapter process communicating via JSON-RPC 2.0 over stdio.
@@ -104,6 +105,7 @@ private slots:
104105
bool writeFramed(const QByteArray& json);
105106

106107
QProcess* _pProcess;
108+
QTimer* _pKillTimer;
107109
FramingReader* _pFramingReader;
108110
QMap<int, QString> _pendingMethods;
109111
int _nextRequestId{ 1 };

src/communication/modbuspoll.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ ModbusPoll::ModbusPoll(SettingsModel* pSettingsModel, QObject* parent) : QObject
2828
connect(_pAdapterClient, &AdapterClient::sessionError, this, [this](QString message) {
2929
qCWarning(scopeComm) << "AdapterClient error:" << message;
3030
_bPollActive = false;
31+
disconnect(_pAdapterClient, &AdapterClient::sessionStopped, this, &ModbusPoll::initAdapter);
3132
});
3233
connect(_pAdapterClient, &AdapterClient::sessionStopped, this, &ModbusPoll::initAdapter);
3334
connect(_pAdapterClient, &AdapterClient::diagnosticReceived, this, &ModbusPoll::onAdapterDiagnostic);

tests/ProtocolAdapter/tst_adapterclient.cpp

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -565,4 +565,49 @@ void TestAdapterClient::shutdownAckEmitsSessionStoppedAfterProcessExit()
565565
QCOMPARE(spyError.count(), 0);
566566
}
567567

568+
void TestAdapterClient::processErrorDuringStoppingNoSessionError()
569+
{
570+
auto* mock = new MockAdapterProcess();
571+
AdapterClient client(mock);
572+
573+
QSignalSpy spyError(&client, &AdapterClient::sessionError);
574+
QSignalSpy spyStopped(&client, &AdapterClient::sessionStopped);
575+
576+
client.prepareAdapter(QStringLiteral("./dummy"));
577+
mock->injectResponse(1, "adapter.initialize", QJsonObject{ { "status", "ok" } });
578+
mock->injectResponse(2, "adapter.describe", describeResult());
579+
client.provideConfig(QJsonObject(), QStringList());
580+
mock->injectResponse(3, "adapter.configure", QJsonObject{ { "status", "ok" } });
581+
mock->injectResponse(4, "adapter.start", QJsonObject{ { "status", "ok" } });
582+
583+
client.stopSession();
584+
mock->injectProcessError(QStringLiteral("Adapter process crashed"));
585+
586+
QCOMPARE(spyError.count(), 0);
587+
QCOMPARE(spyStopped.count(), 0);
588+
}
589+
590+
void TestAdapterClient::processErrorDuringStoppingThenProcessFinished()
591+
{
592+
auto* mock = new MockAdapterProcess();
593+
AdapterClient client(mock);
594+
595+
QSignalSpy spyError(&client, &AdapterClient::sessionError);
596+
QSignalSpy spyStopped(&client, &AdapterClient::sessionStopped);
597+
598+
client.prepareAdapter(QStringLiteral("./dummy"));
599+
mock->injectResponse(1, "adapter.initialize", QJsonObject{ { "status", "ok" } });
600+
mock->injectResponse(2, "adapter.describe", describeResult());
601+
client.provideConfig(QJsonObject(), QStringList());
602+
mock->injectResponse(3, "adapter.configure", QJsonObject{ { "status", "ok" } });
603+
mock->injectResponse(4, "adapter.start", QJsonObject{ { "status", "ok" } });
604+
605+
client.stopSession();
606+
mock->injectProcessError(QStringLiteral("Adapter process crashed"));
607+
mock->injectProcessFinished();
608+
609+
QCOMPARE(spyError.count(), 0);
610+
QCOMPARE(spyStopped.count(), 1);
611+
}
612+
568613
QTEST_GUILESS_MAIN(TestAdapterClient)

tests/ProtocolAdapter/tst_adapterclient.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ private slots:
3131
void awaitingConfigPausesBeforeConfigure();
3232
void stopSessionDuringAwaitingConfig();
3333
void shutdownAckEmitsSessionStoppedAfterProcessExit();
34+
void processErrorDuringStoppingNoSessionError();
35+
void processErrorDuringStoppingThenProcessFinished();
3436
};
3537

3638
#endif // TST_ADAPTERCLIENT_H

0 commit comments

Comments
 (0)