Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixes #1827 Windows a deadlock on nng_close() #1828

Merged
merged 12 commits into from
May 30, 2024
28 changes: 1 addition & 27 deletions src/core/aio.c
Original file line number Diff line number Diff line change
Expand Up @@ -70,28 +70,6 @@ static int nni_aio_expire_q_cnt;
// operations from starting, without waiting for any existing one to
// complete, call nni_aio_close.

// In some places we want to check that an aio is not in use.
// Technically if these checks pass, then they should not need
// to be done with a lock, because the caller should have the only
// references to them. However, race detectors can't necessarily
// know about this semantic, and may complain about potential data
// races. To suppress false positives, define NNG_RACE_DETECTOR.
// Note that this will cause extra locks to be acquired, affecting
// performance, so don't use it in production.
#ifdef __has_feature
#if __has_feature(thread_sanitizer)
#define NNG_RACE_DETECTOR
#endif
#endif

#ifdef NNG_RACE_DETECTOR
#define aio_safe_lock(l) nni_mtx_lock(l)
#define aio_safe_unlock(l) nni_mtx_unlock(l)
#else
#define aio_safe_lock(l) ((void) 1)
#define aio_safe_unlock(l) ((void) 1)
#endif

static nni_reap_list aio_reap_list = {
.rl_offset = offsetof(nni_aio, a_reap_node),
.rl_func = (nni_cb) nni_aio_free,
Expand Down Expand Up @@ -350,8 +328,7 @@ nni_aio_begin(nni_aio *aio)
// checks may wish ignore or suppress these checks.
nni_aio_expire_q *eq = aio->a_expire_q;

aio_safe_lock(&eq->eq_mtx);

nni_mtx_lock(&eq->eq_mtx);
NNI_ASSERT(!nni_aio_list_active(aio));
NNI_ASSERT(aio->a_cancel_fn == NULL);
NNI_ASSERT(!nni_list_node_active(&aio->a_expire_node));
Expand All @@ -365,9 +342,6 @@ nni_aio_begin(nni_aio *aio)
aio->a_count = 0;
aio->a_cancel_fn = NULL;

aio_safe_unlock(&eq->eq_mtx);

nni_mtx_lock(&eq->eq_mtx);
// We should not reschedule anything at this point.
if (aio->a_stop) {
aio->a_result = NNG_ECANCELED;
Expand Down
2 changes: 1 addition & 1 deletion src/core/pipe.c
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ nni_pipe_bump_error(nni_pipe *p, int err)
{
if (p->p_dialer != NULL) {
nni_dialer_bump_error(p->p_dialer, err);
} else {
} else if (p->p_listener != NULL) {
nni_listener_bump_error(p->p_listener, err);
}
}
Expand Down
11 changes: 2 additions & 9 deletions src/core/socket.c
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ struct nni_socket {

bool s_closing; // Socket is closing
bool s_closed; // Socket closed, protected by global lock
bool s_ctxwait; // Waiting for contexts to close.

nni_mtx s_pipe_cbs_mtx;
nni_sock_pipe_cb s_pipe_cbs[NNG_PIPE_EV_NUM];
Expand Down Expand Up @@ -734,7 +733,6 @@ nni_sock_shutdown(nni_sock *sock)
// a chance to do so gracefully.

while (!nni_list_empty(&sock->s_ctxs)) {
sock->s_ctxwait = true;
nni_cv_wait(&sock->s_close_cv);
}
nni_mtx_unlock(&sock_lk);
Expand Down Expand Up @@ -798,7 +796,6 @@ nni_sock_close(nni_sock *s)

// Wait for all other references to drop. Note that we
// have a reference already (from our caller).
s->s_ctxwait = true;
while ((s->s_ref > 1) || (!nni_list_empty(&s->s_ctxs))) {
nni_cv_wait(&s->s_close_cv);
}
Expand Down Expand Up @@ -1307,9 +1304,7 @@ nni_ctx_rele(nni_ctx *ctx)
// tries to avoid ID reuse.
nni_id_remove(&ctx_ids, ctx->c_id);
nni_list_remove(&sock->s_ctxs, ctx);
if (sock->s_closed || sock->s_ctxwait) {
nni_cv_wake(&sock->s_close_cv);
}
nni_cv_wake(&sock->s_close_cv);
nni_mtx_unlock(&sock_lk);

nni_ctx_destroy(ctx);
Expand Down Expand Up @@ -1791,9 +1786,7 @@ nni_pipe_remove(nni_pipe *p)
d->d_pipe = NULL;
dialer_timer_start_locked(d); // Kick the timer to redial.
}
if (s->s_closing) {
nni_cv_wake(&s->s_cv);
}
nni_cv_wake(&s->s_cv);
nni_mtx_unlock(&s->s_mx);
}

Expand Down
4 changes: 2 additions & 2 deletions src/platform/windows/win_debug.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//
// Copyright 2020 Staysail Systems, Inc. <info@staysail.tech>
// Copyright 2024 Staysail Systems, Inc. <info@staysail.tech>
// Copyright 2018 Capitar IT Group BV <info@capitar.com>
//
// This software is supplied under the terms of the MIT License, a
Expand Down Expand Up @@ -111,7 +111,7 @@ static struct {
{ ERROR_DUP_NAME, NNG_EADDRINUSE },
{ ERROR_BROKEN_PIPE, NNG_ECLOSED },
{ ERROR_BAD_PIPE, NNG_ECLOSED },
{ ERROR_NO_DATA, NNG_ECLOSED },
{ ERROR_NO_DATA, NNG_ECONNRESET },
{ ERROR_PIPE_NOT_CONNECTED, NNG_ECLOSED },
{ ERROR_OPERATION_ABORTED, NNG_ECLOSED },
{ ERROR_SHARING_VIOLATION, NNG_EBUSY },
Expand Down
5 changes: 2 additions & 3 deletions src/platform/windows/win_impl.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//
// Copyright 2021 Staysail Systems, Inc. <info@staysail.tech>
// Copyright 2024 Staysail Systems, Inc. <info@staysail.tech>
// Copyright 2018 Capitar IT Group BV <info@capitar.com>
//
// This software is supplied under the terms of the MIT License, a
Expand Down Expand Up @@ -122,8 +122,7 @@ extern void nni_win_udp_sysfini(void);
extern int nni_win_resolv_sysinit(void);
extern void nni_win_resolv_sysfini(void);

extern int nni_win_io_init(nni_win_io *, nni_win_io_cb, void *);
extern void nni_win_io_fini(nni_win_io *);
extern void nni_win_io_init(nni_win_io *, nni_win_io_cb, void *);

extern int nni_win_io_register(HANDLE);

Expand Down
21 changes: 4 additions & 17 deletions src/platform/windows/win_io.c
Original file line number Diff line number Diff line change
Expand Up @@ -60,27 +60,14 @@ nni_win_io_register(HANDLE h)
return (0);
}

int
void
nni_win_io_init(nni_win_io *io, nni_win_io_cb cb, void *ptr)
{
ZeroMemory(&io->olpd, sizeof(io->olpd));

io->cb = cb;
io->ptr = ptr;
io->aio = NULL;
io->olpd.hEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
if (io->olpd.hEvent == NULL) {
return (nni_win_error(GetLastError()));
}
return (0);
}

void
nni_win_io_fini(nni_win_io *io)
{
if (io->olpd.hEvent != NULL) {
CloseHandle((HANDLE) io->olpd.hEvent);
}
io->cb = cb;
io->ptr = ptr;
io->aio = NULL;
}

int
Expand Down
Loading
Loading