fixes #1088 REP protocol does not signal SENDFD properly

We've also added some TEST_NNG_SEND_STR and TEST_NNG_RECV_STR to
help with convenience when writing test code.

4 files changed, 212 insertions, 10 deletions

diff --git a/src/protocol/reqrep0/CMakeLists.txt b/src/protocol/reqrep0/CMakeLists.txt
index 84c25c0e..4d2903c6 100644
--- a/src/protocol/reqrep0/CMakeLists.txt
+++ b/src/protocol/reqrep0/CMakeLists.txt
@@ -23,4 +23,5 @@ nng_sources_if(NNG_PROTO_REP0 rep.c xrep.c)
 nng_headers_if(NNG_PROTO_REP0 nng/protocol/reqrep0/rep.h)
 nng_defines_if(NNG_PROTO_REP0 NNG_HAVE_REP0)
 
-nng_test(reqrep_test)
\ No newline at end of file
+nng_test(reqrep_test)
+nng_test(rep_test)
\ No newline at end of file
diff --git a/src/protocol/reqrep0/rep.c b/src/protocol/reqrep0/rep.c
index 2d00b65f..f8c15fa6 100644
--- a/src/protocol/reqrep0/rep.c
+++ b/src/protocol/reqrep0/rep.c
@@ -1,5 +1,5 @@
 //
-// Copyright 2019 Staysail Systems, Inc. <info@staysail.tech>
+// Copyright 2020 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
@@ -502,6 +502,9 @@ rep0_ctx_recv(void *arg, nni_aio *aio)
 		nni_pollable_clear(s->recvable);
 	}
 	nni_pipe_recv(p->pipe, p->aio_recv);
+	if ((ctx == s->ctx) && !p->busy) {
+		nni_pollable_raise(s->sendable);
+	}
 
 	len = nni_msg_header_len(msg);
 	memcpy(ctx->btrace, nni_msg_header(msg), len);
@@ -583,6 +586,9 @@ rep0_pipe_recv_cb(void *arg)
 	aio       = ctx->raio;
 	ctx->raio = NULL;
 	nni_aio_set_msg(p->aio_recv, NULL);
+	if ((ctx == s->ctx) && !p->busy) {
+		nni_pollable_raise(s->sendable);
+	}
 
 	// schedule another receive
 	nni_pipe_recv(p->pipe, p->aio_recv);
@@ -592,13 +598,6 @@ rep0_pipe_recv_cb(void *arg)
 	nni_msg_header_clear(msg);
 	ctx->pipe_id = p->id;
 
-	// If we got a request on a pipe that wasn't busy, we should
-	// mark it sendable.  (The sendable flag is not set when there
-	// is no request needing a reply.)
-	if ((ctx == s->ctx) && (!p->busy)) {
-		nni_pollable_raise(s->sendable);
-	}
-
 	nni_mtx_unlock(&s->lk);
 
 	nni_aio_set_msg(aio, msg);
