From 533e6b5f8d866dc846d1ac82e0d3958ec13e2925 Mon Sep 17 00:00:00 2001 From: goldsimon Date: Fri, 8 Jan 2010 15:10:03 +0000 Subject: [PATCH] Fixed bug #26672 (close connection when receive window = 0) by correctly draining recvmbox/acceptmbox --- CHANGELOG | 4 ++ src/api/api_lib.c | 12 ++++ src/api/api_msg.c | 136 +++++++++++++++++++++++++++++-------- src/include/lwip/api_msg.h | 8 +++ 4 files changed, 132 insertions(+), 28 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index e564475e..dfd75e38 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -33,6 +33,10 @@ HISTORY ++ Bugfixes: + 2010-01-08: Simon Goldschmidt + * api_msg.h/.c, api_lib.c: Fixed bug #26672 (close connection when receive + window = 0) by correctly draining recvmbox/acceptmbox + 2010-01-08: Simon Goldschmidt * sockets.c: Fixed bug #28519 (lwip_recvfrom bug with len > 65535) diff --git a/src/api/api_lib.c b/src/api/api_lib.c index 195a9a52..3fe129f0 100644 --- a/src/api/api_lib.c +++ b/src/api/api_lib.c @@ -103,12 +103,24 @@ err_t netconn_delete(struct netconn *conn) { struct api_msg msg; + u32_t bytes_drained; + u16_t conns_drained; /* No ASSERT here because possible to get a (conn == NULL) if we got an accept error */ if (conn == NULL) { return ERR_OK; } + netconn_drain(conn, &bytes_drained, &conns_drained); + LWIP_ASSERT("Connection had recv-bytes and unaccepted connections pending -> mixed", + bytes_drained == 0 || conns_drained == 0); + + if (conns_drained != 0) { + msg.msg.msg.dc.drained = conns_drained | DELCONN_UNDRAINED_CONN; + } else { + LWIP_ASSERT("Too many bytes undrained", (bytes_drained & DELCONN_UNDRAINED_CONN) == 0); + msg.msg.msg.dc.drained = bytes_drained; + } msg.function = do_delconn; msg.msg.conn = conn; tcpip_apimsg(&msg); diff --git a/src/api/api_msg.c b/src/api/api_msg.c index 46c9abf2..96599eac 100644 --- a/src/api/api_msg.c +++ b/src/api/api_msg.c @@ -209,9 +209,17 @@ recv_tcp(void *arg, struct tcp_pcb *pcb, struct pbuf *p, err_t err) conn = arg; LWIP_ASSERT("recv_tcp: recv for wrong pcb!", conn->pcb.tcp == pcb); - if ((conn == NULL) || (conn->recvmbox == SYS_MBOX_NULL)) { + if (conn == NULL) { return ERR_VAL; } + if (conn->recvmbox == SYS_MBOX_NULL) { + /* recvmbox already deleted */ + if (p != NULL) { + tcp_recved(pcb, p->tot_len); + pbuf_free(p); + } + return ERR_OK; + } conn->err = err; if (p != NULL) { @@ -369,8 +377,10 @@ accept_function(void *arg, struct tcp_pcb *newpcb, err_t err) #endif /* API_MSG_DEBUG */ conn = (struct netconn *)arg; - LWIP_ERROR("accept_function: invalid conn->acceptmbox", - conn->acceptmbox != SYS_MBOX_NULL, return ERR_VAL;); + if (conn->acceptmbox == SYS_MBOX_NULL) { + LWIP_DEBUGF(API_MSG_DEBUG, ("accept_function: acceptmbox already deleted\n")); + return ERR_VAL; + } /* We have to set the callback here even though * the new socket is unknown. conn->socket is marked as -1. */ @@ -386,6 +396,9 @@ accept_function(void *arg, struct tcp_pcb *newpcb, err_t err) /* When returning != ERR_OK, the connection is aborted in tcp_process(), so do nothing here! */ newconn->pcb.tcp = NULL; + /* no need to call netconn_drain, since we know the state. */ + sys_mbox_free(newconn->recvmbox); + newconn->recvmbox = SYS_MBOX_NULL; netconn_free(newconn); return ERR_MEM; } else { @@ -571,32 +584,11 @@ netconn_alloc(enum netconn_type t, netconn_callback callback) void netconn_free(struct netconn *conn) { - void *mem; LWIP_ASSERT("PCB must be deallocated outside this function", conn->pcb.tcp == NULL); - - /* Drain the recvmbox. */ - if (conn->recvmbox != SYS_MBOX_NULL) { - while (sys_mbox_tryfetch(conn->recvmbox, &mem) != SYS_MBOX_EMPTY) { - if (conn->type == NETCONN_TCP) { - if(mem != NULL) { - pbuf_free((struct pbuf *)mem); - } - } else { - netbuf_delete((struct netbuf *)mem); - } - } - sys_mbox_free(conn->recvmbox); - conn->recvmbox = SYS_MBOX_NULL; - } - - /* Drain the acceptmbox. */ - if (conn->acceptmbox != SYS_MBOX_NULL) { - while (sys_mbox_tryfetch(conn->acceptmbox, &mem) != SYS_MBOX_EMPTY) { - netconn_delete((struct netconn *)mem); - } - sys_mbox_free(conn->acceptmbox); - conn->acceptmbox = SYS_MBOX_NULL; - } + LWIP_ASSERT("recvmbox must be deallocated before calling this function", + conn->recvmbox == SYS_MBOX_NULL); + LWIP_ASSERT("acceptmbox must be deallocated before calling this function", + conn->acceptmbox == SYS_MBOX_NULL); sys_sem_free(conn->op_completed); conn->op_completed = SYS_SEM_NULL; @@ -604,6 +596,59 @@ netconn_free(struct netconn *conn) memp_free(MEMP_NETCONN, conn); } +/** + * Delete rcvmbox and acceptmbox of a netconn and free the left-over data in + * these mboxes + * + * @param conn the netconn to free + * @bytes_drained bytes drained from recvmbox + * @accepts_drained pending connections drained from acceptmbox + */ +void +netconn_drain(struct netconn *conn, u32_t *bytes_drained, u16_t *accepts_drained) +{ + void *mem; + struct pbuf *p; + sys_mbox_t mbox; + SYS_ARCH_DECL_PROTECT(lev); + + *bytes_drained = 0; + *accepts_drained = 0; + + /* Delete and drain the recvmbox. */ + SYS_ARCH_PROTECT(lev); + mbox = conn->recvmbox; + conn->recvmbox = SYS_MBOX_NULL; + SYS_ARCH_UNPROTECT(lev); + if (mbox != SYS_MBOX_NULL) { + while (sys_mbox_tryfetch(mbox, &mem) != SYS_MBOX_EMPTY) { + if (conn->type == NETCONN_TCP) { + if(mem != NULL) { + p = (struct pbuf*)mem; + *bytes_drained += p->tot_len; + pbuf_free(p); + } + } else { + netbuf_delete((struct netbuf *)mem); + } + } + sys_mbox_free(mbox); + } + + /* Delete and drain the acceptmbox. */ + SYS_ARCH_PROTECT(lev); + mbox = conn->acceptmbox; + conn->acceptmbox = SYS_MBOX_NULL; + SYS_ARCH_UNPROTECT(lev); + if (mbox != SYS_MBOX_NULL) { + while (sys_mbox_tryfetch(mbox, &mem) != SYS_MBOX_EMPTY) { + *accepts_drained += 1; + netconn_delete((struct netconn *)mem); + } + sys_mbox_free(mbox); + } +} + #if LWIP_TCP /** * Internal helper function to close a TCP netconn: since this sometimes @@ -662,6 +707,38 @@ do_close_internal(struct netconn *conn) } #endif /* LWIP_TCP */ +/** + * Informs the core of freed data or deleted netconns that were left in + * recvmbox/acceptmbox when netconn_delete() was called. Calls tcp_recved + * or tcp_accepted for these. + * + * @param pcb the TCP pcb that is being closed + * @param drained Number and type of data drained + */ +static void +do_drained(struct tcp_pcb *pcb, u32_t drained) +{ + if (drained & DELCONN_UNDRAINED_CONN) { + /* acceptmbox was drained */ + u16_t conns_left = (u16_t)drained; + while(conns_left != 0) { + tcp_accepted(pcb); + conns_left--; + } + } else { + /* recvmbox was drained */ + u32_t bytes_left = drained; + while(bytes_left != 0) { + u16_t recved = 0xffff; + if (bytes_left < 0xffff) { + recved = (u16_t)bytes_left; + } + tcp_recved(pcb, recved); + bytes_left -= recved; + } + } +} + /** * Delete the pcb inside a netconn. * Called from netconn_delete. @@ -686,6 +763,9 @@ do_delconn(struct api_msg_msg *msg) #endif /* LWIP_UDP */ #if LWIP_TCP case NETCONN_TCP: + if (msg->msg.dc.drained != 0) { + do_drained(msg->conn->pcb.tcp, msg->msg.dc.drained); + } msg->conn->state = NETCONN_CLOSE; do_close_internal(msg->conn); /* API_EVENT is called inside do_close_internal, before releasing diff --git a/src/include/lwip/api_msg.h b/src/include/lwip/api_msg.h index d3b04568..3a7ca916 100644 --- a/src/include/lwip/api_msg.h +++ b/src/include/lwip/api_msg.h @@ -48,6 +48,9 @@ extern "C" { #endif +/* Used for do_delconn: MSB is set if there were undrained netconns in acceptmbox */ +#define DELCONN_UNDRAINED_CONN 0x80000000 + /* IP addresses and port numbers are expected to be in * the same byte order as in the corresponding pcb. */ @@ -100,6 +103,10 @@ struct api_msg_msg { u8_t backlog; } lb; #endif /* TCP_LISTEN_BACKLOG */ + /** used for do_delconn */ + struct { + u32_t drained; + } dc; } msg; }; @@ -151,6 +158,7 @@ void do_gethostbyname(void *arg); #endif /* LWIP_DNS */ struct netconn* netconn_alloc(enum netconn_type t, netconn_callback callback); +void netconn_drain(struct netconn *conn, u32_t *bytes_drained, u16_t *accepts_drained); void netconn_free(struct netconn *conn); #ifdef __cplusplus