Merge pull request #149 from murgatroid99/node_bind_port_zero

Bind servers to port 0 in node tests

include/grpc/grpc.h

@@ -428,7 +428,8 @@ grpc_server *grpc_server_create(grpc_completion_queue *cq,
    REQUIRES: server not started */
 int grpc_server_add_http2_port(grpc_server *server, const char *addr);
 
-/* Add a secure port to server; returns 1 on success, 0 on failure
+/* Add a secure port to server.
+   Returns bound port number on success, 0 on failure.
    REQUIRES: server not started */
 int grpc_server_add_secure_http2_port(grpc_server *server, const char *addr);
 

src/core/security/server_secure_chttp2.c

@@ -93,6 +93,8 @@ int grpc_server_add_secure_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, "https");
   if (!resolved) {
@@ -105,9 +107,15 @@ int grpc_server_add_secure_http2_port(grpc_server *server, const char *addr) {
   }
 
   for (i = 0; i < resolved->naddrs; i++) {
-    if (grpc_tcp_server_add_port(tcp,
+    port_temp = grpc_tcp_server_add_port(
-                                 (struct sockaddr *)&resolved->addrs[i].addr,
+        tcp, (struct sockaddr *)&resolved->addrs[i].addr,
-                                 resolved->addrs[i].len)) {
+        resolved->addrs[i].len);
+    if (port_temp >= 0) {
+      if (port_num == -1) {
+        port_num = port_temp;
+      } else {
+        GPR_ASSERT(port_num == port_temp);
+      }
       count++;
     }
   }
@@ -125,7 +133,7 @@ int grpc_server_add_secure_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:

src/node/interop/interop_server.js

@@ -157,7 +157,8 @@ function handleHalfDuplex(call) {
  * Get a server object bound to the given port
  * @param {string} port Port to which to bind
  * @param {boolean} tls Indicates that the bound port should use TLS
- * @return {Server} Server object bound to the support
+ * @return {{server: Server, port: number}} Server object bound to the support,
+ *     and port number that the server is bound to
  */
 function getServer(port, tls) {
   // TODO(mlumish): enable TLS functionality
@@ -183,8 +184,8 @@ function getServer(port, tls) {
       halfDuplexCall: handleHalfDuplex
     }
   }, options);
-  server.bind('0.0.0.0:' + port, tls);
+  var port_num = server.bind('0.0.0.0:' + port, tls);
-  return server;
+  return {server: server, port: port_num};
 }
 
 if (require.main === module) {
@@ -192,8 +193,8 @@ if (require.main === module) {
   var argv = parseArgs(process.argv, {
     string: ['port', 'use_tls']
   });
-  var server = getServer(argv.port, argv.use_tls === 'true');
+  var server_obj = getServer(argv.port, argv.use_tls === 'true');
-  server.start();
+  server_obj.server.start();
 }
 
 /**

src/node/port_picker.js

@@ -1,52 +0,0 @@
-/*
- *
- * Copyright 2014, Google Inc.
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are
- * met:
- *
- *     * Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- *     * Redistributions in binary form must reproduce the above
- * copyright notice, this list of conditions and the following disclaimer
- * in the documentation and/or other materials provided with the
- * distribution.
- *     * Neither the name of Google Inc. nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- *
- */
-
-var net = require('net');
-
-/**
- * Finds a free port that a server can bind to, in the format
- * "address:port"
- * @param {function(string)} cb The callback that should execute when the port
- *     is available
- */
-function nextAvailablePort(cb) {
-  var server = net.createServer();
-  server.listen(function() {
-    var address = server.address();
-    server.close(function() {
-      cb(address.address + ':' + address.port.toString());
-    });
-  });
-}
-
-exports.nextAvailablePort = nextAvailablePort;

src/node/server.cc

@@ -194,7 +194,7 @@ NAN_METHOD(Server::AddHttp2Port) {
     return NanThrowTypeError("addHttp2Port's argument must be a String");
   }
   Server *server = ObjectWrap::Unwrap<Server>(args.This());
-  NanReturnValue(NanNew<Boolean>(grpc_server_add_http2_port(
+  NanReturnValue(NanNew<Number>(grpc_server_add_http2_port(
       server->wrapped_server, *NanUtf8String(args[0]))));
 }
 
@@ -208,7 +208,7 @@ NAN_METHOD(Server::AddSecureHttp2Port) {
     return NanThrowTypeError("addSecureHttp2Port's argument must be a String");
   }
   Server *server = ObjectWrap::Unwrap<Server>(args.This());
-  NanReturnValue(NanNew<Boolean>(grpc_server_add_secure_http2_port(
+  NanReturnValue(NanNew<Number>(grpc_server_add_secure_http2_port(
       server->wrapped_server, *NanUtf8String(args[0]))));
 }
 

src/node/server.js

@@ -256,9 +256,9 @@ Server.prototype.register = function(name, handler) {
  */
 Server.prototype.bind = function(port, secure) {
   if (secure) {
-    this._server.addSecureHttp2Port(port);
+    return this._server.addSecureHttp2Port(port);
   } else {
-    this._server.addHttp2Port(port);
+    return this._server.addHttp2Port(port);
   }
 };
 

src/node/surface_server.js

@@ -357,8 +357,7 @@ function makeServerConstructor(services) {
    * @return {SurfaceServer} this
    */
   SurfaceServer.prototype.bind = function(port, secure) {
-    this.inner_server.bind(port, secure);
+    return this.inner_server.bind(port, secure);
-    return this;
   };
 
   /**

src/node/test/client_server_test.js

@@ -37,7 +37,6 @@ var path = require('path');
 var grpc = require('bindings')('grpc.node');
 var Server = require('../server');
 var client = require('../client');
-var port_picker = require('../port_picker');
 var common = require('../common');
 var _ = require('highland');
 
@@ -80,14 +79,13 @@ function errorHandler(stream) {
 
 describe('echo client', function() {
   it('should receive echo responses', function(done) {
-    port_picker.nextAvailablePort(function(port) {
     var server = new Server();
-      server.bind(port);
+    var port_num = server.bind('0.0.0.0:0');
     server.register('echo', echoHandler);
     server.start();
 
     var messages = ['echo1', 'echo2', 'echo3', 'echo4'];
-      var channel = new grpc.Channel(port);
+    var channel = new grpc.Channel('localhost:' + port_num);
     var stream = client.makeRequest(
         channel,
         'echo');
@@ -104,15 +102,13 @@ describe('echo client', function() {
       done();
     });
   });
-  });
   it('should get an error status that the server throws', function(done) {
-    port_picker.nextAvailablePort(function(port) {
     var server = new Server();
-      server.bind(port);
+    var port_num = server.bind('0.0.0.0:0');
     server.register('error', errorHandler);
     server.start();
 
-      var channel = new grpc.Channel(port);
+    var channel = new grpc.Channel('localhost:' + port_num);
     var stream = client.makeRequest(
         channel,
         'error',
@@ -128,15 +124,12 @@ describe('echo client', function() {
       server.shutdown();
       done();
     });
-
-    });
   });
 });
 /* TODO(mlumish): explore options for reducing duplication between this test
  * and the insecure echo client test */
 describe('secure echo client', function() {
   it('should recieve echo responses', function(done) {
-    port_picker.nextAvailablePort(function(port) {
     fs.readFile(ca_path, function(err, ca_data) {
       assert.ifError(err);
       fs.readFile(key_path, function(err, key_data) {
@@ -149,12 +142,12 @@ describe('secure echo client', function() {
                                                               pem_data);
 
           var server = new Server({'credentials' : server_creds});
-            server.bind(port, true);
+          var port_num = server.bind('0.0.0.0:0', true);
           server.register('echo', echoHandler);
           server.start();
 
           var messages = ['echo1', 'echo2', 'echo3', 'echo4'];
-            var channel = new grpc.Channel(port, {
+          var channel = new grpc.Channel('localhost:' + port_num, {
             'grpc.ssl_target_name_override' : 'foo.test.google.com',
             'credentials' : creds
           });
@@ -175,8 +168,6 @@ describe('secure echo client', function() {
             done();
           });
         });
-
-        });
       });
     });
   });

src/node/test/end_to_end_test.js

@@ -33,7 +33,6 @@
 
 var assert = require('assert');
 var grpc = require('bindings')('grpc.node');
-var port_picker = require('../port_picker');
 
 /**
  * This is used for testing functions with multiple asynchronous calls that
@@ -58,14 +57,13 @@ function multiDone(done, count) {
 
 describe('end-to-end', function() {
   it('should start and end a request without error', function(complete) {
-    port_picker.nextAvailablePort(function(port) {
     var server = new grpc.Server();
     var done = multiDone(function() {
       complete();
       server.shutdown();
     }, 2);
-      server.addHttp2Port(port);
+    var port_num = server.addHttp2Port('0.0.0.0:0');
-      var channel = new grpc.Channel(port);
+    var channel = new grpc.Channel('localhost:' + port_num);
     var deadline = new Date();
     deadline.setSeconds(deadline.getSeconds() + 3);
     var status_text = 'xyz';
@@ -112,10 +110,8 @@ describe('end-to-end', function() {
           });
     });
   });
-  });
 
   it('should send and receive data without error', function(complete) {
-    port_picker.nextAvailablePort(function(port) {
     var req_text = 'client_request';
     var reply_text = 'server_response';
     var server = new grpc.Server();
@@ -123,8 +119,8 @@ describe('end-to-end', function() {
       complete();
       server.shutdown();
     }, 6);
-      server.addHttp2Port(port);
+    var port_num = server.addHttp2Port('0.0.0.0:0');
-      var channel = new grpc.Channel(port);
+    var channel = new grpc.Channel('localhost:' + port_num);
     var deadline = new Date();
     deadline.setSeconds(deadline.getSeconds() + 3);
     var status_text = 'success';
@@ -197,5 +193,4 @@ describe('end-to-end', function() {
       });
     });
   });
-  });
 });

src/node/test/interop_sanity_test.js

@@ -34,8 +34,6 @@
 var interop_server = require('../interop/interop_server.js');
 var interop_client = require('../interop/interop_client.js');
 
-var port_picker = require('../port_picker');
-
 var server;
 
 var port;
@@ -44,13 +42,12 @@ var name_override = 'foo.test.google.com';
 
 describe('Interop tests', function() {
   before(function(done) {
-    port_picker.nextAvailablePort(function(addr) {
+    var server_obj = interop_server.getServer(0, true);
-      server = interop_server.getServer(addr.substring(addr.indexOf(':') + 1), true);
+    server = server_obj.server;
     server.listen();
-      port = addr;
+    port = 'localhost:' + server_obj.port;
     done();
   });
-  });
   // This depends on not using a binary stream
   it.skip('should pass empty_unary', function(done) {
     interop_client.runTest(port, name_override, 'empty_unary', true, done);

src/node/test/math_client_test.js

@@ -32,7 +32,6 @@
  */
 
 var assert = require('assert');
-var port_picker = require('../port_picker');
 
 var grpc = require('..');
 var math = grpc.load(__dirname + '/../examples/math.proto').math;
@@ -50,12 +49,11 @@ var server = require('../examples/math_server.js');
 
 describe('Math client', function() {
   before(function(done) {
-    port_picker.nextAvailablePort(function(port) {
+    var port_num = server.bind('0.0.0.0:0');
-      server.bind(port).listen();
+    server.listen();
-      math_client = new math.Math(port);
+    math_client = new math.Math('localhost:' + port_num);
     done();
   });
-  });
   after(function() {
     server.shutdown();
   });

src/node/test/server_test.js

@@ -34,7 +34,6 @@
 var assert = require('assert');
 var grpc = require('bindings')('grpc.node');
 var Server = require('../server');
-var port_picker = require('../port_picker');
 
 /**
  * This is used for testing functions with multiple asynchronous calls that
@@ -68,16 +67,15 @@ function echoHandler(stream) {
 describe('echo server', function() {
   it('should echo inputs as responses', function(done) {
     done = multiDone(done, 4);
-    port_picker.nextAvailablePort(function(port) {
     var server = new Server();
-      server.bind(port);
+    var port_num = server.bind('[::]:0');
     server.register('echo', echoHandler);
     server.start();
 
     var req_text = 'echo test string';
     var status_text = 'OK';
 
-      var channel = new grpc.Channel(port);
+    var channel = new grpc.Channel('localhost:' + port_num);
     var deadline = new Date();
     deadline.setSeconds(deadline.getSeconds() + 3);
     var call = new grpc.Call(channel,
@@ -117,5 +115,4 @@ describe('echo server', function() {
       done();
     }, 0);
   });
-  });
 });