Fixed bug #47485 (tcp_close() should not fail on memory error) by retrying to send FIN from tcp_fasttmr

CHANGELOG

@@ -5,7 +5,10 @@ HISTORY
   * [Enter new changes just after this line - do not remove this line]
 
   ++ New features:
-  
+
+  2017-02-10: Simon Goldschmidt
+  * tcp_close does not fail on memory error (instead, FIN is sent from tcp_tmr)
+
   2017-02-04: David van Moolenbroek
     - IPv6 scopes support
   

src/core/tcp.c

@@ -132,6 +132,8 @@ static u8_t tcp_timer;
 static u8_t tcp_timer_ctr;
 static u16_t tcp_new_port(void);
 
+static err_t tcp_close_shutdown_fin(struct tcp_pcb *pcb);
+
 /**
  * Initialize this module.
  */
@@ -258,8 +260,6 @@ tcp_backlog_accepted(struct tcp_pcb* pcb)
 static err_t
 tcp_close_shutdown(struct tcp_pcb *pcb, u8_t rst_on_unacked_data)
 {
-  err_t err;
-
   if (rst_on_unacked_data && ((pcb->state == ESTABLISHED) || (pcb->state == CLOSE_WAIT))) {
     if ((pcb->refused_data != NULL) || (pcb->rcv_wnd != TCP_WND_MAX(pcb))) {
       /* Not all data received by application, send RST to tell the remote
@@ -290,6 +290,8 @@ tcp_close_shutdown(struct tcp_pcb *pcb, u8_t rst_on_unacked_data)
     }
   }
 
+  /* - states which free the pcb are handled here,
+     - states which send FIN and change state are handled in  tcp_close_shutdown_impl() */
   switch (pcb->state) {
   case CLOSED:
     /* Closing a pcb in the CLOSED state might seem erroneous,
@@ -299,27 +301,34 @@ tcp_close_shutdown(struct tcp_pcb *pcb, u8_t rst_on_unacked_data)
      * or for a pcb that has been used and then entered the CLOSED state
      * is erroneous, but this should never happen as the pcb has in those cases
      * been freed, and so any remaining handles are bogus. */
-    err = ERR_OK;
     if (pcb->local_port != 0) {
       TCP_RMV(&tcp_bound_pcbs, pcb);
     }
     memp_free(MEMP_TCP_PCB, pcb);
-    pcb = NULL;
     break;
   case LISTEN:
-    err = ERR_OK;
     tcp_listen_closed(pcb);
     tcp_pcb_remove(&tcp_listen_pcbs.pcbs, pcb);
     memp_free(MEMP_TCP_PCB_LISTEN, pcb);
-    pcb = NULL;
     break;
   case SYN_SENT:
-    err = ERR_OK;
     TCP_PCB_REMOVE_ACTIVE(pcb);
     memp_free(MEMP_TCP_PCB, pcb);
-    pcb = NULL;
     MIB2_STATS_INC(mib2.tcpattemptfails);
     break;
+  default:
+    return tcp_close_shutdown_fin(pcb);
+  }
+  return ERR_OK;
+}
+
+static err_t
+tcp_close_shutdown_fin(struct tcp_pcb *pcb)
+{
+  err_t err;
+  LWIP_ASSERT("pcb != NULL", pcb != NULL);
+
+  switch (pcb->state) {
   case SYN_RCVD:
     err = tcp_send_fin(pcb);
     if (err == ERR_OK) {
@@ -344,18 +353,20 @@ tcp_close_shutdown(struct tcp_pcb *pcb, u8_t rst_on_unacked_data)
     break;
   default:
     /* Has already been closed, do nothing. */
-    err = ERR_OK;
-    pcb = NULL;
+    return ERR_OK;
     break;
   }
 
-  if (pcb != NULL && err == ERR_OK) {
+  if (err == ERR_OK) {
     /* To ensure all data has been sent when tcp_close returns, we have
        to make sure tcp_output doesn't fail.
        Since we don't really have to ensure all data has been sent when tcp_close
        returns (unsent data is sent from tcp timer functions, also), we don't care
        for the return value of tcp_output for now. */
     tcp_output(pcb);
+  } else if (err == ERR_MEM) {
+    /* Mark this pcb for closing. Closing is retried from tcp_tmr. */
+    pcb->flags |= TF_CLOSEPEND;
   }
   return err;
 }
@@ -1276,6 +1287,12 @@ tcp_fasttmr_start:
         tcp_output(pcb);
         pcb->flags &= ~(TF_ACK_DELAY | TF_ACK_NOW);
       }
+      /* send pending FIN */
+      if (pcb->flags & TF_CLOSEPEND) {
+        LWIP_DEBUGF(TCP_DEBUG, ("tcp_fasttmr: pending FIN\n"));
+        pcb->flags &= ~(TF_CLOSEPEND);
+        tcp_close_shutdown_fin(pcb);
+      }
 
       next = pcb->next;
 

src/include/lwip/tcp.h

@@ -145,7 +145,7 @@ typedef u32_t tcpwnd_size_t;
 typedef u16_t tcpwnd_size_t;
 #endif
 
-#if LWIP_WND_SCALE || TCP_LISTEN_BACKLOG
+#if LWIP_WND_SCALE || TCP_LISTEN_BACKLOG || LWIP_TCP_TIMESTAMPS
 typedef u16_t tcpflags_t;
 #else
 typedef u8_t tcpflags_t;
@@ -210,7 +210,7 @@ struct tcp_pcb {
 #define TF_ACK_DELAY   0x01U   /* Delayed ACK. */
 #define TF_ACK_NOW     0x02U   /* Immediate ACK. */
 #define TF_INFR        0x04U   /* In fast recovery. */
-#define TF_TIMESTAMP   0x08U   /* Timestamp option enabled */
+#define TF_CLOSEPEND   0x08U   /* If this is set, tcp_close failed to enqueue the FIN (retried in tcp_tmr) */
 #define TF_RXCLOSED    0x10U   /* rx closed by tcp_shutdown */
 #define TF_FIN         0x20U   /* Connection was closed locally (FIN segment enqueued). */
 #define TF_NODELAY     0x40U   /* Disable Nagle algorithm */
@@ -220,6 +220,9 @@ struct tcp_pcb {
 #endif
 #if TCP_LISTEN_BACKLOG
 #define TF_BACKLOGPEND 0x0200U /* If this is set, a connection pcb has increased the backlog on its listener */
+#endif
+#if LWIP_TCP_TIMESTAMPS
+#define TF_TIMESTAMP   0x0400U   /* Timestamp option enabled */
 #endif
 
   /* the rest of the fields are in host byte order
@@ -358,7 +361,11 @@ void             tcp_accept  (struct tcp_pcb *pcb, tcp_accept_fn accept);
 #endif /* LWIP_CALLBACK_API */
 void             tcp_poll    (struct tcp_pcb *pcb, tcp_poll_fn poll, u8_t interval);
 
+#if LWIP_TCP_TIMESTAMPS
 #define          tcp_mss(pcb)             (((pcb)->flags & TF_TIMESTAMP) ? ((pcb)->mss - 12)  : (pcb)->mss)
+#else /* LWIP_TCP_TIMESTAMPS */
+#define          tcp_mss(pcb)             ((pcb)->mss)
+#endif /* LWIP_TCP_TIMESTAMPS */
 #define          tcp_sndbuf(pcb)          (TCPWND16((pcb)->snd_buf))
 #define          tcp_sndqueuelen(pcb)     ((pcb)->snd_queuelen)
 /** @ingroup tcp_raw */