From 7148859582eb641362406c9dbd4060b03dcc2f0a Mon Sep 17 00:00:00 2001
From: "torlando-agent[bot]"
 <281092095+torlando-agent[bot]@users.noreply.github.com>
Date: Fri, 19 Jun 2026 22:23:03 -0400
Subject: [PATCH] fix(tcp): real task join in stop() + atomic
 _last_connect_attempt (greptile)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- stop() now waits on a _task_done flag the task sets right before exiting,
  instead of a fixed sleep. Closes a use-after-free window where an in-flight
  connect() overrunning CONNECT_TIMEOUT_MS (slow DNS) could touch `this` after
  ~TCPClientInterface() freed it.
- _last_connect_attempt is now std::atomic<uint32_t> — it's read/written by
  task_loop() (core 0) and handle_disconnect() (core 1).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UWZuYkHBRqNb6BZHV8sTG5
---
 src/TCPClientInterface.cpp | 15 +++++++++++----
 src/TCPClientInterface.h   |  6 ++++++
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/src/TCPClientInterface.cpp b/src/TCPClientInterface.cpp
index 2cda6a52..85d9220e 100644
--- a/src/TCPClientInterface.cpp
+++ b/src/TCPClientInterface.cpp
@@ -265,7 +265,9 @@ void TCPClientInterface::handle_disconnect() {
 
 #ifdef ARDUINO
 /*static*/ void TCPClientInterface::tcp_task(void* arg) {
-    static_cast<TCPClientInterface*>(arg)->task_loop();
+    auto* self = static_cast<TCPClientInterface*>(arg);
+    self->task_loop();
+    self->_task_done = true;   // let stop() join before the object is freed
     vTaskDelete(nullptr);
 }
 
@@ -299,11 +301,16 @@ void TCPClientInterface::task_loop() {
 
 /*virtual*/ void TCPClientInterface::stop() {
 #ifdef ARDUINO
-    // Stop the connect task first; wait long enough for any in-flight connect()
-    // (bounded by CONNECT_TIMEOUT_MS) to finish so we don't close _client under it.
+    // Join the task: signal it, then wait until it has actually left task_loop()
+    // before tearing anything down. An in-flight connect() can overrun
+    // CONNECT_TIMEOUT_MS on a slow DNS server, and ~TCPClientInterface() calls
+    // stop() — returning early would risk a use-after-free on `this`.
     _task_running = false;
     if (_task_handle != nullptr) {
-        vTaskDelay(pdMS_TO_TICKS(CONNECT_TIMEOUT_MS + 500));
+        uint32_t deadline = millis() + CONNECT_TIMEOUT_MS + 2000;
+        while (!_task_done && (int32_t)(millis() - deadline) < 0) {
+            vTaskDelay(pdMS_TO_TICKS(20));
+        }
         _task_handle = nullptr;
     }
     _conn_state.store(DISCONNECTED);
diff --git a/src/TCPClientInterface.h b/src/TCPClientInterface.h
index 1fac05f2..de52846d 100644
--- a/src/TCPClientInterface.h
+++ b/src/TCPClientInterface.h
@@ -94,6 +94,7 @@ private:
     void task_loop();
     TaskHandle_t _task_handle = nullptr;
     volatile bool _task_running = false;
+    volatile bool _task_done = false;   // task sets this right before exit; stop() joins on it
     std::atomic<uint8_t> _conn_state{DISCONNECTED};
 #endif
 
@@ -107,7 +108,12 @@ private:
 
     // Connection state
     bool _initiator = true;
+#ifdef ARDUINO
+    // Touched by both task_loop() (core 0) and handle_disconnect() (core 1).
+    std::atomic<uint32_t> _last_connect_attempt{0};
+#else
     uint32_t _last_connect_attempt = 0;
+#endif
 #ifdef ARDUINO
     std::atomic<bool> _reconnected{false};  // re-established after offline (task-set)
 #else