Skip to content

Commit

Permalink
fixes #1543 Deadlock in nng_close(socket)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@gdamore
gdamore committed Feb 25, 2024
1 parent 08e55b9 commit 8e62028
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 34 deletions.
31 changes: 14 additions & 17 deletions src/platform/windows/win_ipcconn.c
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//
// Copyright 2020 Staysail Systems, Inc. <[email protected]>
// Copyright 2024 Staysail Systems, Inc. <[email protected]>
// Copyright 2018 Capitar IT Group BV <[email protected]>
// Copyright 2019 Devolutions <[email protected]>
//
Expand All @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -95,7 +95,7 @@ ipc_recv_start(ipc_conn *c)
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) {
Expand All @@ -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)
{
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -221,7 +223,7 @@ ipc_send_start(ipc_conn *c)
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) {
Expand Down Expand Up @@ -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.
Expand Down
30 changes: 13 additions & 17 deletions src/platform/windows/win_tcpconn.c
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//
// Copyright 2019 Staysail Systems, Inc. <[email protected]>
// Copyright 2024 Staysail Systems, Inc. <[email protected]>
// Copyright 2018 Capitar IT Group BV <[email protected]>
// Copyright 2018 Devolutions <[email protected]>
//
Expand All @@ -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) {
Expand Down Expand Up @@ -68,7 +68,7 @@ tcp_recv_start(nni_tcp_conn *c)
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) {
Expand All @@ -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);
}

Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
}
Expand Down

0 comments on commit 8e62028

Please sign in to comment.