From 568a84ed2d3d41da5ca64cde15a677237fffd991 Mon Sep 17 00:00:00 2001 From: Garrett D'Amore Date: Sat, 21 Jan 2017 17:40:04 -0800 Subject: Fix leaks in bus, socket leaks, tighten up close-side refcnting. This does a few things. First it closes some preexisting leaks. Second it tightens the overall close logic so that we automatically discard idhash resources (while keeping numeric values for next id etc. around) when the last socket is closed. This then eliminates the need for applications to ever explicitly terminate resources. It turns out platform-specific resources established at nni_init() time might still be leaked, but it's also the case that we now no longer dynamically allocate anything at platform initialization time. (This presumes that the platform doesn't do so under the hood when creating critical sections or mutexes for example.) --- tests/bus.c | 3 -- tests/event.c | 23 +++++-------- tests/idhash.c | 100 +++++++++++++++++++++------------------------------------ tests/inproc.c | 2 -- tests/ipc.c | 2 -- tests/sock.c | 11 ++++--- tests/tcp.c | 4 --- 7 files changed, 51 insertions(+), 94 deletions(-) (limited to 'tests') diff --git a/tests/bus.c b/tests/bus.c index 6703fa0e..f0c84406 100644 --- a/tests/bus.c +++ b/tests/bus.c @@ -19,7 +19,6 @@ Main({ const char *addr = "inproc://test"; - nni_init(); Test("BUS pattern", { Convey("We can create a BUS socket", { @@ -96,6 +95,4 @@ Main({ }) }) }) - - nni_fini(); }) diff --git a/tests/event.c b/tests/event.c index 30e8f438..554f990f 100644 --- a/tests/event.c +++ b/tests/event.c @@ -9,8 +9,8 @@ #include "convey.h" #include "nng.h" -#include "core/nng_impl.h" #include +#include #define APPENDSTR(m, s) nng_msg_append(m, s, strlen(s)) #define CHECKSTR(m, s) So(nng_msg_len(m) == strlen(s));\ @@ -31,10 +31,7 @@ bump(nng_event *ev, void *arg) { struct evcnt *cnt = arg; - if (nng_event_socket(ev) != cnt->sock) { - nni_panic("Incorrect socket! %p != %p", - nng_event_socket(ev), cnt->sock); - } + assert(nng_event_socket(ev) == cnt->sock); switch (nng_event_type(ev)) { case NNG_EV_CAN_SEND: cnt->writeable++; @@ -61,7 +58,7 @@ bump(nng_event *ev, void *arg) break; default: - nni_panic("Invalid event type %d", nng_event_type(ev)); + assert(0); break; } } @@ -69,8 +66,6 @@ bump(nng_event *ev, void *arg) Main({ const char *addr = "inproc://test"; - nni_init(); - Test("Event Handling", { Convey("Given a connected pair of pair sockets", { nng_socket sock1; @@ -97,26 +92,26 @@ Main({ So(nng_dial(sock2, addr, NULL, NNG_FLAG_SYNCH) == 0); // Let everything connect. - nni_usleep(100000); + nng_usleep(100000); Convey("We can register callbacks", { So((notify1 = nng_setnotify(sock1, NNG_EV_CAN_SEND, bump, &evcnt1)) != NULL); So((notify2 = nng_setnotify(sock2, NNG_EV_CAN_RECV, bump, &evcnt2)) != NULL); Convey("They are called", { - nni_msg *msg; + nng_msg *msg; - So(nni_msg_alloc(&msg, 0) == 0); + So(nng_msg_alloc(&msg, 0) == 0); APPENDSTR(msg, "abc"); So(nng_sendmsg(sock1, msg, 0) == 0); So(nng_recvmsg(sock2, &msg, 0) == 0); CHECKSTR(msg, "abc"); - nni_msg_free(msg); + nng_msg_free(msg); // The notify runs async... - nni_usleep(100000); + nng_usleep(100000); So(evcnt1.writeable == 1); So(evcnt2.readable == 1); @@ -131,6 +126,4 @@ Main({ }) }) }) - - nni_fini(); }) diff --git a/tests/idhash.c b/tests/idhash.c index 600b17ae..50a8d976 100644 --- a/tests/idhash.c +++ b/tests/idhash.c @@ -8,86 +8,69 @@ // #include "convey.h" -#include "core/nng_impl.h" #include "core/idhash.c" Main({ - nni_init(); - Test("General ID Hash", { int rv; Convey("Given an id hash", { - nni_idhash *h; + nni_idhash h; - rv = nni_idhash_create(&h); - So(rv == 0); - So(h->ih_cap == 8); - So(h->ih_entries != NULL); - So(h->ih_count == 0); + So(nni_idhash_init(&h) == 0); + So(nni_idhash_count(&h) == 0); Reset({ - nni_idhash_destroy(h); + nni_idhash_fini(&h); }) Convey("We can insert an element", { char *five = "five"; char *four = "four"; - rv = nni_idhash_insert(h, 5, five); + rv = nni_idhash_insert(&h, 5, five); + So(nni_idhash_count(&h) == 1); So(rv == 0); - So(h->ih_load == 1); - So(h->ih_count == 1); Convey("And we can find it", { void *ptr; - rv = nni_idhash_find(h, 5, &ptr); + rv = nni_idhash_find(&h, 5, &ptr); So(rv == 0); So(ptr == five); }) Convey("We can delete it", { void *ptr; - rv = nni_idhash_remove(h, 5); + rv = nni_idhash_remove(&h, 5); So(rv == 0); - rv = nni_idhash_find(h, 5, &ptr); + rv = nni_idhash_find(&h, 5, &ptr); So(rv == NNG_ENOENT); }) Convey("We can change the value", { void *ptr; - rv = nni_idhash_insert(h, 5, four); - So(rv == 0); - So(h->ih_count == 1); - rv = nni_idhash_find(h, 5, &ptr); - So(rv == 0); + So(nni_idhash_insert(&h, 5, four) == 0); + So(nni_idhash_count(&h) == 1); + So(nni_idhash_find(&h, 5, &ptr) == 0); So(ptr == four); }) Convey("We can insert a hash collision", { void *ptr; - rv = nni_idhash_insert(h, 13, four); - So(rv == 0); - So(h->ih_load == 2); - So(h->ih_count == 2); - rv = nni_idhash_find(h, 5, &ptr); - So(rv == 0); + So(nni_idhash_insert(&h, 13, four) == 0); + So(nni_idhash_count(&h) == 2); + So(nni_idhash_find(&h, 5, &ptr) == 0); So(ptr == five); - rv = nni_idhash_find(h, 13, &ptr); - So(rv == 0); + So(nni_idhash_find(&h, 13, &ptr) == 0); So(ptr == four); - So(h->ih_entries[5].ihe_skips == 1); Convey("And delete the intermediate", { - rv = nni_idhash_remove(h, 5); - So(rv == 0); + So(nni_idhash_remove(&h, 5) == 0); ptr = NULL; - rv = nni_idhash_find(h, 13, &ptr); - So(rv == 0); + So(nni_idhash_find(&h, 13, &ptr) == 0); So(ptr == four); - So(h->ih_load == 2); }) }) }) Convey("We cannot find bogus values", { void *ptr = NULL; - rv = nni_idhash_find(h, 42, &ptr); + rv = nni_idhash_find(&h, 42, &ptr); So(rv == NNG_ENOENT); So(ptr == NULL); }) @@ -103,34 +86,27 @@ Main({ expect[i] = i; } Convey("Given an id hash", { - nni_idhash *h; + nni_idhash h; - rv = nni_idhash_create(&h); - So(rv == 0); - So(h->ih_cap == 8); - So(h->ih_entries != NULL); - So(h->ih_count == 0); + So(nni_idhash_init(&h) == 0); + So(nni_idhash_count(&h) == 0); Reset({ - nni_idhash_destroy(h); + nni_idhash_fini(&h); }) Convey("We can insert 1024 items", { uint32_t count; for (i = 0; i < 1024; i++) { - nni_idhash_insert(h, i, &expect[i]); + nni_idhash_insert(&h, i, &expect[i]); } - So(nni_idhash_count(h, &count) == 0); - So(count == 1024); - So(h->ih_cap = 2048); - So(h->ih_count == 1024); + So(nni_idhash_count(&h) == 1024); Convey("We can remove them", { for (i = 0; i < 1024; i++) { - nni_idhash_remove(h, i); + nni_idhash_remove(&h, i); } - So(h->ih_count == 0); - So(h->ih_cap == 8); + So(nni_idhash_count(&h) == 0); }) }) }) @@ -138,36 +114,34 @@ Main({ Test("Dynamic ID generation", { Convey("Given a small ID hash", { - nni_idhash *h; + nni_idhash h; int expect[5]; uint32_t id; int i; - So(nni_idhash_create(&h) == 0); + So(nni_idhash_init(&h) == 0); Reset({ - nni_idhash_destroy(h); + nni_idhash_fini(&h); }) - nni_idhash_set_limits(h, 10, 13, 10); + nni_idhash_set_limits(&h, 10, 13, 10); So(1); Convey("We can fill the table", { for (i = 0; i < 4; i++) { - So(nni_idhash_alloc(h, &id, &expect[i]) == 0); + So(nni_idhash_alloc(&h, &id, &expect[i]) == 0); So(id == (i + 10)); } Convey("Adding another fails", { - So(nni_idhash_alloc(h, &id, &expect[5]) == NNG_ENOMEM); + So(nni_idhash_alloc(&h, &id, &expect[5]) == NNG_ENOMEM); }) Convey("Deleting one lets us reinsert", { - nni_idhash_remove(h, 11); - So(nni_idhash_alloc(h, &id, &expect[5]) == 0); + nni_idhash_remove(&h, 11); + So(nni_idhash_alloc(&h, &id, &expect[5]) == 0); So(id == 11); }) }) Convey("We cannot insert bogus values", { - So(nni_idhash_insert(h, 1, &expect[0]) == NNG_EINVAL); - So(nni_idhash_insert(h, 100, &expect[0]) == NNG_EINVAL); + So(nni_idhash_insert(&h, 1, &expect[0]) == NNG_EINVAL); + So(nni_idhash_insert(&h, 100, &expect[0]) == NNG_EINVAL); }) }) }) - - nni_fini(); }) diff --git a/tests/inproc.c b/tests/inproc.c index cfbf3153..0375320f 100644 --- a/tests/inproc.c +++ b/tests/inproc.c @@ -15,7 +15,5 @@ // Inproc tests. TestMain("Inproc Transport", { - nni_init(); trantest_test_all("inproc://TEST"); - nni_fini(); }) diff --git a/tests/ipc.c b/tests/ipc.c index 7e9221f3..69fe36d0 100644 --- a/tests/ipc.c +++ b/tests/ipc.c @@ -14,7 +14,5 @@ // Inproc tests. TestMain("IPC Transport", { - nni_init(); trantest_test_all("ipc:///tmp/nng_ipc_test"); - nni_fini(); }) diff --git a/tests/sock.c b/tests/sock.c index ec923e4a..c43b6fc1 100644 --- a/tests/sock.c +++ b/tests/sock.c @@ -14,8 +14,6 @@ #include Main({ - nni_init(); - Test("Socket Operations", { Convey("We are able to open a PAIR socket", { @@ -137,6 +135,9 @@ Main({ Convey("We can connect to it", { nng_socket sock2; So(nng_open(&sock2, NNG_PROTO_PAIR) == 0); + Reset({ + nng_close(sock2); + }) rv = nng_dial(sock2, "inproc://here", NULL, NNG_FLAG_SYNCH); So(rv == 0); nng_close(sock2); @@ -151,6 +152,9 @@ Main({ char *buf; So(nng_open(&sock2, NNG_PROTO_PAIR) == 0); + Reset({ + nng_close(sock2); + }) So(nng_setopt(sock, NNG_OPT_RCVBUF, &len, sizeof (len)) == 0); So(nng_setopt(sock, NNG_OPT_SNDBUF, &len, sizeof (len)) == 0); @@ -172,10 +176,7 @@ Main({ So(sz == 4); So(memcmp(buf, "abc", 4) == 0); nng_free(buf, sz); - nng_close(sock2); }) }) }) - - nni_fini(); }) diff --git a/tests/tcp.c b/tests/tcp.c index 0ec69121..73039e64 100644 --- a/tests/tcp.c +++ b/tests/tcp.c @@ -16,7 +16,6 @@ TestMain("TCP Transport", { int rv; - nni_init(); trantest_test_all("tcp://127.0.0.1:4450"); @@ -43,7 +42,4 @@ TestMain("TCP Transport", { So(nng_dial(s2, "tcp://127.0.0.1:5771", NULL, NNG_FLAG_SYNCH) == 0); fflush(stdout); }) - nni_fini(); - - }) -- cgit v1.2.3-70-g09d2