Return bound port number from grpc_server_add_http2_port.

Allows tests to bind to port 0 and still have clients connect to them.
	Change on 2015/01/12 by ctiller <ctiller@google.com>
-------------
Created by MOE: http://code.google.com/p/moe-java
MOE_MIGRATED_REVID=83800669

include/grpc/grpc.h

@@ -428,7 +428,8 @@ grpc_call_error grpc_server_request_call(grpc_server *server, void *tag_new);
 grpc_server *grpc_server_create(grpc_completion_queue *cq,
                                 const grpc_channel_args *args);
 
-/* Add a http2 over tcp listener; returns 1 on success, 0 on failure
+/* Add a http2 over tcp listener.
+   Returns bound port number on success, 0 on failure.
    REQUIRES: server not started */
 int grpc_server_add_http2_port(grpc_server *server, const char *addr);
 

src/core/iomgr/sockaddr_utils.c

@@ -153,3 +153,31 @@ int grpc_sockaddr_to_string(char **out, const struct sockaddr *addr,
   errno = save_errno;
   return ret;
 }
+
+int grpc_sockaddr_get_port(const struct sockaddr *addr) {
+  switch (addr->sa_family) {
+    case AF_INET:
+      return ntohs(((struct sockaddr_in *)addr)->sin_port);
+    case AF_INET6:
+      return ntohs(((struct sockaddr_in6 *)addr)->sin6_port);
+    default:
+      gpr_log(GPR_ERROR, "Unknown socket family %d in %s", addr->sa_family,
+              __FUNCTION__);
+      return 0;
+  }
+}
+
+int grpc_sockaddr_set_port(const struct sockaddr *addr, int port) {
+  switch (addr->sa_family) {
+    case AF_INET:
+      ((struct sockaddr_in *)addr)->sin_port = htons(port);
+      return 1;
+    case AF_INET6:
+      ((struct sockaddr_in6 *)addr)->sin6_port = htons(port);
+      return 1;
+    default:
+      gpr_log(GPR_ERROR, "Unknown socket family %d in %s", addr->sa_family,
+              __FUNCTION__);
+      return 0;
+  }
+}

src/core/iomgr/sockaddr_utils.h

@@ -57,6 +57,12 @@ int grpc_sockaddr_is_wildcard(const struct sockaddr *addr, int *port_out);
 void grpc_sockaddr_make_wildcards(int port, struct sockaddr_in *wild4_out,
                                   struct sockaddr_in6 *wild6_out);
 
+/* Return the IP port number of a sockaddr */
+int grpc_sockaddr_get_port(const struct sockaddr *addr);
+
+/* Set IP port number of a sockaddr */
+int grpc_sockaddr_set_port(const struct sockaddr *addr, int port);
+
 /* Converts a sockaddr into a newly-allocated human-readable string.
 
    Currently, only the AF_INET and AF_INET6 families are recognized.

src/core/iomgr/tcp_server.h

@@ -52,7 +52,8 @@ grpc_tcp_server *grpc_tcp_server_create();
 void grpc_tcp_server_start(grpc_tcp_server *server, grpc_pollset *pollset,
                            grpc_tcp_server_cb cb, void *cb_arg);
 
-/* Add a port to the server, returning true on success, or false otherwise.
+/* Add a port to the server, returning port number on success, or negative
+   on failure.
 
    The :: and 0.0.0.0 wildcard addresses are treated identically, accepting
    both IPv4 and IPv6 connections, but :: is the preferred style.  This usually
@@ -60,6 +61,8 @@ void grpc_tcp_server_start(grpc_tcp_server *server, grpc_pollset *pollset,
    but not dualstack sockets.
 
    For raw access to the underlying sockets, see grpc_tcp_server_get_fd(). */
+/* TODO(ctiller): deprecate this, and make grpc_tcp_server_add_ports to handle
+                  all of the multiple socket port matching logic in one place */
 int grpc_tcp_server_add_port(grpc_tcp_server *s, const struct sockaddr *addr,
                              int addr_len);
 

src/core/iomgr/tcp_server_posix.c

