From e749f6ca7e6aea06463500ad9367f3e87fd61a44 Mon Sep 17 00:00:00 2001 From: "chao.an" Date: Wed, 20 Oct 2021 18:19:27 +0800 Subject: [PATCH] net/tcp/monitor: do not migrate the state to close 1. remove the unnecessary interfaces tcp_close_monitor() socket flags(s_flags) is a global state for net connection remove the incorrect update for stop monitor 2. do not start the tcp monitor from duplicated psock the tcp monitor has already registered in connect callback ------------------------------------------------------------ This patch also fix the telnet issue reported by: https://github.com/apache/incubator-nuttx/pull/5434#issuecomment-1035600651 the orignal session fd is closed after dup, the connect state has incorrectly migrated to close: drivers/net/telnet.c: 977 static int telnet_session(FAR struct telnet_session_s *session) ... 1031 ret = psock_dup2(psock, &priv->td_psock); ... 1082 nx_close(session->ts_sd); Signed-off-by: chao.an --- net/inet/inet_sockif.c | 4 --- net/socket/net_dup2.c | 51 ++-------------------------- net/tcp/tcp.h | 23 ------------- net/tcp/tcp_monitor.c | 75 ------------------------------------------ 4 files changed, 2 insertions(+), 151 deletions(-) diff --git a/net/inet/inet_sockif.c b/net/inet/inet_sockif.c index d6384a1163..8858c6603b 100644 --- a/net/inet/inet_sockif.c +++ b/net/inet/inet_sockif.c @@ -1696,10 +1696,6 @@ int inet_close(FAR struct socket *psock) /* No.. Just decrement the reference count */ conn->crefs--; - - /* Stop monitor for this socket only */ - - tcp_close_monitor(psock); } #else nwarn("WARNING: SOCK_STREAM support is not available in this " diff --git a/net/socket/net_dup2.c b/net/socket/net_dup2.c index 9080f28692..53b5316a7f 100644 --- a/net/socket/net_dup2.c +++ b/net/socket/net_dup2.c @@ -61,11 +61,6 @@ int psock_dup2(FAR struct socket *psock1, FAR struct socket *psock2) { -#ifdef NET_TCP_HAVE_STACK - FAR struct tcp_conn_s *conn; -#endif - int ret = OK; - /* Parts of this operation need to be atomic */ net_lock(); @@ -87,49 +82,7 @@ int psock_dup2(FAR struct socket *psock1, FAR struct socket *psock2) psock2->s_sockif->si_addref != NULL); psock2->s_sockif->si_addref(psock2); -#ifdef NET_TCP_HAVE_STACK - /* For connected socket types, it is necessary to also start the network - * monitor so that the newly cloned socket can receive a notification if - * the network connection is lost. - */ - - conn = (FAR struct tcp_conn_s *)psock2->s_conn; - - if ((psock2->s_domain == PF_INET || - psock2->s_domain == PF_INET6) && - psock2->s_type == SOCK_STREAM && conn && - (conn->tcpstateflags == TCP_ESTABLISHED || - conn->tcpstateflags == TCP_SYN_RCVD)) - { - ret = tcp_start_monitor(psock2); - - /* On failure, back out the reference count on the TCP connection - * structure. tcp_start_monitor() will fail only in the race condition - * where the TCP connection has been lost. - */ - - if (ret < 0) - { - /* There should be at least two reference counts on the connection - * structure: At least one from the original socket and the one - * from above where we incremented the reference count. - * inet_close() will handle all cases. - * - * NOTE: As a side-effect, inet_close()will also call - * tcp_stop_monitor() which could inform the loss of connection to - * all open sockets on the connection structure if the reference - * count decrements to zero. - */ - - inet_close(psock2); - - /* The socket will not persist... reset it */ - - memset(psock2, 0, sizeof(*psock2)); - } - } -#endif - net_unlock(); - return ret; + + return OK; } diff --git a/net/tcp/tcp.h b/net/tcp/tcp.h index 7db32786ca..87dab15f8a 100644 --- a/net/tcp/tcp.h +++ b/net/tcp/tcp.h @@ -652,29 +652,6 @@ int tcp_start_monitor(FAR struct socket *psock); void tcp_stop_monitor(FAR struct tcp_conn_s *conn, uint16_t flags); -/**************************************************************************** - * Name: tcp_close_monitor - * - * Description: - * One socket in a group of dup'ed sockets has been closed. We need to - * selectively terminate just those things that are waiting of events - * from this specific socket. And also recover any resources that are - * committed to monitoring this socket. - * - * Input Parameters: - * psock - The TCP socket structure that is closed - * - * Returned Value: - * None - * - * Assumptions: - * The caller holds the network lock (if not, it will be locked momentarily - * by this function). - * - ****************************************************************************/ - -void tcp_close_monitor(FAR struct socket *psock); - /**************************************************************************** * Name: tcp_lost_connection * diff --git a/net/tcp/tcp_monitor.c b/net/tcp/tcp_monitor.c index 88965a551e..2f0de4ce31 100644 --- a/net/tcp/tcp_monitor.c +++ b/net/tcp/tcp_monitor.c @@ -339,81 +339,6 @@ void tcp_stop_monitor(FAR struct tcp_conn_s *conn, uint16_t flags) tcp_shutdown_monitor(conn, flags); } -/**************************************************************************** - * Name: tcp_close_monitor - * - * Description: - * One socket in a group of dup'ed sockets has been closed. We need to - * selectively terminate just those things that are waiting of events - * from this specific socket. And also recover any resources that are - * committed to monitoring this socket. - * - * Input Parameters: - * psock - The TCP socket structure that is closed - * - * Returned Value: - * None - * - * Assumptions: - * The caller holds the network lock (if not, it will be locked momentarily - * by this function). - * - ****************************************************************************/ - -void tcp_close_monitor(FAR struct socket *psock) -{ - FAR struct tcp_conn_s *conn; - FAR struct devif_callback_s *cb; - - DEBUGASSERT(psock != NULL && psock->s_conn != NULL); - conn = (FAR struct tcp_conn_s *)psock->s_conn; - - /* Find and free the the connection event callback */ - - net_lock(); - for (cb = conn->connevents; - cb != NULL && cb->priv != (FAR void *)psock; - cb = cb->nxtconn) - { - } - - if (cb != NULL) - { - devif_conn_callback_free(conn->dev, - cb, - &conn->connevents, - &conn->connevents_tail); - } - - /* Make sure that this socket is explicitly marked as closed */ - - tcp_close_connection(conn, TCP_CLOSE); - - /* Now notify any sockets waiting for events from this particular sockets. - * Other dup'ed sockets sharing the same connection must not be effected. - */ - - /* REVISIT: The following logic won't work: There is no way to compare - * psocks to check for a match. This missing logic could only be an issue - * if the same socket were being used on one thread, but then closed on - * another. Some redesign would be required to find only those event - * handlers that are waiting specifically for this socket (vs. a dup of - * this socket) - */ - -#if 0 - for (cb = conn->list; cb != NULL; cb = cb->nxtconn) - { - if (cb->event != NULL && (cb->flags & TCP_CLOSE) != 0) - { - cb->event(conn->dev, conn, cb->priv, TCP_CLOSE); - } - } -#endif - - net_unlock(); -} - /**************************************************************************** * Name: tcp_lost_connection *