socket/netconn recv: FIN should only be reported once

FIN should only be reported once (as '0' for sockets, as 'ERR_CLSD' for
netconns). Before this change, ERR_CLSD was returned forever...

This is the 2nd try. First try (commit ebcae98ae65c26a0c210c802540bf027d07fe2f1)
was buggy in that it could drop the FIN if it was read together with data
(reverted in commit ebcae98ae65c26a0c210c802540bf027d07fe2f1).

This version fixes this by adding an apiflag and a netconn flag to keep
track of this.

src/api/api_lib.c

@@ -646,8 +646,12 @@ netconn_recv_data_tcp(struct netconn *conn, struct pbuf **new_buf, u8_t apiflags
 #endif /* LWIP_TCP */
 
   if (!sys_mbox_valid(&conn->recvmbox)) {
-    /* This happens when calling this function after receiving FIN */
-    return sys_mbox_valid(&conn->acceptmbox) ? ERR_CONN : ERR_CLSD;
+    /* This only happens when calling this function more than once *after* receiving FIN */
+    return ERR_CONN;
+  }
+  if (netconn_is_flag_set(conn, NETCONN_FIN_RX_PENDING)) {
+    netconn_clear_flags(conn, NETCONN_FIN_RX_PENDING);
+    goto handle_fin;
   }
 
   if (!(apiflags & NETCONN_NOAUTORCVD)) {
@@ -675,19 +679,27 @@ netconn_recv_data_tcp(struct netconn *conn, struct pbuf **new_buf, u8_t apiflags
 
   /* If we are closed, we indicate that we no longer wish to use the socket */
   if (buf == NULL) {
-    API_EVENT(conn, NETCONN_EVT_RCVMINUS, 0);
-    if (conn->pcb.ip == NULL) {
-      /* race condition: RST during recv */
-      err = netconn_err(conn);
-      if (err != ERR_OK) {
-        return err;
+    if (apiflags & NETCONN_NOFIN) {
+      /* received a FIN but the caller cannot handle it right now:
+         re-enqueue it and return "no data" */
+      netconn_set_flags(conn, NETCONN_FIN_RX_PENDING);
+      return ERR_WOULDBLOCK;
+    } else {
+handle_fin:
+      API_EVENT(conn, NETCONN_EVT_RCVMINUS, 0);
+      if (conn->pcb.ip == NULL) {
+        /* race condition: RST during recv */
+        err = netconn_err(conn);
+        if (err != ERR_OK) {
+          return err;
+        }
+        return ERR_RST;
       }
-      return ERR_RST;
+      /* RX side is closed, so deallocate the recvmbox */
+      netconn_close_shutdown(conn, NETCONN_SHUT_RD);
+      /* Don' store ERR_CLSD as conn->err since we are only half-closed */
+      return ERR_CLSD;
     }
-    /* RX side is closed, so deallocate the recvmbox */
-    netconn_close_shutdown(conn, NETCONN_SHUT_RD);
-    /* Don' store ERR_CLSD as conn->err since we are only half-closed */
-    return ERR_CLSD;
   }
   return err;
 }

src/api/sockets.c

@@ -956,7 +956,7 @@ lwip_recv_tcp(struct lwip_sock *sock, void *mem, size_t len, int flags)
       }
     }
     /* once we have some data to return, only add more if we don't need to wait */
-    apiflags |= NETCONN_DONTBLOCK;
+    apiflags |= NETCONN_DONTBLOCK | NETCONN_NOFIN;
     /* @todo: do we need to support peeking more than one pbuf? */
   } while ((recv_left > 0) && !(flags & MSG_PEEK));
 lwip_recv_tcp_done:

src/include/lwip/api.h

@@ -64,6 +64,7 @@ extern "C" {
 #define NETCONN_MORE        0x02
 #define NETCONN_DONTBLOCK   0x04
 #define NETCONN_NOAUTORCVD  0x08 /* prevent netconn_recv_data_tcp() from updating the tcp window - must be done manually via netconn_tcp_recvd() */
+#define NETCONN_NOFIN       0x10 /* upper layer already received data, leave FIN in queue until called again */
 
 /* Flags for struct netconn.flags (u8_t) */
 /** This netconn had an error, don't block on recvmbox/acceptmbox any more */
@@ -85,7 +86,8 @@ extern "C" {
 /** Received packet info will be recorded for this netconn */
 #define NETCONN_FLAG_PKTINFO                  0x40
 #endif /* LWIP_NETBUF_RECVINFO */
-
+/** A FIN has been received but not passed to the application yet */
+#define NETCONN_FIN_RX_PENDING                0x80
 
 /* Helpers to process several netconn_types by the same code */
 #define NETCONNTYPE_GROUP(t)         ((t)&0xF0)

test/unit/api/test_sockets.c

@@ -173,9 +173,8 @@ static void test_sockets_allfunctions_basic_domain(int domain)
   ret = lwip_read(s2, buf, 1);
   fail_unless(ret == 0);
 
-  /* @todo: re-enable when this is working */
-  /* ret = lwip_read(s2, buf, 1);
-  fail_unless(ret == -1); */
+  ret = lwip_read(s2, buf, 1);
+  fail_unless(ret == -1);
 
   ret = lwip_write(s2, "foo", 3);
   fail_unless(ret == 3);
@@ -192,9 +191,8 @@ static void test_sockets_allfunctions_basic_domain(int domain)
   ret = lwip_read(s3, buf, 1);
   fail_unless(ret == 0);
 
-  /* @todo: re-enable when this is working */
-  /* ret = lwip_read(s3, buf, 1);
-  fail_unless(ret == -1); */
+  ret = lwip_read(s3, buf, 1);
+  fail_unless(ret == -1);
 
   while(tcpip_thread_poll_one());