From 368c80688a6d71fb5f02dc34a15370aa2d04abb1 Mon Sep 17 00:00:00 2001
From: Patrick Schurig <patrick@etm-schurig.eu>
Date: Fri, 3 Jul 2026 10:48:47 +0200
Subject: [PATCH] abbterra: fix TCP discovery SEGV + restore
 checkReachableRegister
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two coupled fixes so the ABB Terra AC can be added over Modbus TCP:

- Restore checkReachableRegister ("serialNumber") in the register JSON.
  The libnymea-modbus 1.15 code generator *requires* it — without it the
  connection classes (abbterramodbustcpconnection.h) are never generated and
  the plugin fails to build. Reverts the false lead in b23a9a2. The register
  is valid: mbpoll confirms holding reg 16384 (uint64) responds at slave 2.

- Fix a use-after-free in AbbTerraTcpDiscovery::cleanupConnection() that
  SIGSEGV'd nymead during network discovery whenever any Modbus device on the
  LAN answered on :502 with an exception (e.g. "illegal data address"). The
  method is invoked from several of the connection's own signals; it now
  bails out on the second (re-entrant) call and disconnects the connection's
  signals before disconnectDevice(), so the teardown can no longer re-enter
  while the QModbusReply "finished" handler is still dereferencing the reply.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 abbterra/abbterra-registers.json  |  1 +
 abbterra/abbterratcpdiscovery.cpp | 19 ++++++++++++++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/abbterra/abbterra-registers.json b/abbterra/abbterra-registers.json
index 4883cad..a2b385c 100644
--- a/abbterra/abbterra-registers.json
+++ b/abbterra/abbterra-registers.json
@@ -1,6 +1,7 @@
 {
     "className": "AbbTerra",
     "protocol": "BOTH",
+    "checkReachableRegister": "serialNumber",
     "endianness": "BigEndian",
     "stringEndianness": "BigEndian",
     "errorLimitUntilNotReachable": 3,
diff --git a/abbterra/abbterratcpdiscovery.cpp b/abbterra/abbterratcpdiscovery.cpp
index e0077f1..dd104ef 100644
--- a/abbterra/abbterratcpdiscovery.cpp
+++ b/abbterra/abbterratcpdiscovery.cpp
@@ -92,7 +92,24 @@ void AbbTerraTcpDiscovery::checkNetworkDevice(const QHostAddress &address)
 
 void AbbTerraTcpDiscovery::cleanupConnection(AbbTerraModbusTcpConnection *connection)
 {
-    m_connections.removeAll(connection);
+    // A single reachability failure fans out into several signals
+    // (reachableChanged, checkReachabilityFailed, connectionErrorOccurred), each
+    // wired to this method. removeAll() returns the number of entries actually
+    // removed, so the first call tears the connection down and every subsequent
+    // (possibly re-entrant) call becomes a no-op.
+    if (m_connections.removeAll(connection) == 0)
+        return;
+
+    // Stop reacting to any further signals from this connection (and its Modbus
+    // master) before tearing it down. Otherwise disconnectDevice() below can
+    // synchronously emit reachableChanged()/connectionErrorOccurred() and
+    // re-enter this method while we are still inside one of the connection's own
+    // signal handlers (e.g. the QModbusReply "finished" lambda that keeps
+    // dereferencing the reply after the emit) — a use-after-free that crashes
+    // nymead with SIGSEGV during network discovery.
+    disconnect(connection, nullptr, this, nullptr);
+    disconnect(connection->modbusTcpMaster(), nullptr, this, nullptr);
+
     connection->disconnectDevice();
     connection->deleteLater();
 }