From 75019860893bdd75001fffe2dcdd0ccc317ecf1c Mon Sep 17 00:00:00 2001 From: "Earle F. Philhower, III" Date: Tue, 25 Sep 2018 13:35:49 -0700 Subject: [PATCH 1/2] Fix TCP race condition, remove fixed delay in CC ClientContext::_wait_for_sent9) could dereference a TCP's _pcb after the connection was dropped by the OS, resulting in a crash. Move the connection dropped check to catch this case, and replace a fixed millisecond delay() with a yield and timeout value to minimize wasted time when transmission completes. --- .../ESP8266WiFi/src/include/ClientContext.h | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/libraries/ESP8266WiFi/src/include/ClientContext.h b/libraries/ESP8266WiFi/src/include/ClientContext.h index 86327dc403..dc6aae213d 100644 --- a/libraries/ESP8266WiFi/src/include/ClientContext.h +++ b/libraries/ESP8266WiFi/src/include/ClientContext.h @@ -308,13 +308,19 @@ class ClientContext if (!_pcb) return true; - int loop = -1; int prevsndbuf = -1; max_wait_ms++; // wait for peer's acks to flush lwIP's output buffer - + uint32_t last_sent = millis(); while (1) { + if (millis() - last_sent > (uint32_t) max_wait_ms) { +#ifdef DEBUGV + // wait until sent: timeout + DEBUGV(":wustmo\n"); +#endif + break; + } // force lwIP to send what can be sent tcp_output(_pcb); @@ -322,23 +328,17 @@ class ClientContext int sndbuf = tcp_sndbuf(_pcb); if (sndbuf != prevsndbuf) { // send buffer has changed (or first iteration) - // we received an ack: restart the loop counter + // we received an ack: restart the timeout prevsndbuf = sndbuf; - loop = max_wait_ms; + last_sent = millis(); // We just sent a bit, move timeout forward } - if (state() != ESTABLISHED || sndbuf == TCP_SND_BUF || --loop <= 0) - break; - - delay(1); - } + yield(); - #ifdef DEBUGV - if (loop <= 0) { - // wait until sent: timeout - DEBUGV(":wustmo\n"); + if ((state() != ESTABLISHED) || (sndbuf == TCP_SND_BUF)) { + break; + } } - #endif return max_wait_ms > 0; } From 9ee6b01239343c69915a7e07584cce5ec6fd6b8f Mon Sep 17 00:00:00 2001 From: "Earle F. Philhower, III" Date: Tue, 25 Sep 2018 14:17:55 -0700 Subject: [PATCH 2/2] Fix the return values and address review comments --- libraries/ESP8266WiFi/src/include/ClientContext.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/libraries/ESP8266WiFi/src/include/ClientContext.h b/libraries/ESP8266WiFi/src/include/ClientContext.h index dc6aae213d..e0841d1c52 100644 --- a/libraries/ESP8266WiFi/src/include/ClientContext.h +++ b/libraries/ESP8266WiFi/src/include/ClientContext.h @@ -309,7 +309,6 @@ class ClientContext return true; int prevsndbuf = -1; - max_wait_ms++; // wait for peer's acks to flush lwIP's output buffer uint32_t last_sent = millis(); @@ -319,7 +318,8 @@ class ClientContext // wait until sent: timeout DEBUGV(":wustmo\n"); #endif - break; + // All data was not flushed, timeout hit + return false; } // force lwIP to send what can be sent @@ -328,9 +328,9 @@ class ClientContext int sndbuf = tcp_sndbuf(_pcb); if (sndbuf != prevsndbuf) { // send buffer has changed (or first iteration) - // we received an ack: restart the timeout prevsndbuf = sndbuf; - last_sent = millis(); // We just sent a bit, move timeout forward + // We just sent a bit, move timeout forward + last_sent = millis(); } yield(); @@ -340,7 +340,8 @@ class ClientContext } } - return max_wait_ms > 0; + // All data flushed + return true; } uint8_t state() const