@@ -154,6 +154,9 @@ static int get_max_accept_queue_size() {
 
 /* Prepare a recently-created socket for listening. */
 static int prepare_socket(int fd, const struct sockaddr *addr, int addr_len) {
+  struct sockaddr_storage sockname_temp;
+  socklen_t sockname_len;
+
   if (fd < 0) {
     goto error;
   }
@@ -179,13 +182,18 @@ static int prepare_socket(int fd, const struct sockaddr *addr, int addr_len) {
     goto error;
   }
 
-  return 1;
+  sockname_len = sizeof(sockname_temp);
+  if (getsockname(fd, (struct sockaddr *)&sockname_temp, &sockname_len) < 0) {
+    goto error;
+  }
+
+  return grpc_sockaddr_get_port((struct sockaddr *)&sockname_temp);
 
 error:
   if (fd >= 0) {
     close(fd);
   }
-  return 0;
+  return -1;
 }
 
 /* event manager callback when reads are ready */
@@ -234,39 +242,64 @@ error:
 static int add_socket_to_server(grpc_tcp_server *s, int fd,
                                 const struct sockaddr *addr, int addr_len) {
   server_port *sp;
+  int port;
 
-  if (!prepare_socket(fd, addr, addr_len)) {
-    return 0;
-  }
-
-  gpr_mu_lock(&s->mu);
-  GPR_ASSERT(!s->cb && "must add ports before starting server");
-  /* append it to the list under a lock */
-  if (s->nports == s->port_capacity) {
-    s->port_capacity *= 2;
-    s->ports = gpr_realloc(s->ports, sizeof(server_port *) * s->port_capacity);
+  port = prepare_socket(fd, addr, addr_len);
+  if (port >= 0) {
+    gpr_mu_lock(&s->mu);
+    GPR_ASSERT(!s->cb && "must add ports before starting server");
+    /* append it to the list under a lock */
+    if (s->nports == s->port_capacity) {
+      s->port_capacity *= 2;
+      s->ports =
+          gpr_realloc(s->ports, sizeof(server_port *) * s->port_capacity);
+    }
+    sp = &s->ports[s->nports++];
+    sp->server = s;
+    sp->fd = fd;
+    sp->emfd = grpc_fd_create(fd);
+    GPR_ASSERT(sp->emfd);
+    gpr_mu_unlock(&s->mu);
   }
-  sp = &s->ports[s->nports++];
-  sp->server = s;
-  sp->fd = fd;
-  sp->emfd = grpc_fd_create(fd);
-  GPR_ASSERT(sp->emfd);
-  gpr_mu_unlock(&s->mu);
 
-  return 1;
+  return port;
 }
 
 int grpc_tcp_server_add_port(grpc_tcp_server *s, const struct sockaddr *addr,
                              int addr_len) {
-  int ok = 0;
+  int allocated_port1 = -1;
+  int allocated_port2 = -1;
+  int i;
   int fd;
   grpc_dualstack_mode dsmode;
   struct sockaddr_in6 addr6_v4mapped;
   struct sockaddr_in wild4;
   struct sockaddr_in6 wild6;
   struct sockaddr_in addr4_copy;
+  struct sockaddr *allocated_addr = NULL;
+  struct sockaddr_storage sockname_temp;
+  socklen_t sockname_len;
   int port;
 
+  /* Check if this is a wildcard port, and if so, try to keep the port the same
+     as some previously created listener. */
+  if (grpc_sockaddr_get_port(addr) == 0) {
+    for (i = 0; i < s->nports; i++) {
+      sockname_len = sizeof(sockname_temp);
+      if (0 == getsockname(s->ports[i].fd, (struct sockaddr *)&sockname_temp,
+                           &sockname_len)) {
+        port = grpc_sockaddr_get_port((struct sockaddr *)&sockname_temp);
+        if (port > 0) {
+          allocated_addr = malloc(addr_len);
+          memcpy(allocated_addr, addr, addr_len);
+          grpc_sockaddr_set_port(allocated_addr, port);
+          addr = allocated_addr;
+          break;
+        }
+      }
+    }
+  }
+
   if (grpc_sockaddr_to_v4mapped(addr, &addr6_v4mapped)) {
     addr = (const struct sockaddr *)&addr6_v4mapped;
     addr_len = sizeof(addr6_v4mapped);
@@ -280,12 +313,15 @@ int grpc_tcp_server_add_port(grpc_tcp_server *s, const struct sockaddr *addr,
     addr = (struct sockaddr *)&wild6;
     addr_len = sizeof(wild6);
     fd = grpc_create_dualstack_socket(addr, SOCK_STREAM, 0, &dsmode);
-    ok |= add_socket_to_server(s, fd, addr, addr_len);
+    allocated_port1 = add_socket_to_server(s, fd, addr, addr_len);
     if (fd >= 0 && dsmode == GRPC_DSMODE_DUALSTACK) {
-      return ok;
+      goto done;
     }
 
     /* If we didn't get a dualstack socket, also listen on 0.0.0.0. */
+    if (port == 0 && allocated_port1 > 0) {
+      grpc_sockaddr_set_port((struct sockaddr *)&wild4, allocated_port1);
+    }
     addr = (struct sockaddr *)&wild4;
     addr_len = sizeof(wild4);
   }
@@ -299,8 +335,11 @@ int grpc_tcp_server_add_port(grpc_tcp_server *s, const struct sockaddr *addr,
     addr = (struct sockaddr *)&addr4_copy;
     addr_len = sizeof(addr4_copy);
   }
-  ok |= add_socket_to_server(s, fd, addr, addr_len);
-  return ok;
+  allocated_port2 = add_socket_to_server(s, fd, addr, addr_len);
+
+done:
+  gpr_free(allocated_addr);
+  return allocated_port1 >= 0 ? allocated_port1 : allocated_port2;
 }
 
 int grpc_tcp_server_get_fd(grpc_tcp_server *s, int index) {

src/core/surface/server_chttp2.c

@@ -76,6 +76,8 @@ int grpc_server_add_http2_port(grpc_server *server, const char *addr) {
   grpc_tcp_server *tcp = NULL;
   size_t i;
   int count = 0;
+  int port_num = -1;
+  int port_temp;
 
   resolved = grpc_blocking_resolve_address(addr, "http");
   if (!resolved) {
@@ -88,9 +90,15 @@ int grpc_server_add_http2_port(grpc_server *server, const char *addr) {
   }
 
   for (i = 0; i < resolved->naddrs; i++) {
-    if (grpc_tcp_server_add_port(tcp,
-                                 (struct sockaddr *)&resolved->addrs[i].addr,
-                                 resolved->addrs[i].len)) {
+    port_temp = grpc_tcp_server_add_port(
+        tcp, (struct sockaddr *)&resolved->addrs[i].addr,
+        resolved->addrs[i].len);
+    if (port_temp >= 0) {
+      if (port_num == -1) {
+        port_num = port_temp;
+      } else {
+        GPR_ASSERT(port_num == port_temp);
+      }
       count++;
     }
   }
@@ -108,7 +116,7 @@ int grpc_server_add_http2_port(grpc_server *server, const char *addr) {
   /* Register with the server only upon success */
   grpc_server_add_listener(server, tcp, start, destroy);
 
-  return 1;
+  return port_num;
 
 /* Error path: cleanup and return */
 error:

test/core/end2end/dualstack_socket_test.c

@@ -73,26 +73,35 @@ void test_connect(const char *server_host, const char *client_host, int port,
   cq_verifier *v_client;
   cq_verifier *v_server;
   gpr_timespec deadline;
+  int got_port;
 
-  gpr_join_host_port(&client_hostport, client_host, port);
   gpr_join_host_port(&server_hostport, server_host, port);
-  gpr_log(GPR_INFO, "Testing with server=%s client=%s (expecting %s)",
-          server_hostport, client_hostport, expect_ok ? "success" : "failure");
 
   /* Create server. */
   server_cq = grpc_completion_queue_create();
   server = grpc_server_create(server_cq, NULL);
-  GPR_ASSERT(grpc_server_add_http2_port(server, server_hostport));
+  GPR_ASSERT((got_port = grpc_server_add_http2_port(server, server_hostport)) >
+             0);
+  if (port == 0) {
+    port = got_port;
+  } else {
+    GPR_ASSERT(port == got_port);
+  }
   grpc_server_start(server);
-  gpr_free(server_hostport);
   v_server = cq_verifier_create(server_cq);
 
   /* Create client. */
+  gpr_join_host_port(&client_hostport, client_host, port);
   client_cq = grpc_completion_queue_create();
   client = grpc_channel_create(client_hostport, NULL);
-  gpr_free(client_hostport);
   v_client = cq_verifier_create(client_cq);
 
+  gpr_log(GPR_INFO, "Testing with server=%s client=%s (expecting %s)",
+          server_hostport, client_hostport, expect_ok ? "success" : "failure");
+
+  gpr_free(client_hostport);
+  gpr_free(server_hostport);
+
   if (expect_ok) {
     /* Normal deadline, shouldn't be reached. */
     deadline = ms_from_now(60000);
@@ -170,8 +179,7 @@ void test_connect(const char *server_host, const char *client_host, int port,
 
 int main(int argc, char **argv) {
   int do_ipv6 = 1;
-  int i;
-  int port = grpc_pick_unused_port_or_die();
+  int fixed_port;
 
   grpc_test_init(argc, argv);
   grpc_init();
@@ -181,30 +189,34 @@ int main(int argc, char **argv) {
     do_ipv6 = 0;
   }
 
-  for (i = 0; i <= 1; i++) {
+  for (fixed_port = 0; fixed_port <= 1; fixed_port++) {
+    int port = fixed_port ? grpc_pick_unused_port_or_die() : 0;
+
     /* For coverage, test with and without dualstack sockets. */
-    grpc_forbid_dualstack_sockets_for_testing = i;
-
-    /* :: and 0.0.0.0 are handled identically. */
-    test_connect("::", "127.0.0.1", port, 1);
-    test_connect("::", "::ffff:127.0.0.1", port, 1);
-    test_connect("::", "localhost", port, 1);
-    test_connect("0.0.0.0", "127.0.0.1", port, 1);
-    test_connect("0.0.0.0", "::ffff:127.0.0.1", port, 1);
-    test_connect("0.0.0.0", "localhost", port, 1);
-    if (do_ipv6) {
-      test_connect("::", "::1", port, 1);
-      test_connect("0.0.0.0", "::1", port, 1);
-    }
+    for (grpc_forbid_dualstack_sockets_for_testing = 0;
+         grpc_forbid_dualstack_sockets_for_testing <= 1;
+         grpc_forbid_dualstack_sockets_for_testing++) {
+      /* :: and 0.0.0.0 are handled identically. */
+      test_connect("::", "127.0.0.1", port, 1);
+      test_connect("::", "::ffff:127.0.0.1", port, 1);
+      test_connect("::", "localhost", port, 1);
+      test_connect("0.0.0.0", "127.0.0.1", port, 1);
+      test_connect("0.0.0.0", "::ffff:127.0.0.1", port, 1);
+      test_connect("0.0.0.0", "localhost", port, 1);
+      if (do_ipv6) {
+        test_connect("::", "::1", port, 1);
+        test_connect("0.0.0.0", "::1", port, 1);
+      }
+
+      /* These only work when the families agree. */
+      test_connect("127.0.0.1", "127.0.0.1", port, 1);
+      if (do_ipv6) {
+        test_connect("::1", "::1", port, 1);
+        test_connect("::1", "127.0.0.1", port, 0);
+        test_connect("127.0.0.1", "::1", port, 0);
+      }
 
-    /* These only work when the families agree. */
-    test_connect("127.0.0.1", "127.0.0.1", port, 1);
-    if (do_ipv6) {
-      test_connect("::1", "::1", port, 1);
-      test_connect("::1", "127.0.0.1", port, 0);
-      test_connect("127.0.0.1", "::1", port, 0);
     }
-
   }
 
   grpc_shutdown();