Repository: qpid-proton Updated Branches: refs/heads/master 1bb1897fe -> 485cdbd3f
PROTON-1618: c++ tests use test_port.h for listen ports POSIX: Use bind(0) with SO_REUSEADDR and hold the socket to acquire a port that can safely be used for listen() Windows: Use bind(0) to pick a port, but close the socket immediately. In theory another process could steal the port between bind() and listen(), but in practice this seems to be very unlikely. The previous randomize-and-retry approach makes it hard to test the sequence of events for listen(), since the random retry may cause multiple listen errors even when it is finally successful. Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/485cdbd3 Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/485cdbd3 Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/485cdbd3 Branch: refs/heads/master Commit: 485cdbd3f680772e081ca05fbda97c0f271c676e Parents: 1d2d791 Author: Alan Conway <acon...@redhat.com> Authored: Wed Oct 11 09:47:33 2017 -0400 Committer: Alan Conway <acon...@redhat.com> Committed: Wed Oct 11 22:04:42 2017 -0400 ---------------------------------------------------------------------- proton-c/bindings/cpp/src/container_test.cpp | 49 ++++---- proton-c/src/tests/proactor.c | 96 +-------------- proton-c/src/tests/test_port.h | 138 ++++++++++++++++++++++ 3 files changed, 168 insertions(+), 115 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/485cdbd3/proton-c/bindings/cpp/src/container_test.cpp ---------------------------------------------------------------------- diff --git a/proton-c/bindings/cpp/src/container_test.cpp b/proton-c/bindings/cpp/src/container_test.cpp index 3b0c86f..c9657b9 100644 --- a/proton-c/bindings/cpp/src/container_test.cpp +++ b/proton-c/bindings/cpp/src/container_test.cpp @@ -19,6 +19,10 @@ #include "test_bits.hpp" +extern "C" { +#include "../../../../src/tests/test_port.h" +} + #include "proton/connection.hpp" #include "proton/connection_options.hpp" #include "proton/container.hpp" @@ -35,28 +39,23 @@ namespace { -static std::string make_url(const std::string host, int port) { +std::string make_url(const std::string& host, int port) { std::ostringstream url; url << "amqp://" << host << ":" << port; return url.str(); } -int listen_on_random_port(proton::container& c, proton::listener& l, proton::listen_handler* lh=0) { - int port = 0; - // I'm going to hell for this: - std::srand((unsigned int)time(0)); - while (true) { - port = 20000 + (std::rand() % 30000); - std::string url = make_url("", port); - try { - l = lh ? c.listen(url, *lh) : c.listen(url); - break; - } catch (...) { - // keep trying - } - } - return port; -} +// C++ Wrapper for C test port. +// Binds to a port with REUSEADDR set so that the port is protected from +// other processes and can safely be used for listening. +class listen_port { + ::test_port_t tp; + public: + listen_port() { tp = ::test_port(""); } // NOTE: assign tp, don't initialize - Windows. + ~listen_port() { ::test_port_close(&tp); } + int port() const { return tp.port; } + std::string url(const std::string& host="") const { return make_url(host, tp.port); } +}; struct test_listen_handler : public proton::listen_handler { bool on_open_, on_accept_, on_close_; @@ -92,6 +91,7 @@ class test_handler : public proton::messaging_handler { std::string peer_vhost; std::string peer_container_id; + listen_port port; proton::listener listener; test_listen_handler listen_handler; @@ -100,8 +100,8 @@ class test_handler : public proton::messaging_handler { {} void on_container_start(proton::container &c) PN_CPP_OVERRIDE { - int port = listen_on_random_port(c, listener, &listen_handler); - proton::connection conn = c.connect(make_url(host, port), opts); + listener = c.listen(port.url(), listen_handler); + proton::connection conn = c.connect(port.url(host), opts); } void on_connection_open(proton::connection &c) PN_CPP_OVERRIDE { @@ -183,13 +183,14 @@ int test_container_bad_address() { } class stop_tester : public proton::messaging_handler { + listen_port port; proton::listener listener; // Set up a listener which would block forever void on_container_start(proton::container& c) PN_CPP_OVERRIDE { ASSERT(state==0); - int port = listen_on_random_port(c, listener); - c.connect(make_url("", port)); + listener = c.listen(port.url()); + c.connect(port.url()); c.auto_stop(false); state = 1; } @@ -231,17 +232,17 @@ int test_container_stop() { struct hang_tester : public proton::messaging_handler { proton::listener listener; - int port; + listen_port port; bool done; hang_tester() : done(false) {} void connect(proton::container* c) { - c->connect(make_url("", port)); + c->connect(port.url()); } void on_container_start(proton::container& c) PN_CPP_OVERRIDE { - port = listen_on_random_port(c, listener); + listener = c.listen(port.url()); c.schedule(proton::duration(250), make_work(&hang_tester::connect, this, &c)); } http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/485cdbd3/proton-c/src/tests/proactor.c ---------------------------------------------------------------------- diff --git a/proton-c/src/tests/proactor.c b/proton-c/src/tests/proactor.c index 5296802..5f84bfe 100644 --- a/proton-c/src/tests/proactor.c +++ b/proton-c/src/tests/proactor.c @@ -17,7 +17,7 @@ * under the License. */ -#include "../platform/platform.h" +#include "test_port.h" #include "test_tools.h" #include "test_handler.h" #include "test_config.h" @@ -38,78 +38,6 @@ static const char *localhost = ""; /* host for connect/listen */ -/* Some very simple platform-secifics to acquire an unused socket */ -#if defined(_WIN32) - -#include <winsock2.h> -#include <ws2tcpip.h> -typedef SOCKET sock_t; -void sock_close(sock_t sock) { closesocket(sock); } -// pni_snprintf not exported. We can live with a simplified version -// for this test's limited use. Abort if that assumption is wrong. -#define pni_snprintf pnitst_snprintf -static int pnitst_snprintf(char *buf, size_t count, const char *fmt, ...) { - va_list ap; - va_start(ap, fmt); - int n = _vsnprintf(buf, count, fmt, ap); - va_end(ap); - if (count == 0 || n < 0) { - perror("proton internal failure on Windows test snprintf"); - abort(); - } - // Windows and C99 are in agreement. - return n; -} - -#else /* POSIX */ -# include <sys/types.h> -# include <sys/socket.h> -# include <netinet/in.h> -# include <unistd.h> -typedef int sock_t; -void sock_close(sock_t sock) { close(sock); } -#endif - -/* Combines a sock_t with the int and char* versions of the port for convenience */ -typedef struct test_port_t { - sock_t sock; - int port; /* port as integer */ - char str[PN_MAX_ADDR]; /* port as string */ - char host_port[PN_MAX_ADDR]; /* host:port string */ -} test_port_t; - -/* Modifies tp->host_port to use host, returns the new tp->host_port */ -const char *test_port_use_host(test_port_t *tp, const char *host) { - pni_snprintf(tp->host_port, sizeof(tp->host_port), "%s:%d", host, tp->port); - return tp->host_port; -} - -/* Create a socket and bind(INADDR_LOOPBACK:0) to get a free port. - Use SO_REUSEADDR so other processes can bind and listen on this port. - Use host to create the host_port address string. -*/ -test_port_t test_port(const char* host) { - test_port_t tp = {0}; - tp.sock = socket(AF_INET, SOCK_STREAM, 0); - TEST_ASSERT_ERRNO(tp.sock >= 0, errno); - int on = 1; - int err = setsockopt(tp.sock, SOL_SOCKET, SO_REUSEADDR, (const char*)&on, sizeof(on)); - TEST_ASSERT_ERRNO(!err, errno); - struct sockaddr_in addr = {0}; - addr.sin_family = AF_INET; /* set the type of connection to TCP/IP */ - addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); - addr.sin_port = 0; /* bind to port 0 */ - err = bind(tp.sock, (struct sockaddr*)&addr, sizeof(addr)); - TEST_ASSERT_ERRNO(!err, errno); - socklen_t len = sizeof(addr); - err = getsockname(tp.sock, (struct sockaddr*)&addr, &len); /* Get the bound port */ - TEST_ASSERT_ERRNO(!err, errno); - tp.port = ntohs(addr.sin_port); - pni_snprintf(tp.str, sizeof(tp.str), "%d", tp.port); - test_port_use_host(&tp, host); - return tp; -} - #define ARRAYLEN(A) (sizeof(A)/sizeof((A)[0])) /* Proactor and handler that take part in a test */ @@ -216,14 +144,9 @@ typedef struct test_listener_t { test_listener_t test_listen(test_proactor_t *tp, const char *host) { test_listener_t l = { test_port(host), pn_listener() }; -#if defined(_WIN32) - sock_close(l.port.sock); // small chance another process will steal the port in Windows -#endif pn_proactor_listen(tp->proactor, l.listener, l.port.host_port, 4); TEST_ETYPE_EQUAL(tp->handler.t, PN_LISTENER_OPEN, test_proactors_run(tp, 1)); -#if !defined(_WIN32) - sock_close(l.port.sock); -#endif + test_port_close(&l.port); return l; } @@ -620,7 +543,7 @@ static void test_errors(test_t *t) { TEST_ETYPE_EQUAL(t, PN_PROACTOR_INACTIVE, TEST_PROACTORS_RUN(tps)); } - sock_close(port.sock); + test_port_close(&port); TEST_PROACTORS_DESTROY(tps); } @@ -932,15 +855,6 @@ static void test_parse_addr(test_t *t) { /* Test pn_proactor_addr funtions */ -/* Windows will need winsock2.h etc. - These headers are *only* needed for test_netaddr and only for the getnameinfo part. - This is the only non-portable part of the proactor test suite. - */ -#if !defined(_WIN32) -#include <sys/socket.h> /* For socket_storage */ -#include <netdb.h> /* For NI_MAXHOST/NI_MAXSERV */ -#endif - static void test_netaddr(test_t *t) { test_proactor_t tps[] ={ test_proactor(t, open_wake_handler), test_proactor(t, listen_handler) }; pn_proactor_t *client = tps[0].proactor; @@ -975,8 +889,8 @@ static void test_netaddr(test_t *t) { const pn_netaddr_t *na = pn_netaddr_remote(ct); const struct sockaddr *sa = pn_netaddr_sockaddr(na); TEST_CHECK(t, AF_INET == sa->sa_family); - char host[NI_MAXHOST] = ""; - char serv[NI_MAXSERV] = ""; + char host[TEST_PORT_MAX_STR] = ""; + char serv[TEST_PORT_MAX_STR] = ""; int err = getnameinfo(sa, pn_netaddr_socklen(na), host, sizeof(host), serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV); http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/485cdbd3/proton-c/src/tests/test_port.h ---------------------------------------------------------------------- diff --git a/proton-c/src/tests/test_port.h b/proton-c/src/tests/test_port.h new file mode 100644 index 0000000..85a3fd7 --- /dev/null +++ b/proton-c/src/tests/test_port.h @@ -0,0 +1,138 @@ +#ifndef TESTS_TEST_PORT_H +#define TESTS_TEST_PORT_H + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/* Some simple platform-secifics to acquire an unused socket */ + +#include <stdio.h> +#include <stdlib.h> +#include <string.h> + +#if defined(_WIN32) + +# include <winsock2.h> +# include <ws2tcpip.h> + +typedef SOCKET sock_t; + +static void test_snprintf(char *buf, size_t count, const char *fmt, ...) { + va_list ap; + va_start(ap, fmt); + _vsnprintf(buf, count, fmt, ap); + buf[count-1] = '\0'; /* _vsnprintf doesn't null-terminate on overflow */ +} + +void check_err(int ret, const char *what) { + if (ret) { + char buf[512]; + FormatMessage( + FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, + NULL, WSAGetLastError(), NULL, buf, sizeof(buf), NULL); + fprintf(stderr, "%s: %s\n", what, buf); + abort(); + } +} + +#else /* POSIX */ + +# include <sys/types.h> +# include <sys/socket.h> +# include <netinet/in.h> +# include <unistd.h> +# include <netdb.h> +# define test_snprintf snprintf + +typedef int sock_t; + +void check_err(int ret, const char *what) { + if (ret) { + perror(what); abort(); + } +} + +#endif + +#define TEST_PORT_MAX_STR 1060 + +/* Combines a sock_t with the int and char* versions of the port for convenience */ +typedef struct test_port_t { + sock_t sock; + int port; /* port as integer */ + char str[TEST_PORT_MAX_STR]; /* port as string */ + char host_port[TEST_PORT_MAX_STR]; /* host:port string */ +} test_port_t; + +/* Modifies tp->host_port to use host, returns the new tp->host_port */ +const char *test_port_use_host(test_port_t *tp, const char *host) { + test_snprintf(tp->host_port, sizeof(tp->host_port), "%s:%d", host, tp->port); + return tp->host_port; +} + +/* Create a socket and bind(INADDR_LOOPBACK:0) to get a free port. + Set socket options so the port can be bound and used for listen() within this process, + even though it is bound to the test_port socket. + Use host to create the host_port address string. +*/ +test_port_t test_port(const char* host) { +#ifdef _WIN32 + static int wsa_started = 0; + if (!wsa_started) { + WORD wsa_ver = MAKEWORD(2, 2); + WSADATA unused; + check_err(WSAStartup(wsa_ver, &unused), "WSAStartup"); + } +#endif + int err = 0; + test_port_t tp = {0}; + tp.sock = socket(AF_INET, SOCK_STREAM, 0); + check_err(tp.sock < 0, "socket"); + int on = 1; +#ifndef _WIN32 + check_err(setsockopt(tp.sock, SOL_SOCKET, SO_REUSEADDR, (const char*)&on, sizeof(on)), "setsockopt"); +#endif + struct sockaddr_in addr = {0}; + addr.sin_family = AF_INET; /* set the type of connection to TCP/IP */ + addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); + addr.sin_port = 0; /* bind to port 0 */ + err = bind(tp.sock, (struct sockaddr*)&addr, sizeof(addr)); + check_err(err, "bind"); + socklen_t len = sizeof(addr); + err = getsockname(tp.sock, (struct sockaddr*)&addr, &len); /* Get the bound port */ + check_err(err, "getsockname"); + tp.port = ntohs(addr.sin_port); + test_snprintf(tp.str, sizeof(tp.str), "%d", tp.port); + test_port_use_host(&tp, host); +#ifdef _WIN32 /* Windows doesn't support the twice-open socket trick */ + closesocket(tp.sock); +#endif + return tp; +} + +void test_port_close(test_port_t *tp) { +#ifdef _WIN32 + WSACleanup(); +#else + close(tp->sock); +#endif +} + + +#endif // TESTS_TEST_PORT_H --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@qpid.apache.org For additional commands, e-mail: commits-h...@qpid.apache.org