From 8e62028a0db24364ea218007811e58ea11d0b64f Mon Sep 17 00:00:00 2001 From: Garrett D'Amore Date: Sun, 25 Feb 2024 14:56:24 -0800 Subject: fixes #1543 Deadlock in nng_close(socket) This looks like a possible problem that may be windows specific involving the flow for IO completion ports. This simplifies the logic a little bit, and should ensure that canceled requests on pipes do not restart. --- src/platform/windows/win_ipcconn.c | 31 ++++++++++++++----------------- src/platform/windows/win_tcpconn.c | 30 +++++++++++++----------------- 2 files changed, 27 insertions(+), 34 deletions(-) (limited to 'src') diff --git a/src/platform/windows/win_ipcconn.c b/src/platform/windows/win_ipcconn.c index 135a961e..abe92604 100644 --- a/src/platform/windows/win_ipcconn.c +++ b/src/platform/windows/win_ipcconn.c @@ -1,5 +1,5 @@ // -// Copyright 2020 Staysail Systems, Inc. +// Copyright 2024 Staysail Systems, Inc. // Copyright 2018 Capitar IT Group BV // Copyright 2019 Devolutions // @@ -25,7 +25,7 @@ typedef struct ipc_conn { nni_win_io conn_io; nni_list recv_aios; nni_list send_aios; - nni_aio * conn_aio; + nni_aio *conn_aio; nng_sockaddr sa; bool dialer; int recv_rv; @@ -44,7 +44,7 @@ ipc_recv_start(ipc_conn *c) unsigned idx; unsigned naiov; nni_iov *aiov; - void * buf; + void *buf; DWORD len; int rv; @@ -95,7 +95,7 @@ again: static void ipc_recv_cb(nni_win_io *io, int rv, size_t num) { - nni_aio * aio; + nni_aio *aio; ipc_conn *c = io->ptr; nni_mtx_lock(&c->mtx); if ((aio = nni_list_first(&c->recv_aios)) == NULL) { @@ -107,19 +107,21 @@ ipc_recv_cb(nni_win_io *io, int rv, size_t num) rv = c->recv_rv; c->recv_rv = 0; } + if ((rv == 0) && (num == 0)) { + // A zero byte receive is a remote close from the peer. + rv = NNG_ECONNSHUT; + } nni_aio_list_remove(aio); - ipc_recv_start(c); if (c->closed) { nni_cv_wake(&c->cv); + } else { + ipc_recv_start(c); } nni_mtx_unlock(&c->mtx); - if ((rv == 0) && (num == 0)) { - // A zero byte receive is a remote close from the peer. - rv = NNG_ECONNSHUT; - } nni_aio_finish_sync(aio, rv, num); } + static void ipc_recv_cancel(nni_aio *aio, void *arg, int rv) { @@ -170,7 +172,7 @@ ipc_send_start(ipc_conn *c) unsigned idx; unsigned naiov; nni_iov *aiov; - void * buf; + void *buf; DWORD len; int rv; @@ -221,7 +223,7 @@ again: static void ipc_send_cb(nni_win_io *io, int rv, size_t num) { - nni_aio * aio; + nni_aio *aio; ipc_conn *c = io->ptr; nni_mtx_lock(&c->mtx); if ((aio = nni_list_first(&c->send_aios)) == NULL) { @@ -293,12 +295,7 @@ ipc_close(void *arg) nni_mtx_lock(&c->mtx); if (!c->closed) { c->closed = true; - if (!nni_list_empty(&c->recv_aios)) { - CancelIoEx(c->f, &c->recv_io.olpd); - } - if (!nni_list_empty(&c->send_aios)) { - CancelIoEx(c->f, &c->send_io.olpd); - } + CancelIoEx(c->f, NULL); // cancel all requests if (c->f != INVALID_HANDLE_VALUE) { // NB: closing the pipe is dangerous at this point. diff --git a/src/platform/windows/win_tcpconn.c b/src/platform/windows/win_tcpconn.c index cc906803..fad7c495 100644 --- a/src/platform/windows/win_tcpconn.c +++ b/src/platform/windows/win_tcpconn.c @@ -1,5 +1,5 @@ // -// Copyright 2019 Staysail Systems, Inc. +// Copyright 2024 Staysail Systems, Inc. // Copyright 2018 Capitar IT Group BV // Copyright 2018 Devolutions // @@ -26,7 +26,7 @@ tcp_recv_start(nni_tcp_conn *c) unsigned i; unsigned naiov; nni_iov *aiov; - WSABUF * iov; + WSABUF *iov; if (c->closed) { while ((aio = nni_list_first(&c->recv_aios)) != NULL) { @@ -68,7 +68,7 @@ again: static void tcp_recv_cb(nni_win_io *io, int rv, size_t num) { - nni_aio * aio; + nni_aio *aio; nni_tcp_conn *c = io->ptr; nni_mtx_lock(&c->mtx); if ((aio = nni_list_first(&c->recv_aios)) == NULL) { @@ -80,17 +80,18 @@ tcp_recv_cb(nni_win_io *io, int rv, size_t num) rv = c->recv_rv; c->recv_rv = 0; } + if ((rv == 0) && (num == 0)) { + // A zero byte receive is a remote close from the peer. + rv = NNG_ECONNSHUT; + } nni_aio_list_remove(aio); - tcp_recv_start(c); if (c->closed) { nni_cv_wake(&c->cv); + } else { + tcp_recv_start(c); } nni_mtx_unlock(&c->mtx); - if ((rv == 0) && (num == 0)) { - // A zero byte receive is a remote close from the peer. - rv = NNG_ECONNSHUT; - } nni_aio_finish_sync(aio, rv, num); } @@ -114,7 +115,7 @@ static void tcp_recv(void *arg, nni_aio *aio) { nni_tcp_conn *c = arg; - int rv; + int rv; if (nni_aio_begin(aio) != 0) { return; @@ -146,7 +147,7 @@ tcp_send_start(nni_tcp_conn *c) unsigned i; unsigned naiov; nni_iov *aiov; - WSABUF * iov; + WSABUF *iov; if (c->closed) { while ((aio = nni_list_first(&c->send_aios)) != NULL) { @@ -204,7 +205,7 @@ tcp_send_cancel(nni_aio *aio, void *arg, int rv) static void tcp_send_cb(nni_win_io *io, int rv, size_t num) { - nni_aio * aio; + nni_aio *aio; nni_tcp_conn *c = io->ptr; nni_mtx_lock(&c->mtx); if ((aio = nni_list_first(&c->send_aios)) == NULL) { @@ -260,13 +261,8 @@ tcp_close(void *arg) nni_mtx_lock(&c->mtx); if (!c->closed) { c->closed = true; - if (!nni_list_empty(&c->recv_aios)) { - CancelIoEx((HANDLE) c->s, &c->recv_io.olpd); - } - if (!nni_list_empty(&c->send_aios)) { - CancelIoEx((HANDLE) c->s, &c->send_io.olpd); - } if (c->s != INVALID_SOCKET) { + CancelIoEx((HANDLE) c->s, NULL); // cancel everything shutdown(c->s, SD_BOTH); } } -- cgit v1.2.3-70-g09d2