diff --git a/src/protocol/reqrep0/rep_test.c b/src/protocol/reqrep0/rep_test.c
new file mode 100644
index 00000000..784cdeb7
--- /dev/null
+++ b/src/protocol/reqrep0/rep_test.c
@@ -0,0 +1,187 @@
+//
+// Copyright 2020 Staysail Systems, Inc. <info@staysail.tech>
+//
+// This software is supplied under the terms of the MIT License, a
+// copy of which should be located in the distribution where this
+// file was obtained (LICENSE.txt).  A copy of the license may also be
+// found online at https://opensource.org/licenses/MIT.
+//
+
+#include <string.h>
+
+#include <nng/nng.h>
+#include <nng/protocol/reqrep0/rep.h>
+#include <nng/protocol/reqrep0/req.h>
+
+#include <acutest.h>
+#include <testutil.h>
+
+#ifndef NNI_PROTO
+#define NNI_PROTO(x, y) (((x) << 4u) | (y))
+#endif
+
+void
+test_rep_identity(void)
+{
+	nng_socket s;
+	int        p;
+	char *     n;
+
+	TEST_CHECK(nng_rep0_open(&s) == 0);
+	TEST_CHECK(nng_getopt_int(s, NNG_OPT_PROTO, &p) == 0);
+	TEST_CHECK(p == NNI_PROTO(3u, 1u)); // 49
+	TEST_CHECK(nng_getopt_int(s, NNG_OPT_PEER, &p) == 0);
+	TEST_CHECK(p == NNI_PROTO(3u, 0u)); // 48
+	TEST_CHECK(nng_getopt_string(s, NNG_OPT_PROTONAME, &n) == 0);
+	TEST_CHECK(strcmp(n, "rep") == 0);
+	nng_strfree(n);
+	TEST_CHECK(nng_getopt_string(s, NNG_OPT_PEERNAME, &n) == 0);
+	TEST_CHECK(strcmp(n, "req") == 0);
+	nng_strfree(n);
+	TEST_CHECK(nng_close(s) == 0);
+}
+
+void
+test_rep_send_bad_state(void)
+{
+	nng_socket rep;
+	nng_msg *  msg = NULL;
+
+	TEST_CHECK(nng_rep0_open(&rep) == 0);
+	TEST_CHECK(nng_msg_alloc(&msg, 0) == 0);
+	TEST_CHECK(nng_sendmsg(rep, msg, 0) == NNG_ESTATE);
+	nng_msg_free(msg);
+	TEST_CHECK(nng_close(rep) == 0);
+}
+
+void
+test_rep_poll_writeable(void)
+{
+	int        fd;
+	nng_socket req;
+	nng_socket rep;
+
+	TEST_NNG_PASS(nng_req0_open(&req));
+	TEST_NNG_PASS(nng_rep0_open(&rep));
+	TEST_NNG_PASS(nng_getopt_int(rep, NNG_OPT_SENDFD, &fd));
+	TEST_CHECK(fd >= 0);
+
+	// Not writable before connect.
+	TEST_CHECK(testutil_pollfd(fd) == false);
+
+	TEST_NNG_PASS(testutil_marry(req, rep));
+
+	// Still not writable.
+	TEST_CHECK(testutil_pollfd(fd) == false);
+
+	// If we get a job, *then* we become writeable
+	TEST_NNG_SEND_STR(req, "abc");
+	TEST_NNG_RECV_STR(rep, "abc");
+	TEST_CHECK(testutil_pollfd(fd) == true);
+
+	// And is no longer writable once we send a message
+	TEST_NNG_SEND_STR(rep, "def");
+	TEST_CHECK(testutil_pollfd(fd) == false);
+	// Even after receiving it
+	TEST_NNG_RECV_STR(req, "def");
+	TEST_CHECK(testutil_pollfd(fd) == false);
+
+	TEST_NNG_PASS(nng_close(req));
+	TEST_NNG_PASS(nng_close(rep));
+}
+
+void
+test_rep_poll_readable(void)
+{
+	int        fd;
+	nng_socket req;
+	nng_socket rep;
+	nng_msg *  msg;
+
+	TEST_NNG_PASS(nng_req0_open(&req));
+	TEST_NNG_PASS(nng_rep0_open(&rep));
+	TEST_NNG_PASS(nng_getopt_int(rep, NNG_OPT_RECVFD, &fd));
+	TEST_CHECK(fd >= 0);
+
+	// Not readable if not connected!
+	TEST_CHECK(testutil_pollfd(fd) == false);
+
+	// Even after connect (no message yet)
+	TEST_NNG_PASS(testutil_marry(req, rep));
+	TEST_CHECK(testutil_pollfd(fd) == false);
+
+	// But once we send messages, it is.
+	// We have to send a request, in order to send a reply.
+	TEST_NNG_SEND_STR(req, "abc");
+	testutil_sleep(100);
+
+	TEST_CHECK(testutil_pollfd(fd) == true);
+
+	// and receiving makes it no longer pollable
+	TEST_NNG_PASS(nng_recvmsg(rep, &msg, 0));
+	nng_msg_free(msg);
+	TEST_CHECK(testutil_pollfd(fd) == false);
+
+	// TODO verify unsolicited response
+
+	TEST_NNG_PASS(nng_close(req));
+	TEST_NNG_PASS(nng_close(rep));
+}
+
+void
+test_rep_context_not_pollable(void)
+{
+	int        fd;
+	nng_socket req;
+	nng_ctx    ctx;
+
+	TEST_NNG_PASS(nng_rep0_open(&req));
+	TEST_NNG_PASS(nng_ctx_open(&ctx, req));
+	TEST_NNG_FAIL(
+	    nng_ctx_getopt_int(ctx, NNG_OPT_SENDFD, &fd), NNG_ENOTSUP);
+	TEST_NNG_FAIL(
+	    nng_ctx_getopt_int(ctx, NNG_OPT_RECVFD, &fd), NNG_ENOTSUP);
+	TEST_NNG_PASS(nng_ctx_close(ctx));
+	TEST_NNG_PASS(nng_close(req));
+}
+
+void
+test_rep_validate_peer(void)
+{
+	nng_socket s1, s2;
+	nng_stat * stats;
+	nng_stat * reject;
+	char       addr[64];
+
+	testutil_scratch_addr("inproc", sizeof(addr), addr);
+
+	TEST_NNG_PASS(nng_rep0_open(&s1));
+	TEST_NNG_PASS(nng_rep0_open(&s2));
+
+	TEST_NNG_PASS(nng_listen(s1, addr, NULL, 0));
+	TEST_NNG_PASS(nng_dial(s2, addr, NULL, NNG_FLAG_NONBLOCK));
+
+	testutil_sleep(100);
+	TEST_NNG_PASS(nng_stats_get(&stats));
+
+	TEST_CHECK(stats != NULL);
+	TEST_CHECK((reject = nng_stat_find_socket(stats, s1)) != NULL);
+	TEST_CHECK((reject = nng_stat_find(reject, "reject")) != NULL);
+
+	TEST_CHECK(nng_stat_type(reject) == NNG_STAT_COUNTER);
+	TEST_CHECK(nng_stat_value(reject) > 0);
+
+	TEST_NNG_PASS(nng_close(s1));
+	TEST_NNG_PASS(nng_close(s2));
+	nng_stats_free(stats);
+}
+
+TEST_LIST = {
+	{ "rep identity", test_rep_identity },
+	{ "rep send bad state", test_rep_send_bad_state },
+	{ "rep poll readable", test_rep_poll_readable },
+	{ "rep poll writable", test_rep_poll_writeable },
+	{ "rep context not pollable", test_rep_context_not_pollable },
+	{ "rep validate peer", test_rep_validate_peer },
+	{ NULL, NULL },
+};
diff --git a/tests/testutil.h b/tests/testutil.h
index 14cc1712..008c8061 100644
--- a/tests/testutil.h
+++ b/tests/testutil.h
@@ -55,7 +55,7 @@ extern int testutil_marry(nng_socket, nng_socket);
 #define TEST_NNG_PASS(cond)                                          \
 	do {                                                         \
 		int result_ = (cond);                                \
-		TEST_CHECK_(result_ == 0, "%s succeeds", #cond);                    \
+		TEST_CHECK_(result_ == 0, "%s succeeds", #cond);     \
 		TEST_MSG("%s: expected success, got %s (%d)", #cond, \
 		    nng_strerror(result_), result_);                 \
 	} while (0)
@@ -70,6 +70,21 @@ extern int testutil_marry(nng_socket, nng_socket);
 		    result_);                                             \
 	} while (0)
 
+#define TEST_NNG_SEND_STR(sock, string) \
+	TEST_NNG_PASS(nng_send(sock, string, strlen(string) + 1, 0))
+
+#define TEST_NNG_RECV_STR(sock, string)                                    \
+	do {                                                               \
+		char * buf_;                                                \
+		size_t sz_;                                                \
+		int    rv_ = nng_recv(sock, &buf_, &sz_, NNG_FLAG_ALLOC);  \
+		TEST_CHECK_(                                               \
+		    rv_ == 0, "nng_recv (%d %s)", rv_, nng_strerror(rv_)); \
+		TEST_CHECK(sz_ == strlen(string) + 1);                     \
+		TEST_CHECK(strcmp(string, buf_) == 0);                     \
+		nng_free(buf_, sz_);                                       \
+	} while (0)
+
 #ifdef __cplusplus
 };
 #endif