From a9633313ec8e578c805cd53b37ba3360d83157bc Mon Sep 17 00:00:00 2001 From: Garrett D'Amore Date: Tue, 15 Aug 2017 21:59:55 -0700 Subject: Provide versions of mutex, condvar, and aio init that never fail. If the underlying platform fails (FreeBSD is the only one I'm aware of that does this!), we use a global lock or condition variable instead. This means that our lock initializers never ever fail. Probably we could eliminate most of this for Linux and Darwin, since on those platforms, mutex and condvar initialization reasonably never fails. Initial benchmarks show little difference either way -- so we can revisit (optimize) later. This removes a lot of otherwise untested code in error cases and so forth, improving coverage and resilience in the face of allocation failures. Platforms other than POSIX should follow a similar pattern if they need this. (VxWorks, I'm thinking of you.) Most sane platforms won't have an issue here, since normally these initializations do not need to allocate memory. (Reportedly, even FreeBSD has plans to "fix" this in libthr2.) While here, some bugs were fixed in initialization & teardown. The fallback code is properly tested with dedicated test cases. --- src/transport/inproc/inproc.c | 12 ++++-------- src/transport/ipc/ipc.c | 22 ++++++++++------------ src/transport/tcp/tcp.c | 22 ++++++++++------------ 3 files changed, 24 insertions(+), 32 deletions(-) (limited to 'src/transport') diff --git a/src/transport/inproc/inproc.c b/src/transport/inproc/inproc.c index 3bc24c41..08cf99a2 100644 --- a/src/transport/inproc/inproc.c +++ b/src/transport/inproc/inproc.c @@ -65,14 +65,9 @@ static nni_inproc_global nni_inproc; static int nni_inproc_init(void) { - int rv; - NNI_LIST_INIT(&nni_inproc.servers, nni_inproc_ep, node); - if ((rv = nni_mtx_init(&nni_inproc.mx)) != 0) { - return (rv); - } - + nni_mtx_init(&nni_inproc.mx); return (0); } @@ -309,8 +304,7 @@ nni_inproc_accept_clients(nni_inproc_ep *server) continue; } - if (((rv = nni_mtx_init(&pair->mx)) != 0) || - ((rv = nni_msgq_init(&pair->q[0], 4)) != 0) || + if (((rv = nni_msgq_init(&pair->q[0], 4)) != 0) || ((rv = nni_msgq_init(&pair->q[1], 4)) != 0)) { nni_inproc_pair_destroy(pair); nni_inproc_conn_finish(caio, rv); @@ -318,6 +312,8 @@ nni_inproc_accept_clients(nni_inproc_ep *server) continue; } + nni_mtx_init(&pair->mx); + pair->pipes[0] = caio->a_pipe; pair->pipes[1] = saio->a_pipe; pair->pipes[0]->rq = pair->pipes[1]->wq = pair->q[0]; diff --git a/src/transport/ipc/ipc.c b/src/transport/ipc/ipc.c index 7d976122..0b0e487f 100644 --- a/src/transport/ipc/ipc.c +++ b/src/transport/ipc/ipc.c @@ -107,18 +107,14 @@ static int nni_ipc_pipe_init(nni_ipc_pipe **pipep, nni_ipc_ep *ep, void *ipp) { nni_ipc_pipe *p; - int rv; if ((p = NNI_ALLOC_STRUCT(p)) == NULL) { return (NNG_ENOMEM); } - if (((rv = nni_mtx_init(&p->mtx)) != 0) || - ((rv = nni_aio_init(&p->txaio, nni_ipc_pipe_send_cb, p)) != 0) || - ((rv = nni_aio_init(&p->rxaio, nni_ipc_pipe_recv_cb, p)) != 0) || - ((rv = nni_aio_init(&p->negaio, nni_ipc_pipe_nego_cb, p)) != 0)) { - nni_ipc_pipe_fini(p); - return (rv); - } + nni_mtx_init(&p->mtx); + nni_aio_init(&p->txaio, nni_ipc_pipe_send_cb, p); + nni_aio_init(&p->rxaio, nni_ipc_pipe_recv_cb, p); + nni_aio_init(&p->negaio, nni_ipc_pipe_nego_cb, p); p->proto = ep->proto; p->rcvmax = ep->rcvmax; @@ -490,12 +486,14 @@ nni_ipc_ep_init(void **epp, const char *url, nni_sock *sock, int mode) if ((ep = NNI_ALLOC_STRUCT(ep)) == NULL) { return (NNG_ENOMEM); } - if (((rv = nni_mtx_init(&ep->mtx)) != 0) || - ((rv = nni_aio_init(&ep->aio, nni_ipc_ep_cb, ep)) != 0) || - ((rv = nni_plat_ipc_ep_init(&ep->iep, url, mode)) != 0)) { - nni_ipc_ep_fini(ep); + if ((rv = nni_plat_ipc_ep_init(&ep->iep, url, mode)) != 0) { + NNI_FREE_STRUCT(ep); return (rv); } + + nni_mtx_init(&ep->mtx); + nni_aio_init(&ep->aio, nni_ipc_ep_cb, ep); + ep->closed = 0; ep->proto = nni_sock_proto(sock); ep->rcvmax = nni_sock_rcvmaxsz(sock); diff --git a/src/transport/tcp/tcp.c b/src/transport/tcp/tcp.c index 4d47733b..b3136b35 100644 --- a/src/transport/tcp/tcp.c +++ b/src/transport/tcp/tcp.c @@ -107,18 +107,15 @@ static int nni_tcp_pipe_init(nni_tcp_pipe **pipep, nni_tcp_ep *ep, void *tpp) { nni_tcp_pipe *p; - int rv; if ((p = NNI_ALLOC_STRUCT(p)) == NULL) { return (NNG_ENOMEM); } - if (((rv = nni_mtx_init(&p->mtx)) != 0) || - ((rv = nni_aio_init(&p->txaio, nni_tcp_pipe_send_cb, p)) != 0) || - ((rv = nni_aio_init(&p->rxaio, nni_tcp_pipe_recv_cb, p)) != 0) || - ((rv = nni_aio_init(&p->negaio, nni_tcp_pipe_nego_cb, p)) != 0)) { - nni_tcp_pipe_fini(p); - return (rv); - } + nni_mtx_init(&p->mtx); + nni_aio_init(&p->txaio, nni_tcp_pipe_send_cb, p); + nni_aio_init(&p->rxaio, nni_tcp_pipe_recv_cb, p); + nni_aio_init(&p->negaio, nni_tcp_pipe_nego_cb, p); + p->proto = ep->proto; p->rcvmax = ep->rcvmax; p->tpp = tpp; @@ -555,12 +552,13 @@ nni_tcp_ep_init(void **epp, const char *url, nni_sock *sock, int mode) if ((ep = NNI_ALLOC_STRUCT(ep)) == NULL) { return (NNG_ENOMEM); } - if (((rv = nni_mtx_init(&ep->mtx)) != 0) || - ((rv = nni_aio_init(&ep->aio, nni_tcp_ep_cb, ep)) != 0) || - ((rv = nni_plat_tcp_ep_init(&ep->tep, url, mode)) != 0)) { - nni_tcp_ep_fini(ep); + if ((rv = nni_plat_tcp_ep_init(&ep->tep, url, mode)) != 0) { + NNI_FREE_STRUCT(ep); return (rv); } + nni_mtx_init(&ep->mtx); + nni_aio_init(&ep->aio, nni_tcp_ep_cb, ep); + ep->closed = 0; ep->proto = nni_sock_proto(sock); ep->rcvmax = nni_sock_rcvmaxsz(sock); -- cgit v1.2.3-70-g09d2