TCP: fixes backlog limit handling, and allows ephemeral TCP connectio…

…ns (#7096) * fix backlog limit handling, and allow ephemeral TCP connections * do not retain empty connections
esp8266 · Feb 24, 2020 · 2d58be7 · 2d58be7
1 parent 2b640c0
commit 2d58be7
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 33 deletions.
diff --git a/libraries/ESP8266WiFi/src/WiFiClient.cpp b/libraries/ESP8266WiFi/src/WiFiClient.cpp
@@ -346,7 +346,7 @@ uint8_t WiFiClient::status()
 
 WiFiClient::operator bool()
 {
-    return connected();
+    return available() || connected();
 }
 
 IPAddress WiFiClient::remoteIP()

diff --git a/libraries/ESP8266WiFi/src/WiFiServer.cpp b/libraries/ESP8266WiFi/src/WiFiServer.cpp
@@ -125,12 +125,14 @@ WiFiClient WiFiServer::available(byte* status) {
     if (_unclaimed) {
         WiFiClient result(_unclaimed);
 #if LWIP_VERSION_MAJOR != 1
-        _unclaimed->acceptPCB();
-        tcp_backlog_accepted(_unclaimed->getPCB());
+        // pcb can be null when peer has already closed the connection
+        if (_unclaimed->getPCB())
+            // give permission to lwIP to accept one more peer
+            tcp_backlog_accepted(_unclaimed->getPCB());
 #endif
         _unclaimed = _unclaimed->next();
         result.setNoDelay(getNoDelay());
-        DEBUGV("WS:av\r\n");
+        DEBUGV("WS:av status=%d WCav=%d\r\n", result.status(), result.available());
         return result;
     }
 
@@ -187,9 +189,12 @@ long WiFiServer::_accept(tcp_pcb* apcb, long err) {
     (void) err;
     DEBUGV("WS:ac\r\n");
 
+    // always accept new PCB so incoming data can be stored in our buffers even before
+    // user calls ::available()
+    ClientContext* client = new ClientContext(apcb, &WiFiServer::_s_discard, this);
+
 #if LWIP_VERSION_MAJOR == 1
 
-    ClientContext* client = new ClientContext(apcb, &WiFiServer::_s_discard, this);
     tcp_accepted(_listen_pcb);
 
 #else
@@ -198,20 +203,9 @@ long WiFiServer::_accept(tcp_pcb* apcb, long err) {
     // http://lwip.100.n7.nabble.com/Problem-re-opening-listening-pbc-tt32484.html#a32494
     // https://www.nongnu.org/lwip/2_1_x/group__tcp__raw.html#gaeff14f321d1eecd0431611f382fcd338
 
-    // lwip-v2: Tell ClientContext to not accept yet the connection (final 'false' below)
-    ClientContext* client = new ClientContext(apcb, &WiFiServer::_s_discard, this, false);
     // increase lwIP's backlog
     tcp_backlog_delayed(apcb);
 
-    // Optimization Path:
-    // when lwip-v1.4 is not allowed anymore,
-    // - _accept() should not create ClientContext anymore
-    // - apcb should be stored into some sort of linked list (->_unclaimed)
-    //   (the linked list would store tcp_pcb* instead of ClientContext*)
-    //   (TCP_PCB_EXTARGS might be used for that (lwIP's struct tcp_pcb))
-    // - on available(), get the pcb back and create the ClientContext
-    // (this is not done today for better source readability with lwip-1.4 around)
-
 #endif
 
     _unclaimed = slist_append_tail(_unclaimed, client);

diff --git a/libraries/ESP8266WiFi/src/include/ClientContext.h b/libraries/ESP8266WiFi/src/include/ClientContext.h
@@ -36,20 +36,9 @@ bool getDefaultPrivateGlobalSyncValue ();
 class ClientContext
 {
 public:
-    ClientContext(tcp_pcb* pcb, discard_cb_t discard_cb, void* discard_cb_arg, bool acceptNow = true) :
+    ClientContext(tcp_pcb* pcb, discard_cb_t discard_cb, void* discard_cb_arg) :
         _pcb(pcb), _rx_buf(0), _rx_buf_offset(0), _discard_cb(discard_cb), _discard_cb_arg(discard_cb_arg), _refcnt(0), _next(0),
         _sync(::getDefaultPrivateGlobalSyncValue())
-    {
-        if (acceptNow)
-            acceptPCB();
-    }
-
-    tcp_pcb* getPCB ()
-    {
-        return _pcb;
-    }
-
-    void acceptPCB()
     {
         tcp_setprio(_pcb, TCP_PRIO_MIN);
         tcp_arg(_pcb, this);
@@ -62,6 +51,11 @@ class ClientContext
         //keepAlive();
     }
 
+    tcp_pcb* getPCB ()
+    {
+        return _pcb;
+    }
+
     err_t abort()
     {
         if(_pcb) {
@@ -302,6 +296,7 @@ class ClientContext
 
     void discard_received()
     {
+        DEBUGV(":dsrcv %d\n", _rx_buf? _rx_buf->tot_len: 0);
         if(!_rx_buf) {
             return;
         }
@@ -360,7 +355,8 @@ class ClientContext
 
     uint8_t state() const
     {
-        if(!_pcb) {
+        if(!_pcb || _pcb->state == CLOSE_WAIT || _pcb->state == CLOSING) {
+            // CLOSED for WiFIClient::status() means nothing more can be written
             return CLOSED;
         }
 
@@ -587,11 +583,23 @@ class ClientContext
     {
         (void) pcb;
         (void) err;
-        if(pb == 0) { // connection closed
-            DEBUGV(":rcl\r\n");
+        if(pb == 0) {
+            // connection closed by peer
+            DEBUGV(":rcl pb=%p sz=%d\r\n", _rx_buf, _rx_buf? _rx_buf->tot_len: -1);
             _notify_error();
-            abort();
-            return ERR_ABRT;
+            if (_rx_buf && _rx_buf->tot_len)
+            {
+                // there is still something to read
+                return ERR_OK;
+            }
+            else
+            {
+                // nothing in receive buffer,
+                // peer closed = nothing can be written:
+                // closing in the legacy way
+                abort();
+                return ERR_ABRT;
+            }
         }
 
         if(_rx_buf) {