From 9fac2903ebad14752bfaa2324f01e526240bc2f3 Mon Sep 17 00:00:00 2001 From: Peter Fabri Date: Thu, 15 Dec 2016 17:31:08 +0100 Subject: [PATCH 1/6] Added close_abort() function to WiFiClient This branch adds a close_abort() method to the WiFiClient class. How it works, what it does: Calling `close_abort()` will close and abort the client connection it is invoked on and, as a result, free its resources (i.e. memory). **WARNING:** aborting connections without a good reason violates the TCP protocol, because a closed connection would normally need to spend some time in `TIME_WAIT` state before its resources are freed. Usage example: WiFiClient client; // set up a client { /* do things with your client */ } client.stop_abort() // when you're done,abort the // connection if you must Why it's useful: 1. Give programmers a way to shut down connections immediately if need be. The underlying `tcp.c` file has an abort function, but this has not been directly accessible via the `WiFiClient` class until now. 2. There are a number of reported issues for the repository addressing the heap corruption that can result from trying to retain too many connections in `TIME_WAIT` state (most notably: #230, #1070, #1923). Although the warning above holds, there may be circumstances where this isn't very important. For example an ESP8266 running in AP mode hosting a page, which requests a new connection every second via an AJAX script to monitor a sensor/button/etc. continusously. Currently existing alternative approach: When building a project, defining the `-D MEMP_NUM_TCP_PCB_TIME_WAIT=5`compiler directive will limit the maximum number of clients allowed to stay in TIME_WAIT state. `5` is the default, lower it as necessary. See reference [here](https://github.com/esp8266/Arduino/blob/master/tools/sdk/lwip/include/lwipopts.h#L263) Thanks: Thank you to @me-no-dev, @everslick and @Palatis for bringing the ` MEMP_NUM_TCP_PCB_TIME_WAIT` option to my attention. --- libraries/ESP8266WiFi/src/WiFiClient.cpp | 9 +++++ libraries/ESP8266WiFi/src/WiFiClient.h | 1 + .../ESP8266WiFi/src/include/ClientContext.h | 36 +++++++++++++++++++ 3 files changed, 46 insertions(+) diff --git a/libraries/ESP8266WiFi/src/WiFiClient.cpp b/libraries/ESP8266WiFi/src/WiFiClient.cpp index 75a550b6ed..53d6c7e934 100644 --- a/libraries/ESP8266WiFi/src/WiFiClient.cpp +++ b/libraries/ESP8266WiFi/src/WiFiClient.cpp @@ -269,6 +269,15 @@ void WiFiClient::stop() _client = 0; } +void WiFiClient::stop_abort() //Compare this to ::stop +{ + if (!_client) + return; + + _client->unref_abort(); + _client = 0; +} + uint8_t WiFiClient::connected() { if (!_client) diff --git a/libraries/ESP8266WiFi/src/WiFiClient.h b/libraries/ESP8266WiFi/src/WiFiClient.h index dabd65f26e..ce5b072ed0 100644 --- a/libraries/ESP8266WiFi/src/WiFiClient.h +++ b/libraries/ESP8266WiFi/src/WiFiClient.h @@ -64,6 +64,7 @@ class WiFiClient : public Client, public SList { } virtual void flush(); virtual void stop(); + virtual void stop_abort(); virtual uint8_t connected(); virtual operator bool(); diff --git a/libraries/ESP8266WiFi/src/include/ClientContext.h b/libraries/ESP8266WiFi/src/include/ClientContext.h index d2ee0903b3..d4f2870c1b 100644 --- a/libraries/ESP8266WiFi/src/include/ClientContext.h +++ b/libraries/ESP8266WiFi/src/include/ClientContext.h @@ -86,6 +86,27 @@ class ClientContext } return err; } + + err_t close_abort() + { + /* ERR_ABRT must be sent back on abortion as specified in the comments + * of tools/sdk/lwip/src/core/tcp.c at the footnote of tcp_abort() */ + err_t err = ERR_ABRT; + if(_pcb) { + DEBUGV(":close\r\n"); + tcp_arg(_pcb, NULL); + tcp_sent(_pcb, NULL); + tcp_recv(_pcb, NULL); + tcp_err(_pcb, NULL); + tcp_close(_pcb); + /* Without delay some clients fail to receive the response and + * report a 'cannot connect' error message */ + delay(10); + tcp_abort(_pcb); + _pcb = 0; + } + return err; + } ~ClientContext() { @@ -123,6 +144,21 @@ class ClientContext } } } + + void unref_abort() + { + if(this != 0) { + DEBUGV(":ur_abrt %d\r\n", _refcnt); + if(--_refcnt == 0) { + flush(); + close_abort(); + if(_discard_cb) + _discard_cb(_discard_cb_arg, this); + DEBUGV(":del\r\n"); + delete this; + } + } + } void setNoDelay(bool nodelay) { From 98c64db36a737c2ca2381db47315438575c13ce5 Mon Sep 17 00:00:00 2001 From: david gauchard Date: Thu, 9 Jan 2020 00:32:57 +0100 Subject: [PATCH 2/6] merge close and close_abort --- .../ESP8266WiFi/src/include/ClientContext.h | 37 ++++++------------- 1 file changed, 12 insertions(+), 25 deletions(-) diff --git a/libraries/ESP8266WiFi/src/include/ClientContext.h b/libraries/ESP8266WiFi/src/include/ClientContext.h index 8f334af7e1..e07cd4ed10 100644 --- a/libraries/ESP8266WiFi/src/include/ClientContext.h +++ b/libraries/ESP8266WiFi/src/include/ClientContext.h @@ -66,19 +66,27 @@ class ClientContext return ERR_ABRT; } - err_t close() + err_t close(bool force_abort = false) { err_t err = ERR_OK; if(_pcb) { - DEBUGV(":close\r\n"); + if (force_abort) + DEBUGV(":closeabort\r\n"); + else + DEBUGV(":close\r\n"); tcp_arg(_pcb, NULL); tcp_sent(_pcb, NULL); tcp_recv(_pcb, NULL); tcp_err(_pcb, NULL); tcp_poll(_pcb, NULL, 0); err = tcp_close(_pcb); - if(err != ERR_OK) { - DEBUGV(":tc err %d\r\n", (int) err); + if(err != ERR_OK || force_abort) { + if (force_abort) + /* Without delay some clients fail to receive the response and + * report a 'cannot connect' error message */ + delay(10); + else + DEBUGV(":tc err %d\r\n", (int) err); tcp_abort(_pcb); err = ERR_ABRT; } @@ -86,27 +94,6 @@ class ClientContext } return err; } - - err_t close_abort() - { - /* ERR_ABRT must be sent back on abortion as specified in the comments - * of tools/sdk/lwip/src/core/tcp.c at the footnote of tcp_abort() */ - err_t err = ERR_ABRT; - if(_pcb) { - DEBUGV(":close\r\n"); - tcp_arg(_pcb, NULL); - tcp_sent(_pcb, NULL); - tcp_recv(_pcb, NULL); - tcp_err(_pcb, NULL); - tcp_close(_pcb); - /* Without delay some clients fail to receive the response and - * report a 'cannot connect' error message */ - delay(10); - tcp_abort(_pcb); - _pcb = 0; - } - return err; - } ~ClientContext() { From dedc77e0c7e54c063224bb8b8347bac3e8991940 Mon Sep 17 00:00:00 2001 From: david gauchard Date: Thu, 9 Jan 2020 00:43:16 +0100 Subject: [PATCH 3/6] merge close, abort and close_abort --- .../ESP8266WiFi/src/include/ClientContext.h | 39 ++++++++----------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/libraries/ESP8266WiFi/src/include/ClientContext.h b/libraries/ESP8266WiFi/src/include/ClientContext.h index e07cd4ed10..feba46b92f 100644 --- a/libraries/ESP8266WiFi/src/include/ClientContext.h +++ b/libraries/ESP8266WiFi/src/include/ClientContext.h @@ -51,41 +51,24 @@ class ClientContext //keepAlive(); } - err_t abort() + err_t close(bool force_close = true, bool force_abort = false) { + err_t err = force_close? ERR_OK: ERR_ABRT; if(_pcb) { - DEBUGV(":abort\r\n"); + DEBUGV(":close:%d abort:%d\r\n", force_close, force_abort); tcp_arg(_pcb, NULL); tcp_sent(_pcb, NULL); tcp_recv(_pcb, NULL); tcp_err(_pcb, NULL); tcp_poll(_pcb, NULL, 0); - tcp_abort(_pcb); - _pcb = nullptr; - } - return ERR_ABRT; - } - - err_t close(bool force_abort = false) - { - err_t err = ERR_OK; - if(_pcb) { - if (force_abort) - DEBUGV(":closeabort\r\n"); - else - DEBUGV(":close\r\n"); - tcp_arg(_pcb, NULL); - tcp_sent(_pcb, NULL); - tcp_recv(_pcb, NULL); - tcp_err(_pcb, NULL); - tcp_poll(_pcb, NULL, 0); - err = tcp_close(_pcb); + if (force_close) + err = tcp_close(_pcb); if(err != ERR_OK || force_abort) { if (force_abort) /* Without delay some clients fail to receive the response and * report a 'cannot connect' error message */ delay(10); - else + else DEBUGV(":tc err %d\r\n", (int) err); tcp_abort(_pcb); err = ERR_ABRT; @@ -95,6 +78,16 @@ class ClientContext return err; } + err_t abort() + { + return close(false, true); + } + + err_t close_abort(bool force_abort = false) + { + return close(true, true); + } + ~ClientContext() { } From 0df0005d364e1dcbcf85a865483f64d5e4ec8a9b Mon Sep 17 00:00:00 2001 From: david gauchard Date: Thu, 9 Jan 2020 00:46:47 +0100 Subject: [PATCH 4/6] merge unref & unref_abort --- .../ESP8266WiFi/src/include/ClientContext.h | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/libraries/ESP8266WiFi/src/include/ClientContext.h b/libraries/ESP8266WiFi/src/include/ClientContext.h index feba46b92f..ecf961e930 100644 --- a/libraries/ESP8266WiFi/src/include/ClientContext.h +++ b/libraries/ESP8266WiFi/src/include/ClientContext.h @@ -109,12 +109,12 @@ class ClientContext DEBUGV(":ref %d\r\n", _refcnt); } - void unref() + void unref(bool force_abort = false) { DEBUGV(":ur %d\r\n", _refcnt); if(--_refcnt == 0) { discard_received(); - close(); + close(true, force_abort); if(_discard_cb) { _discard_cb(_discard_cb_arg, this); } @@ -125,18 +125,8 @@ class ClientContext void unref_abort() { - if(this != 0) { - DEBUGV(":ur_abrt %d\r\n", _refcnt); - if(--_refcnt == 0) { - flush(); - close_abort(); - if(_discard_cb) - _discard_cb(_discard_cb_arg, this); - DEBUGV(":del\r\n"); - delete this; - } - } - } + unref(true); + } int connect(CONST ip_addr_t* addr, uint16_t port) { From 07104a72487e7c05d0d8d1cd5f4ea8e023dedc54 Mon Sep 17 00:00:00 2001 From: david gauchard Date: Thu, 9 Jan 2020 00:48:16 +0100 Subject: [PATCH 5/6] minimize diff --- libraries/ESP8266WiFi/src/WiFiClient.h | 1 - 1 file changed, 1 deletion(-) diff --git a/libraries/ESP8266WiFi/src/WiFiClient.h b/libraries/ESP8266WiFi/src/WiFiClient.h index 6650914973..f1469c3294 100644 --- a/libraries/ESP8266WiFi/src/WiFiClient.h +++ b/libraries/ESP8266WiFi/src/WiFiClient.h @@ -72,7 +72,6 @@ class WiFiClient : public Client, public SList { size_t peekBytes(char *buffer, size_t length) { return peekBytes((uint8_t *) buffer, length); } - virtual void flush() override { (void)flush(0); } virtual void stop() override { (void)stop(0); } bool flush(unsigned int maxWaitMs); From cc140484913a99ff2d6116a56c75d0e127f48b44 Mon Sep 17 00:00:00 2001 From: david gauchard Date: Thu, 9 Jan 2020 01:19:49 +0100 Subject: [PATCH 6/6] fix typo --- libraries/ESP8266WiFi/src/include/ClientContext.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/ESP8266WiFi/src/include/ClientContext.h b/libraries/ESP8266WiFi/src/include/ClientContext.h index ecf961e930..b11939027a 100644 --- a/libraries/ESP8266WiFi/src/include/ClientContext.h +++ b/libraries/ESP8266WiFi/src/include/ClientContext.h @@ -83,7 +83,7 @@ class ClientContext return close(false, true); } - err_t close_abort(bool force_abort = false) + err_t close_abort() { return close(true, true); }