From 1eaf9e86a8f54d77d6f392829d1b859c94965329 Mon Sep 17 00:00:00 2001 From: Garrett D'Amore Date: Sun, 5 Jan 2020 11:16:03 -0800 Subject: fixes #1112 POSIX pollq finalizers could be simpler We reap the connections when closing, to ensure that the clean up is done outside the pollq thread. This also reduces pressure on the pollq, we think. But more importantly it eliminates some complex code that was meant to avoid deadlocks, but ultimately created other use-after-free challenges. This work is an enabler for further simplifications in the aio/task logic. While here we converted some potentially racy locking of the dialers and reference counts to simpler lock-free reference counting. --- src/platform/posix/posix_ipcconn.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) (limited to 'src/platform/posix/posix_ipcconn.c') diff --git a/src/platform/posix/posix_ipcconn.c b/src/platform/posix/posix_ipcconn.c index 2b078aa5..e4d783f3 100644 --- a/src/platform/posix/posix_ipcconn.c +++ b/src/platform/posix/posix_ipcconn.c @@ -1,5 +1,5 @@ // -// Copyright 2019 Staysail Systems, Inc. +// Copyright 2020 Staysail Systems, Inc. // Copyright 2018 Capitar IT Group BV // Copyright 2019 Devolutions // @@ -200,7 +200,9 @@ ipc_close(void *arg) nni_aio_list_remove(aio); nni_aio_finish_error(aio, NNG_ECLOSED); } - nni_posix_pfd_close(c->pfd); + if (c->pfd != NULL) { + nni_posix_pfd_close(c->pfd); + } } nni_mtx_unlock(&c->mtx); } @@ -483,14 +485,13 @@ nni_posix_ipc_start(nni_ipc_conn *c) } static void -ipc_free(void *arg) +ipc_reap(void *arg) { ipc_conn *c = arg; ipc_close(c); - nni_posix_pfd_fini(c->pfd); - nni_mtx_lock(&c->mtx); // not strictly needed, but shut up TSAN - c->pfd = NULL; - nni_mtx_unlock(&c->mtx); + if (c->pfd != NULL) { + nni_posix_pfd_fini(c->pfd); + } nni_mtx_fini(&c->mtx); if (c->dialer != NULL) { @@ -500,6 +501,13 @@ ipc_free(void *arg) NNI_FREE_STRUCT(c); } +static void +ipc_free(void *arg) +{ + ipc_conn *c = arg; + nni_reap(&c->reap, ipc_reap, c); +} + static const nni_option ipc_options[] = { { .o_name = NNG_OPT_LOCADDR, @@ -545,7 +553,7 @@ ipc_setx(void *arg, const char *name, const void *val, size_t sz, nni_type t) } int -nni_posix_ipc_init(nni_ipc_conn **cp, nni_posix_pfd *pfd) +nni_posix_ipc_alloc(nni_ipc_conn **cp, nni_ipc_dialer *d) { ipc_conn *c; @@ -554,7 +562,7 @@ nni_posix_ipc_init(nni_ipc_conn **cp, nni_posix_pfd *pfd) } c->closed = false; - c->pfd = pfd; + c->dialer = d; c->stream.s_free = ipc_free; c->stream.s_close = ipc_close; c->stream.s_send = ipc_send; @@ -569,3 +577,9 @@ nni_posix_ipc_init(nni_ipc_conn **cp, nni_posix_pfd *pfd) *cp = c; return (0); } + +void +nni_posix_ipc_init(nni_ipc_conn *c, nni_posix_pfd *pfd) +{ + c->pfd = pfd; +} -- cgit v1.2.3-70-g09d2