fixed bug# 43739 (Accept not reporting errors about aborted connections): netconn_accept() returns ERR_ABRT (sockets: ECONNABORTED) for aborted connections, ERR_CLSD (sockets: EINVAL) if the listening netconn is closed, which better seems to follow the standard

CHANGELOG

@@ -317,6 +317,12 @@ HISTORY
 
   ++ Bugfixes:
 
+  2016-04-05: Simon Goldschmidt
+  * netconn/socket API: fixed bug# 43739 (Accept not reporting errors about
+    aborted connections): netconn_accept() returns ERR_ABRT (sockets: ECONNABORTED)
+    for aborted connections, ERR_CLSD (sockets: EINVAL) if the listening netconn
+    is closed, which better seems to follow the standard.
+
   2016-03-23: Florent Matignon
   * dhcp.c: fixed bug #38203: DHCP options are not recorded in all DHCP ack messages
 

src/api/api_lib.c

@@ -347,6 +347,7 @@ err_t
 netconn_accept(struct netconn *conn, struct netconn **new_conn)
 {
 #if LWIP_TCP
+  void *accept_ptr;
   struct netconn *newconn;
   err_t err;
 #if TCP_LISTEN_BACKLOG
@@ -356,7 +357,6 @@ netconn_accept(struct netconn *conn, struct netconn **new_conn)
   LWIP_ERROR("netconn_accept: invalid pointer",    (new_conn != NULL),                  return ERR_ARG;);
   *new_conn = NULL;
   LWIP_ERROR("netconn_accept: invalid conn",       (conn != NULL),                      return ERR_ARG;);
-  LWIP_ERROR("netconn_accept: invalid acceptmbox", sys_mbox_valid(&conn->acceptmbox),   return ERR_ARG;);
 
   err = conn->last_err;
   if (ERR_IS_FATAL(err)) {
@@ -364,24 +364,32 @@ netconn_accept(struct netconn *conn, struct netconn **new_conn)
        waiting on acceptmbox forever! */
     return err;
   }
-
+  if (!sys_mbox_valid(&conn->acceptmbox)) {
+    return ERR_CLSD;
+  }
 #if LWIP_SO_RCVTIMEO
-  if (sys_arch_mbox_fetch(&conn->acceptmbox, (void **)&newconn, conn->recv_timeout) == SYS_ARCH_TIMEOUT) {
+  if (sys_arch_mbox_fetch(&conn->acceptmbox, &accept_ptr, conn->recv_timeout) == SYS_ARCH_TIMEOUT) {
     return ERR_TIMEOUT;
   }
 #else
-  sys_arch_mbox_fetch(&conn->acceptmbox, (void **)&newconn, 0);
+  sys_arch_mbox_fetch(&conn->acceptmbox, &accept_ptr, 0);
 #endif /* LWIP_SO_RCVTIMEO*/
+  newconn = (struct netconn *)accept_ptr;
   /* Register event with callback */
   API_EVENT(conn, NETCONN_EVT_RCVMINUS, 0);
 
+  if (accept_ptr == &netconn_aborted) {
+    /* a connection has been aborted: out of pcbs or out of netconns during accept */
+    /* @todo: set netconn error, but this would be fatal and thus block further accepts */
+    return ERR_ABRT;
+  }
   if (newconn == NULL) {
     /* connection has been aborted */
     /* in this special case, we set the netconn error from application thread, as
        on a ready-to-accept listening netconn, there should not be anything running
        in tcpip_thread */
-    NETCONN_SET_SAFE_ERR(conn, ERR_ABRT);
-    return ERR_ABRT;
+    NETCONN_SET_SAFE_ERR(conn, ERR_CLSD);
+    return ERR_CLSD;
   }
 #if TCP_LISTEN_BACKLOG
   /* Let the stack know that we have accepted the connection. */

src/api/api_msg.c

@@ -83,6 +83,10 @@ static err_t lwip_netconn_do_close_internal(struct netconn *conn  WRITE_DELAYED_
 #define TCPIP_APIMSG_ACK(m)   do { NETCONN_SET_SAFE_ERR((m)->conn, (m)->err); sys_sem_signal(LWIP_API_MSG_SEM(m)); } while(0)
 #endif /* LWIP_TCPIP_CORE_LOCKING */
 
+#if LWIP_TCP
+u8_t netconn_aborted;
+#endif /* LWIP_TCP */
+
 #if LWIP_RAW
 /**
  * Receive callback function for RAW netconns.
@@ -468,23 +472,32 @@ accept_function(void *arg, struct tcp_pcb *newpcb, err_t err)
 
   LWIP_DEBUGF(API_MSG_DEBUG, ("accept_function: newpcb->tate: %s\n", tcp_debug_state_str(newpcb->state)));
 
-  if ((err != ERR_OK) || (arg == NULL)) {
+  if (conn == NULL) {
     return ERR_VAL;
   }
-  if (newpcb == NULL) {
-    /* @todo: out-of-pcbs during connect: pass on this error to the application */
-    return ERR_VAL;
-  }
-
   if (!sys_mbox_valid(&conn->acceptmbox)) {
     LWIP_DEBUGF(API_MSG_DEBUG, ("accept_function: acceptmbox already deleted\n"));
     return ERR_VAL;
   }
 
+  if (newpcb == NULL) {
+    /* out-of-pcbs during connect: pass on this error to the application */
+    if (sys_mbox_trypost(&conn->acceptmbox, &netconn_aborted) == ERR_OK) {
+      /* Register event with callback */
+      API_EVENT(conn, NETCONN_EVT_RCVPLUS, 0);
+    }
+    return ERR_VAL;
+  }
+
   /* We have to set the callback here even though
-   * the new socket is unknown. conn->socket is marked as -1. */
+   * the new socket is unknown. newconn->socket is marked as -1. */
   newconn = netconn_alloc(conn->type, conn->callback);
   if (newconn == NULL) {
+    /* outof netconns: pass on this error to the application */
+    if (sys_mbox_trypost(&conn->acceptmbox, &netconn_aborted) == ERR_OK) {
+      /* Register event with callback */
+      API_EVENT(conn, NETCONN_EVT_RCVPLUS, 0);
+    }
     return ERR_MEM;
   }
   newconn->pcb.tcp = newpcb;

src/api/sockets.c

@@ -514,9 +514,11 @@ lwip_accept(int s, struct sockaddr *addr, socklen_t *addrlen)
     LWIP_DEBUGF(SOCKETS_DEBUG, ("lwip_accept(%d): netconn_acept failed, err=%d\n", s, err));
     if (NETCONNTYPE_GROUP(netconn_type(sock->conn)) != NETCONN_TCP) {
       sock_set_errno(sock, EOPNOTSUPP);
-      return -1;
+    } else if (err == ERR_CLSD) {
+      sock_set_errno(sock, EINVAL);
+    } else {
+      sock_set_errno(sock, err_to_errno(err));
     }
-    sock_set_errno(sock, err_to_errno(err));
     return -1;
   }
   LWIP_ASSERT("newconn != NULL", newconn != NULL);

src/include/lwip/priv/api_msg.h

@@ -176,6 +176,10 @@ struct dns_api_msg {
 };
 #endif /* LWIP_DNS */
 
+#if LWIP_TCP
+extern u8_t netconn_aborted;
+#endif /* LWIP_TCP */
+
 void lwip_netconn_do_newconn         (void *m);
 void lwip_netconn_do_delconn         (void *m);
 void lwip_netconn_do_bind            (void *m);