diff --git a/CHANGELOG b/CHANGELOG index cc584596..a637d7c7 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -26,6 +26,9 @@ HISTORY ++ Bugfixes: + 2017-02-24: Simon Goldschmidt + * sockets.c: fixed close race conditions in lwip_select (for LWIP_NETCONN_FULLDUPLEX) + 2017-02-24: Simon Goldschmidt * sockets.c: fixed that select ignored invalid/not open sockets in the fd_sets (bug #50392) diff --git a/src/api/sockets.c b/src/api/sockets.c index e8337149..fd7c953e 100644 --- a/src/api/sockets.c +++ b/src/api/sockets.c @@ -453,7 +453,7 @@ alloc_socket(struct netconn *newconn, int accepted) for (i = 0; i < NUM_SOCKETS; ++i) { /* Protect socket array */ SYS_ARCH_PROTECT(lev); - if (!sockets[i].conn && (sockets[i].select_waiting == 0)) { + if (!sockets[i].conn) { #if LWIP_NETCONN_FULLDUPLEX if (sockets[i].fd_used) { continue; @@ -1420,7 +1420,7 @@ lwip_selscan(int maxfdp1, fd_set *readset_in, fd_set *writeset_in, fd_set *excep } /* First get the socket's status (protected)... */ SYS_ARCH_PROTECT(lev); - sock = tryget_socket(i); + sock = tryget_socket_unconn(i); if (sock != NULL) { void* lastdata = sock->lastdata; s16_t rcvevent = sock->rcvevent; @@ -1463,6 +1463,68 @@ lwip_selscan(int maxfdp1, fd_set *readset_in, fd_set *writeset_in, fd_set *excep return nready; } +#if LWIP_NETCONN_FULLDUPLEX +/* Mark all of the set sockets in one of the three fdsets passed to select as used. + * All sockets are marked (and later unmarked), whether they are open or not. + * This is OK as lwip_selscan aborts select when non-open sockets are found. + */ +static void +lwip_select_inc_sockets_used_set(int maxfdp, fd_set *fdset, fd_set *used_sockets) +{ + SYS_ARCH_DECL_PROTECT(lev); + if (fdset) { + int i; + for (i = LWIP_SOCKET_OFFSET; i < maxfdp; i++) { + /* if this FD is in the set, lock it (unless already done) */ + if (FD_ISSET(i, fdset) && !FD_ISSET(i, used_sockets)) { + struct lwip_sock *sock; + SYS_ARCH_PROTECT(lev); + sock = tryget_socket_unconn(i); + if (sock != NULL) { + sock_inc_used(sock); + FD_SET(i, used_sockets); + } + SYS_ARCH_UNPROTECT(lev); + } + } + } +} + +/* Mark all sockets passed to select as used to prevent them from being freed + * from other threads while select is running. + * Marked sockets are added to 'used_sockets' to mark them only once an be able + * to unmark them correctly. + */ +static void +lwip_select_inc_sockets_used(int maxfdp, fd_set *fdset1, fd_set *fdset2, fd_set *fdset3, fd_set *used_sockets) +{ + FD_ZERO(used_sockets); + lwip_select_inc_sockets_used_set(maxfdp, fdset1, used_sockets); + lwip_select_inc_sockets_used_set(maxfdp, fdset2, used_sockets); + lwip_select_inc_sockets_used_set(maxfdp, fdset3, used_sockets); +} + +/* Let go all sockets that were marked as used when starting select */ +static void +lwip_select_dec_sockets_used(int maxfdp, fd_set *used_sockets) +{ + int i; + for (i = LWIP_SOCKET_OFFSET; i < maxfdp; i++) { + /* if this FD is not in the set, continue */ + if (FD_ISSET(i, used_sockets)) { + struct lwip_sock *sock = tryget_socket_unconn(i); + LWIP_ASSERT("socket gone at the end of select", sock != NULL); + if (sock != NULL) { + done_socket(sock); + } + } + } +} +#else /* LWIP_NETCONN_FULLDUPLEX */ +#define lwip_select_inc_sockets_used(maxfdp1, readset, writeset, exceptset, used_sockets) +#define lwip_select_dec_sockets_used(maxfdp1, used_sockets) +#endif /* LWIP_NETCONN_FULLDUPLEX */ + int lwip_select(int maxfdp1, fd_set *readset, fd_set *writeset, fd_set *exceptset, struct timeval *timeout) @@ -1476,6 +1538,9 @@ lwip_select(int maxfdp1, fd_set *readset, fd_set *writeset, fd_set *exceptset, int maxfdp2; #if LWIP_NETCONN_SEM_PER_THREAD int waited = 0; +#endif +#if LWIP_NETCONN_FULLDUPLEX + fd_set used_sockets; #endif SYS_ARCH_DECL_PROTECT(lev); @@ -1489,6 +1554,8 @@ lwip_select(int maxfdp1, fd_set *readset, fd_set *writeset, fd_set *exceptset, return -1; } + lwip_select_inc_sockets_used(maxfdp1, readset, writeset, exceptset, &used_sockets); + /* Go through each socket in each list to count number of sockets which currently match */ nready = lwip_selscan(maxfdp1, readset, writeset, exceptset, &lreadset, &lwriteset, &lexceptset); @@ -1524,6 +1591,7 @@ lwip_select(int maxfdp1, fd_set *readset, fd_set *writeset, fd_set *exceptset, if (sys_sem_new(&select_cb.sem, 0) != ERR_OK) { /* failed to create semaphore */ set_errno(ENOMEM); + lwip_select_dec_sockets_used(maxfdp1, &used_sockets); return -1; } #endif /* LWIP_NETCONN_SEM_PER_THREAD */ @@ -1551,7 +1619,7 @@ lwip_select(int maxfdp1, fd_set *readset, fd_set *writeset, fd_set *exceptset, (exceptset && FD_ISSET(i, exceptset))) { struct lwip_sock *sock; SYS_ARCH_PROTECT(lev); - sock = tryget_socket(i); + sock = tryget_socket_unconn(i); if (sock != NULL) { sock->select_waiting++; LWIP_ASSERT("sock->select_waiting > 0", sock->select_waiting > 0); @@ -1598,7 +1666,7 @@ lwip_select(int maxfdp1, fd_set *readset, fd_set *writeset, fd_set *exceptset, (exceptset && FD_ISSET(i, exceptset))) { struct lwip_sock *sock; SYS_ARCH_PROTECT(lev); - sock = tryget_socket(i); + sock = tryget_socket_unconn(i); if (sock != NULL) { /* for now, handle select_waiting==0... */ LWIP_ASSERT("sock->select_waiting > 0", sock->select_waiting > 0); @@ -1641,6 +1709,7 @@ lwip_select(int maxfdp1, fd_set *readset, fd_set *writeset, fd_set *exceptset, if (nready < 0) { /* This happens when a socket got closed while waiting */ set_errno(EBADF); + lwip_select_dec_sockets_used(maxfdp1, &used_sockets); return -1; } @@ -1658,6 +1727,7 @@ lwip_select(int maxfdp1, fd_set *readset, fd_set *writeset, fd_set *exceptset, LWIP_DEBUGF(SOCKETS_DEBUG, ("lwip_select: nready=%d\n", nready)); return_copy_fdsets: + lwip_select_dec_sockets_used(maxfdp1, &used_sockets); set_errno(0); if (readset) { *readset = lreadset;