diff --git a/CHANGELOG b/CHANGELOG index 9b71c25b..5adb50e0 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -241,6 +241,12 @@ HISTORY ++ Bug fixes: + 2007-07-03 Simon Goldschmidt + * api.h, api_lib.c, api_msg.c: Final fix for bug #20021 and some other problems + when closing tcp netconns: removed conn->sem, less context switches when + closing, both netconn_close and netconn_delete should safely close tcp + connections. + 2007-07-02 Simon Goldschmidt * ipv4/ip.h, ipv6/ip.h, opt.h, netif.h, etharp.h, ipv4/ip.c, netif.c, raw.c, tcp_out.c, udp.c, etharp.c: Added option LWIP_NETIF_HWADDRHINT (default=off) diff --git a/src/api/api_lib.c b/src/api/api_lib.c index 4751129a..d696ff28 100644 --- a/src/api/api_lib.c +++ b/src/api/api_lib.c @@ -314,12 +314,6 @@ netconn *netconn_new_with_proto_and_callback(enum netconn_type t, u8_t proto, } conn->recvmbox = SYS_MBOX_NULL; conn->acceptmbox = SYS_MBOX_NULL; - conn->sem = sys_sem_new(0); - if (conn->sem == SYS_SEM_NULL) { - sys_mbox_free(conn->mbox); - memp_free(MEMP_NETCONN, conn); - return NULL; - } conn->state = NETCONN_NONE; conn->socket = 0; conn->callback = callback; @@ -334,7 +328,6 @@ netconn *netconn_new_with_proto_and_callback(enum netconn_type t, u8_t proto, TCPIP_APIMSG(&msg); if (conn->err != ERR_OK) { - sys_sem_free(conn->sem); sys_mbox_free(conn->mbox); memp_free(MEMP_NETCONN, conn); return NULL; @@ -364,7 +357,7 @@ netconn_delete(struct netconn *conn) msg.function = do_delconn; msg.msg.conn = conn; - TCPIP_APIMSG(&msg); + tcpip_apimsg(&msg); /* Drain the recvmbox. */ if (conn->recvmbox != SYS_MBOX_NULL) { @@ -392,11 +385,6 @@ netconn_delete(struct netconn *conn) sys_mbox_free(conn->mbox); conn->mbox = SYS_MBOX_NULL; - if (conn->sem != SYS_SEM_NULL) { - sys_sem_free(conn->sem); - /* conn->sem = SYS_SEM_NULL; */ - } - memp_free(MEMP_NETCONN, conn); return ERR_OK; } @@ -812,16 +800,9 @@ netconn_close(struct netconn *conn) LWIP_ERROR("netconn_close: invalid conn", (conn != NULL), return ERR_ARG;); - conn->state = NETCONN_CLOSE; - again: msg.function = do_close; msg.msg.conn = conn; - TCPIP_APIMSG(&msg); - if (conn->err == ERR_MEM && conn->sem != SYS_SEM_NULL) { - sys_sem_wait(conn->sem); - goto again; - } - conn->state = NETCONN_NONE; + tcpip_apimsg(&msg); return conn->err; } diff --git a/src/api/api_msg.c b/src/api/api_msg.c index 9bb3c463..0ef1db2e 100644 --- a/src/api/api_msg.c +++ b/src/api/api_msg.c @@ -43,6 +43,7 @@ /* forward declarations */ #if LWIP_TCP static err_t do_writemore(struct netconn *conn); +static void do_close_internal(struct netconn *conn); #endif #if LWIP_RAW @@ -188,8 +189,8 @@ poll_tcp(void *arg, struct tcp_pcb *pcb) if (conn->state == NETCONN_WRITE) { do_writemore(conn); - } else if ((conn->state == NETCONN_CLOSE) && (conn->sem != SYS_SEM_NULL)) { - sys_sem_signal(conn->sem); + } else if (conn->state == NETCONN_CLOSE) { + do_close_internal(conn); } return ERR_OK; @@ -212,8 +213,8 @@ sent_tcp(void *arg, struct tcp_pcb *pcb, u16_t len) if (conn->state == NETCONN_WRITE) { do_writemore(conn); - } else if ((conn->state == NETCONN_CLOSE) && (conn->sem != SYS_SEM_NULL)) { - sys_sem_signal(conn->sem); + } else if (conn->state == NETCONN_CLOSE) { + do_close_internal(conn); } if (conn && conn->callback) @@ -257,17 +258,13 @@ err_tcp(void *arg, err_t err) (*conn->callback)(conn, NETCONN_EVT_RCVPLUS, 0); sys_mbox_post(conn->acceptmbox, NULL); } - if (conn->state == NETCONN_WRITE) { - /* calling do_writemore is not necessary + if ((conn->state == NETCONN_WRITE) || (conn->state == NETCONN_CLOSE)) { + /* calling do_writemore/do_close_internal is not necessary since the pcb has already been deleted! */ conn->state = NETCONN_NONE; /* wake up the waiting task */ sys_mbox_post(conn->mbox, NULL); - } else - if ((conn->sem != SYS_SEM_NULL) && - (conn->state == NETCONN_CLOSE)) { - sys_sem_signal(conn->sem); - } + } } /** @@ -326,13 +323,6 @@ accept_function(void *arg, struct tcp_pcb *newpcb, err_t err) memp_free(MEMP_NETCONN, newconn); return ERR_MEM; } - newconn->sem = sys_sem_new(0); - if (newconn->sem == SYS_SEM_NULL) { - sys_mbox_free(newconn->mbox); - sys_mbox_free(newconn->recvmbox); - memp_free(MEMP_NETCONN, newconn); - return ERR_MEM; - } /* Allocations were OK, setup the PCB etc */ newconn->type = NETCONN_TCP; newconn->pcb.tcp = newpcb; @@ -433,6 +423,60 @@ do_newconn(struct api_msg_msg *msg) TCPIP_APIMSG_ACK(msg); } +static void +do_close_internal(struct netconn *conn) +{ + LWIP_ASSERT("invalid conn", (conn != NULL)); + LWIP_ASSERT("this is for tcp netconns only", (conn->type == NETCONN_TCP)); + LWIP_ASSERT("conn must be in state NETCONN_CLOSE", (conn->state == NETCONN_CLOSE)); + LWIP_ASSERT("pcb already closed", (conn->pcb.tcp != NULL)); + + if (conn->pcb.tcp->state == LISTEN) { + tcp_arg(conn->pcb.tcp, NULL); + tcp_accept(conn->pcb.tcp, NULL); + /* for state==LISTEN, tcp_close can't fail */ + tcp_close(conn->pcb.tcp); + conn->state = NETCONN_NONE; + conn->pcb.tcp = NULL; + conn->err = ERR_OK; + sys_mbox_post(conn->mbox, NULL); + } else { + err_t err; + enum tcp_state old_state = conn->pcb.tcp->state; + if ((old_state == SYN_RCVD) || (old_state == ESTABLISHED) || + (old_state == CLOSE_WAIT)) { + tcp_arg(conn->pcb.tcp, NULL); + tcp_sent(conn->pcb.tcp, NULL); + tcp_recv(conn->pcb.tcp, NULL); + tcp_poll(conn->pcb.tcp, NULL, 0); + tcp_err(conn->pcb.tcp, NULL); + err = tcp_close(conn->pcb.tcp); + switch (err) { + default: + /* any other error: abort! */ + tcp_abort(conn->pcb.tcp); + /* fall through */ + case (ERR_OK): + /* ERR_OK: fall through */ + case (ERR_BUF): + /* ERR_BUF: tcp_output failed, + will be called again by internal tcp timers */ + conn->state = NETCONN_NONE; + conn->pcb.tcp = NULL; + conn->err = err; + /* wake up the application task */ + sys_mbox_post(conn->mbox, NULL); + break; + case (ERR_MEM): + /* ERR_MEM: error sending FIN? + try again in sent_tcp or poll_tcp */ + /* stay in state NETCONN_CLOSE */ + break; + } + } + } +} + /** * Delete the pcb inside a netconn. * Called from netconn_delete. @@ -457,20 +501,8 @@ do_delconn(struct api_msg_msg *msg) #endif /* LWIP_UDP */ #if LWIP_TCP case NETCONN_TCP: - if (msg->conn->pcb.tcp->state == LISTEN) { - tcp_arg(msg->conn->pcb.tcp, NULL); - tcp_accept(msg->conn->pcb.tcp, NULL); - tcp_close(msg->conn->pcb.tcp); - } else { - tcp_arg(msg->conn->pcb.tcp, NULL); - tcp_sent(msg->conn->pcb.tcp, NULL); - tcp_recv(msg->conn->pcb.tcp, NULL); - tcp_poll(msg->conn->pcb.tcp, NULL, 0); - tcp_err(msg->conn->pcb.tcp, NULL); - if (tcp_close(msg->conn->pcb.tcp) != ERR_OK) { - tcp_abort(msg->conn->pcb.tcp); - } - } + msg->conn->state = NETCONN_CLOSE; + do_close_internal(msg->conn); break; #endif /* LWIP_TCP */ default: @@ -483,7 +515,14 @@ do_delconn(struct api_msg_msg *msg) (*msg->conn->callback)(msg->conn, NETCONN_EVT_SENDPLUS, 0); } - if (msg->conn->mbox != SYS_MBOX_NULL) { + if ((msg->conn->mbox != SYS_MBOX_NULL) +#if LWIP_TCP + /* for tcp netconns, do_close_internal ACKs the message */ + && (NETCONNTYPE_GROUP(msg->conn->type) != NETCONN_TCP)) +#else + ) +#endif + { TCPIP_APIMSG_ACK(msg); } } @@ -846,17 +885,16 @@ void do_close(struct api_msg_msg *msg) { #if LWIP_TCP - if (msg->conn->pcb.tcp != NULL) { - if (msg->conn->type == NETCONN_TCP) { - if (msg->conn->pcb.tcp->state == LISTEN) { - msg->conn->err = tcp_close(msg->conn->pcb.tcp); - } else if (msg->conn->pcb.tcp->state == CLOSE_WAIT) { - msg->conn->err = tcp_output(msg->conn->pcb.tcp); - } - } - } + if ((msg->conn->pcb.tcp != NULL) && (msg->conn->type == NETCONN_TCP)) { + msg->conn->state = NETCONN_CLOSE; + do_close_internal(msg->conn); + /* for tcp netconns, do_close_internal ACKs the message */ + } else #endif /* LWIP_TCP */ - TCPIP_APIMSG_ACK(msg); + { + msg->conn->err = ERR_VAL; + TCPIP_APIMSG_ACK(msg); + } } #if LWIP_IGMP diff --git a/src/include/lwip/api.h b/src/include/lwip/api.h index 7e61c227..6f3b585f 100644 --- a/src/include/lwip/api.h +++ b/src/include/lwip/api.h @@ -112,7 +112,6 @@ struct netconn { sys_mbox_t mbox; sys_mbox_t recvmbox; sys_mbox_t acceptmbox; - sys_sem_t sem; int socket; #if LWIP_SO_RCVTIMEO int recv_timeout;