net/inet/inet_close.c: Fixes two problems, both noted by Bernd Walter:

1) The change of commit ed9fe70024 left some dangling logic and incorrect, confusing comments.  Prior to that commit the 'pstate' variable was non-NULL only when doing a lingering close.  Comments to this effect as well as tests of pstate should also have been updated.  These are confusing and inappropriate, but do not lead to incorrect behavior.

2) Eliminate an incomplete test when a disconnection event. When a disconnection event occurs, the close logic MUST always terminate the wait.  The conditional test was not incorrect, however, it lacked 'else' logic and would simply ignore that disconnection event in some cases.  That is bad because there may not be another disconnection event and that can lead to hangs (or at least very, very long delays).
This commit is contained in:
Gregory Nutt 2019-09-25 07:24:20 -06:00
parent 0008ff33b3
commit f3ab9abe51
1 changed files with 23 additions and 25 deletions

View File

@ -108,7 +108,7 @@ static uint16_t tcp_close_eventhandler(FAR struct net_driver_s *dev,
FAR struct tcp_close_s *pstate = (FAR struct tcp_close_s *)pvpriv;
FAR struct tcp_conn_s *conn = (FAR struct tcp_conn_s *)pvconn;
DEBUGASSERT(conn != NULL);
DEBUGASSERT(pstate != NULL && conn != NULL);
ninfo("conn: %p flags: %04x\n", conn, flags);
@ -121,35 +121,33 @@ static uint16_t tcp_close_eventhandler(FAR struct net_driver_s *dev,
if ((flags & TCP_DISCONN_EVENTS) != 0)
{
/* The disconnection is complete */
/* pstate non-NULL means that we are performing a LINGERing close. */
if (pstate != NULL)
{
if (conn->tcpstateflags == TCP_CLOSED)
{
/* Wake up the waiting thread with a successful result */
pstate->cl_result = OK;
goto end_wait;
}
}
/* Otherwise, nothing is waiting on the close event and we can perform
* the completion actions here.
/* The disconnection is complete. Wake up the waiting thread with an
* appropriate result:
*
* * TCP_CLOSE indicates normal successful closure. TCP_ABORT is less
* likely but still means that the socket was closed, albeit
* abnormally. We will call either a success.
*
* * NETDEV_DOWN would indicate that the network went down before the
* close completed. TCP_TIMEDOUT is not expected in this context.
* Non-standard return values are used to indicate these anomalous
* cases.
*/
if ((flags & NETDEV_DOWN) != 0)
{
pstate->cl_result = -ENODEV;
}
else if ((flags & TCP_TIMEDOUT) != 0)
{
pstate->cl_result = -ETIMEDOUT;
}
else
{
/* Free connection resources */
tcp_free(conn);
/* Stop further callbacks */
flags = 0;
pstate->cl_result = OK;
}
goto end_wait;
}
#ifdef CONFIG_NET_TCP_WRITE_BUFFERS