net_startmonitor.c always returned zero. In the case where a socket has already been closed, it correctly handled the disconnetion event but still returned OK. Returning OK causes the callers of net_startmonitor to assume that the connection was okay, undoing the good things that net_startmonitor did and causing the socket to be marked as connected. This behavior was noted by Pelle Windestam.

This commit is contained in:
Gregory Nutt 2015-05-28 08:23:51 -06:00
parent 0e2b6929b4
commit 01d176af76
6 changed files with 74 additions and 24 deletions

View File

@ -299,7 +299,19 @@ int accept(int sockfd, FAR struct sockaddr *addr, FAR socklen_t *addrlen)
* socket
*/
net_startmonitor(pnewsock);
ret = net_startmonitor(pnewsock);
if (ret < 0)
{
/* net_startmonitor() can only fail on certain race conditions
* where the connection was lost just before this function was
* called. Undo everything we have done and return a failure.
*/
net_unlock(state);
err = -ret;
goto errout_after_accept;
}
net_unlock(state);
}
#endif /* CONFIG_NET_TCP */
@ -310,6 +322,9 @@ int accept(int sockfd, FAR struct sockaddr *addr, FAR socklen_t *addrlen)
pnewsock->s_flags &= ~_SF_CLOSED;
return newfd;
errout_after_accept:
psock_close(pnewsock);
errout_with_socket:
sockfd_release(newfd);

View File

@ -81,8 +81,8 @@ struct tcp_connect_s
#ifdef CONFIG_NET_TCP
static inline int psock_setup_callbacks(FAR struct socket *psock,
FAR struct tcp_connect_s *pstate);
static inline void psock_teardown_callbacks(FAR struct tcp_connect_s *pstate,
int status);
static void psock_teardown_callbacks(FAR struct tcp_connect_s *pstate,
int status);
static uint16_t psock_connect_interrupt(FAR struct net_driver_s *dev,
FAR void *pvconn, FAR void *pvpriv,
uint16_t flags);
@ -124,9 +124,18 @@ static inline int psock_setup_callbacks(FAR struct socket *psock,
/* Set up the connection event monitor */
net_startmonitor(psock);
ret = OK;
ret = net_startmonitor(psock);
if (ret < 0)
{
/* net_startmonitor() can only fail on certain race conditions
* where the connection was lost just before this function was
* called. Undo everything we have done and return a failure.
*/
psock_teardown_callbacks(pstate, ret);
}
}
return ret;
}
#endif /* CONFIG_NET_TCP */
@ -136,15 +145,14 @@ static inline int psock_setup_callbacks(FAR struct socket *psock,
****************************************************************************/
#ifdef CONFIG_NET_TCP
static inline void psock_teardown_callbacks(FAR struct tcp_connect_s *pstate,
int status)
static void psock_teardown_callbacks(FAR struct tcp_connect_s *pstate,
int status)
{
FAR struct tcp_conn_s *conn = pstate->tc_conn;
/* Make sure that no further interrupts are processed */
tcp_callback_free(conn, pstate->tc_cb);
pstate->tc_cb = NULL;
/* If we successfully connected, we will continue to monitor the connection

View File

@ -523,8 +523,9 @@ int psock_close(FAR struct socket *psock)
goto errout;
}
/* We perform the uIP close operation only if this is the last count on the socket.
* (actually, I think the socket crefs only takes the values 0 and 1 right now).
/* We perform the uIP close operation only if this is the last count on
* the socket. (actually, I think the socket crefs only takes the values
* 0 and 1 right now).
*/
if (psock->s_crefs <= 1)

View File

@ -121,7 +121,13 @@ static void connection_event(FAR struct tcp_conn_s *conn, uint16_t flags)
* psock - The socket of interest
*
* Returned Value:
* For now, this function always returns OK.
* On success, net_startmonitor returns OK; On any failure,
* net_startmonitor will return a negated errno value. The only failure
* that can occur is if the socket has already been closed and, in this
* case, -ENOTCONN is returned.
*
* Assumptions:
* The caller holds the network lock.
*
****************************************************************************/
@ -131,22 +137,35 @@ int net_startmonitor(FAR struct socket *psock)
DEBUGASSERT(psock && conn);
/* Set up to receive callbacks on connection-related events */
conn->connection_private = (void*)psock;
conn->connection_event = connection_event;
/* Check if the connection has already been closed before any callbacks have
* been registered. (Maybe the connection is lost before accept has registered
* the monitoring callback.)
/* Check if the connection has already been closed before any callbacks
* have been registered. (Maybe the connection is lost before accept has
* registered the monitoring callback.)
*/
if (!(conn->tcpstateflags == TCP_ESTABLISHED ||
conn->tcpstateflags == TCP_SYN_RCVD))
{
/* Invoke the TCP_CLOSE connection event now */
connection_event(conn, TCP_CLOSE);
/* Make sure that the monitor is stopped */
conn->connection_private = NULL;
conn->connection_event = NULL;
/* And return -ENOTCONN to indicate the the monitor was not started
* because the socket was already disconnected.
*/
return -ENOTCONN;
}
/* Set up to receive callbacks on connection-related events */
conn->connection_private = (void*)psock;
conn->connection_event = connection_event;
return OK;
}

View File

@ -231,7 +231,13 @@ FAR struct socket *sockfd_socket(int sockfd);
* psock - The socket of interest
*
* Returned Value:
* For now, this function always returns OK.
* On success, net_startmonitor returns OK; On any failure,
* net_startmonitor will return a negated errno value. The only failure
* that can occur is if the socket has already been closed and, in this
* case, -ENOTCONN is returned.
*
* Assumptions:
* The caller holds the network lock.
*
****************************************************************************/

View File

@ -219,7 +219,8 @@ static int accept_interrupt(FAR struct tcp_conn_s *listener,
* Parameters:
* psock The listening TCP socket structure
* addr Receives the address of the connecting client
* addrlen Input: allocated size of 'addr', Return: returned size of 'addr'
* addrlen Input: allocated size of 'addr', Return: returned size of
* 'addr'
* newconn The new, accepted TCP connection structure
*
* Returned Value:
@ -259,9 +260,9 @@ int psock_tcp_accept(FAR struct socket *psock, FAR struct sockaddr *addr,
}
/* In general, this uIP-based implementation will not support non-blocking
* socket operations... except in a few cases: Here for TCP accept with backlog
* enabled. If this socket is configured as non-blocking then return EAGAIN
* if there is no pending connection in the backlog.
* socket operations... except in a few cases: Here for TCP accept with
* backlog enabled. If this socket is configured as non-blocking then
* return EAGAIN if there is no pending connection in the backlog.
*/
else if (_SS_ISNONBLOCK(psock->s_flags))