From b77438a77c6560af0a9b5b38c3ddd10a5db9613c Mon Sep 17 00:00:00 2001 From: Garrett D'Amore Date: Mon, 12 Aug 2024 00:09:24 -0700 Subject: [PATCH] fixups for TCP (testing needed) as well, and limit the close wait --- src/platform/windows/win_ipcconn.c | 9 +++++++- src/platform/windows/win_tcp.h | 8 ++++--- src/platform/windows/win_tcpconn.c | 37 ++++++++++++++++++++++++------ 3 files changed, 43 insertions(+), 11 deletions(-) diff --git a/src/platform/windows/win_ipcconn.c b/src/platform/windows/win_ipcconn.c index ee743bcf8..ef6ceb66d 100644 --- a/src/platform/windows/win_ipcconn.c +++ b/src/platform/windows/win_ipcconn.c @@ -283,6 +283,7 @@ static void ipc_close(void *arg) { ipc_conn *c = arg; + nni_time now; nni_mtx_lock(&c->mtx); if (!c->closed) { HANDLE f = c->f; @@ -297,7 +298,13 @@ ipc_close(void *arg) CloseHandle(f); } } - while (c->recving || c->sending) { + now = nni_clock(); + // wait up to a maximum of 10 seconds before assuming something is + // badly amiss. from what we can tell, this doesn't happen, and we do + // see the timer expire properly, but this safeguard can prevent a + // hang. + while ((c->recving || c->sending) && + ((nni_clock() - now) < (NNI_SECOND * 10))) { nni_mtx_unlock(&c->mtx); nni_msleep(1); nni_mtx_lock(&c->mtx); diff --git a/src/platform/windows/win_tcp.h b/src/platform/windows/win_tcp.h index e54f853c3..d151b9a3d 100644 --- a/src/platform/windows/win_tcp.h +++ b/src/platform/windows/win_tcp.h @@ -1,5 +1,5 @@ // -// Copyright 2019 Staysail Systems, Inc. +// Copyright 2024 Staysail Systems, Inc. // Copyright 2018 Capitar IT Group BV // Copyright 2019 Devolutions // @@ -24,16 +24,18 @@ struct nni_tcp_conn { nni_win_io conn_io; nni_list recv_aios; nni_list send_aios; - nni_aio * conn_aio; + nni_aio *conn_aio; SOCKADDR_STORAGE sockname; SOCKADDR_STORAGE peername; - nni_tcp_dialer * dialer; + nni_tcp_dialer *dialer; nni_tcp_listener *listener; int recv_rv; int send_rv; int conn_rv; bool closed; char buf[512]; // to hold acceptex results + bool sending; + bool recving; nni_mtx mtx; nni_cv cv; }; diff --git a/src/platform/windows/win_tcpconn.c b/src/platform/windows/win_tcpconn.c index 246055058..0e74ccfd8 100644 --- a/src/platform/windows/win_tcpconn.c +++ b/src/platform/windows/win_tcpconn.c @@ -48,13 +48,15 @@ tcp_recv_start(nni_tcp_conn *c) } } - flags = 0; - rv = WSARecv( + c->recving = true; + flags = 0; + rv = WSARecv( c->s, iov, niov, &nrecv, &flags, &c->recv_io.olpd, NULL); if ((rv == SOCKET_ERROR) && ((rv = GetLastError()) != ERROR_IO_PENDING)) { // Synchronous error. + c->recving = false; nni_aio_list_remove(aio); nni_aio_finish_error(aio, nni_win_error(rv)); } else { @@ -76,7 +78,6 @@ tcp_recv_cb(nni_win_io *io, int rv, size_t num) nni_mtx_lock(&c->mtx); aio = nni_list_first(&c->recv_aios); NNI_ASSERT(aio != NULL); - nni_aio_list_remove(aio); if (c->recv_rv != 0) { rv = c->recv_rv; @@ -86,6 +87,8 @@ tcp_recv_cb(nni_win_io *io, int rv, size_t num) // A zero byte receive is a remote close from the peer. rv = NNG_ECONNSHUT; } + c->recving = false; + nni_aio_list_remove(aio); tcp_recv_start(c); nni_mtx_unlock(&c->mtx); @@ -164,11 +167,13 @@ tcp_send_start(nni_tcp_conn *c) } } + c->sending = true; rv = WSASend(c->s, iov, niov, NULL, 0, &c->send_io.olpd, NULL); if ((rv == SOCKET_ERROR) && ((rv = GetLastError()) != ERROR_IO_PENDING)) { // Synchronous failure. + c->sending = false; nni_aio_list_remove(aio); nni_aio_finish_error(aio, nni_win_error(rv)); } else { @@ -208,6 +213,7 @@ tcp_send_cb(nni_win_io *io, int rv, size_t num) aio = nni_list_first(&c->send_aios); NNI_ASSERT(aio != NULL); nni_aio_list_remove(aio); // should always be at head + c->sending = false; if (c->send_rv != 0) { rv = c->send_rv; @@ -246,14 +252,31 @@ tcp_close(void *arg) { nni_tcp_conn *c = arg; nni_mtx_lock(&c->mtx); + nni_time now; if (!c->closed) { + SOCKET s = c->s; + c->closed = true; - if (c->s != INVALID_SOCKET) { - shutdown(c->s, SD_BOTH); - closesocket(c->s); - c->s = INVALID_SOCKET; + c->s = INVALID_SOCKET; + + if (s != INVALID_SOCKET) { + CancelIoEx(s, &c->send_io.olpd); + CancelIoEx(s, &c->recv_io.olpd); + shutdown(s, SD_BOTH); + closesocket(s); } } + now = nni_clock(); + // wait up to a maximum of 10 seconds before assuming something is + // badly amiss. from what we can tell, this doesn't happen, and we do + // see the timer expire properly, but this safeguard can prevent a + // hang. + while ((c->recving || c->sending) && + ((nni_clock() - now) < (NNI_SECOND * 10))) { + nni_mtx_unlock(&c->mtx); + nni_msleep(1); + nni_mtx_lock(&c->mtx); + } nni_mtx_unlock(&c->mtx); }