Skip to content

Commit

Permalink
Fix zap_sock rejected endpoint leak
Browse files Browse the repository at this point in the history
The incoming-connection-request endpoint that got rejected by the
application did not yet have a thread to process its events. As a
result, when the remote peer disconnected after receiving REJECTED
message, the passive side would not get EPOLLHUP event (which will drop
corresponding references and clean up endpoint resources) because the
endpoint was not in any thread's epoll. The rejected endpoint was left
dangling, leaking endpoint resources including socket file descriptor.

This patch fixes the issue by dropping the corresponding endpoint
references to clean up rejected endpoint resources. The rejected
endpoint does not need to get a zap thread assigned to handle this.

NOTE:
This bug is introduced by Rail implementation. The RAIL implementation
modified `zap_sock` to use the thread of the listening endpoint to
process connection request instead of assigning it a new thread right
away. The behavior BEFORE Rail was to assign a thread to the incoming
endpoint right away. This bug does not affect OVIS-4.4.x.
  • Loading branch information
narategithub authored and tom95858 committed Jan 19, 2024
1 parent 1e39a78 commit a2bdf82
Showing 1 changed file with 8 additions and 7 deletions.
15 changes: 8 additions & 7 deletions lib/src/zap/sock/zap_sock.c
Original file line number Diff line number Diff line change
Expand Up @@ -1992,7 +1992,8 @@ static void z_sock_destroy(zap_ep_t ep)

DEBUG_LOG(sep, "%ld z_sock_destroy(%p)\n", GETTID(), sep);

zap_io_thread_ep_remove(ep);
if (ep->thread)
zap_io_thread_ep_remove(ep);

while (!TAILQ_EMPTY(&sep->sq)) {
wr = TAILQ_FIRST(&sep->sq);
Expand Down Expand Up @@ -2061,14 +2062,14 @@ static zap_err_t z_sock_reject(zap_ep_t ep, char *data, size_t data_len)

pthread_mutex_lock(&sep->ep.lock);
zerr = __sock_send(sep, SOCK_MSG_REJECTED, data, data_len);
if (zerr)
goto err;
pthread_mutex_unlock(&sep->ep.lock);
return ZAP_ERR_OK;
err:

/* move to error state before we terminate it */
sep->ep.state = ZAP_EP_ERROR;
shutdown(sep->sock, SHUT_RDWR);

pthread_mutex_unlock(&sep->ep.lock);
ref_put(&ep->ref, "accept/connect"); /* from __z_sock_conn_request() */
zap_free(ep);
/* The caller never touched ep after reject */
return zerr;
}

Expand Down

0 comments on commit a2bdf82

Please sign in to comment.