You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by ma...@apache.org on 2022/02/11 09:56:52 UTC

[incubator-nuttx] branch master updated: net/tcp/monitor: do not migrate the state to close

This is an automated email from the ASF dual-hosted git repository.

masayuki pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git


The following commit(s) were added to refs/heads/master by this push:
     new e749f6c  net/tcp/monitor: do not migrate the state to close
e749f6c is described below

commit e749f6ca7e6aea06463500ad9367f3e87fd61a44
Author: chao.an <an...@xiaomi.com>
AuthorDate: Wed Oct 20 18:19:27 2021 +0800

    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 <an...@xiaomi.com>
---
 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 d6384a1..8858c66 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 9080f28..53b5316 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 7db3278..87dab15 100644
--- a/net/tcp/tcp.h
+++ b/net/tcp/tcp.h
@@ -653,29 +653,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
  *
  * Description:
diff --git a/net/tcp/tcp_monitor.c b/net/tcp/tcp_monitor.c
index 88965a5..2f0de4c 100644
--- a/net/tcp/tcp_monitor.c
+++ b/net/tcp/tcp_monitor.c
@@ -340,81 +340,6 @@ 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)
-{
-  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
  *
  * Description: