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 <anchao@xiaomi.com>
This commit is contained in:
chao.an 2021-10-20 18:19:27 +08:00 committed by Masayuki Ishikawa
parent 3fe9c9523c
commit e749f6ca7e
4 changed files with 2 additions and 151 deletions

View File

@ -1696,10 +1696,6 @@ int inet_close(FAR struct socket *psock)
/* No.. Just decrement the reference count */ /* No.. Just decrement the reference count */
conn->crefs--; conn->crefs--;
/* Stop monitor for this socket only */
tcp_close_monitor(psock);
} }
#else #else
nwarn("WARNING: SOCK_STREAM support is not available in this " nwarn("WARNING: SOCK_STREAM support is not available in this "

View File

@ -61,11 +61,6 @@
int psock_dup2(FAR struct socket *psock1, FAR struct socket *psock2) 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 */ /* Parts of this operation need to be atomic */
net_lock(); 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 != NULL);
psock2->s_sockif->si_addref(psock2); 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(); net_unlock();
return ret;
return OK;
} }

View File

@ -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); 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 * Name: tcp_lost_connection
* *

View File

@ -339,81 +339,6 @@ void tcp_stop_monitor(FAR struct tcp_conn_s *conn, uint16_t flags)
tcp_shutdown_monitor(conn, 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 * Name: tcp_lost_connection
* *