From fbafade78c74d17bb4dcd4262a650961d6b63d19 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 27 Apr 2026 05:29:40 -0500 Subject: [PATCH 1/2] [ota] Add wall-clock timeout to OTA data transfer loop The main `while (total < ota_size)` loop in `ESPHomeOTAComponent::handle_data_()` had a `// TODO: timeout check` and no wall-clock guard. If the uploader side dropped the TCP connection without a FIN/RST being delivered to the device (uploader process killed mid-transfer, NAT/router state dropped, packet loss eating the RST), `recv_fn` would never see a close and `s_err_fn` would never fire, so: - `pcb_` stays non-null - `rx_closed_` stays false - `waiting_for_data_()` stays true forever - the loop spins on `EWOULDBLOCK`, fed by `App.feed_wdt(); continue;`, indefinitely LwIP TCP keepalive is not enabled on the OTA socket, and even when enabled the default keepalive timer is on the order of hours, so the device is effectively unresponsive until power cycle. This matches the "device unresponsive until I do a hard power-reset" symptom in issue #15953. Track the timestamp of the last successful read and abort if no data arrives for `OTA_SOCKET_TIMEOUT_DATA` (90s, same constant the handshake, `readall_()`, and `writeall_()` already use). The timeout resets on every successful read so a slow but live link does not false-trigger. --- esphome/components/esphome/ota/ota_esphome.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/esphome/components/esphome/ota/ota_esphome.cpp b/esphome/components/esphome/ota/ota_esphome.cpp index 47f661a8eaa..028338ff782 100644 --- a/esphome/components/esphome/ota/ota_esphome.cpp +++ b/esphome/components/esphome/ota/ota_esphome.cpp @@ -350,8 +350,17 @@ void ESPHomeOTAComponent::handle_data_() { // Acknowledge MD5 OK - 1 byte this->write_byte_(ota::OTA_RESPONSE_BIN_MD5_OK); + // Track when we last received data so a silently-vanished peer (no FIN/RST + // delivered, e.g. uploader killed mid-transfer or NAT/router dropped state) + // can't wedge the device indefinitely. Without this, the loop only exits + // on actual data, EOF, or a non-EWOULDBLOCK error from read(), and lwIP + // TCP keepalive isn't enabled here. + uint32_t last_data_ms = millis(); while (total < ota_size) { - // TODO: timeout check + if (millis() - last_data_ms > OTA_SOCKET_TIMEOUT_DATA) { + ESP_LOGW(TAG, "No data received for %u ms", (unsigned) OTA_SOCKET_TIMEOUT_DATA); + goto error; // NOLINT(cppcoreguidelines-avoid-goto) + } size_t remaining = ota_size - total; size_t requested = remaining < OTA_BUFFER_SIZE ? remaining : OTA_BUFFER_SIZE; ssize_t read = this->client_->read(buf, requested); @@ -369,6 +378,7 @@ void ESPHomeOTAComponent::handle_data_() { goto error; // NOLINT(cppcoreguidelines-avoid-goto) } + last_data_ms = millis(); error_code = this->backend_->write(buf, read); if (error_code != ota::OTA_RESPONSE_OK) { ESP_LOGW(TAG, "Flash write err %d", error_code); From 801cfe621b29808529177e7d361d0d163b5c299f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 27 Apr 2026 05:54:32 -0500 Subject: [PATCH 2/2] [ota] Fix clang-tidy goto-jumps-init and set error_code on timeout Address two issues on the new data-loop timeout: - clang-tidy ESP32 Arduino flagged "cannot jump from this goto statement to its label" at the existing pre-loop `goto error` sites because the new `uint32_t last_data_ms = millis();` declaration sat between them and the `error:` label, so the jumps would skip past its initialization. Move the declaration up next to the other handle_data_ locals (`total`, `last_progress`) and just assign it before the loop entry. The earlier gotos no longer cross an init. - Copilot pointed out the new timeout `goto error` left `error_code` at `OTA_RESPONSE_OK` (set by `backend_->begin()`), so the device would write OK back to the uploader even though it was aborting. Set `error_code = ota::OTA_RESPONSE_ERROR_UNKNOWN` before the goto so the uploader receives a correct failure code. --- esphome/components/esphome/ota/ota_esphome.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/esphome/components/esphome/ota/ota_esphome.cpp b/esphome/components/esphome/ota/ota_esphome.cpp index 028338ff782..be771eb6899 100644 --- a/esphome/components/esphome/ota/ota_esphome.cpp +++ b/esphome/components/esphome/ota/ota_esphome.cpp @@ -292,6 +292,7 @@ void ESPHomeOTAComponent::handle_data_() { bool update_started = false; size_t total = 0; uint32_t last_progress = 0; + uint32_t last_data_ms = 0; uint8_t buf[OTA_BUFFER_SIZE]; char *sbuf = reinterpret_cast(buf); size_t ota_size; @@ -355,10 +356,11 @@ void ESPHomeOTAComponent::handle_data_() { // can't wedge the device indefinitely. Without this, the loop only exits // on actual data, EOF, or a non-EWOULDBLOCK error from read(), and lwIP // TCP keepalive isn't enabled here. - uint32_t last_data_ms = millis(); + last_data_ms = millis(); while (total < ota_size) { if (millis() - last_data_ms > OTA_SOCKET_TIMEOUT_DATA) { ESP_LOGW(TAG, "No data received for %u ms", (unsigned) OTA_SOCKET_TIMEOUT_DATA); + error_code = ota::OTA_RESPONSE_ERROR_UNKNOWN; goto error; // NOLINT(cppcoreguidelines-avoid-goto) } size_t remaining = ota_size - total;