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

Reply via email to