Merge remote-tracking branch 'google/v1.1.x'

11 files changed, 177 insertions, 76 deletions

diff --git a/src/core/lib/iomgr/resolve_address_uv.c b/src/core/lib/iomgr/resolve_address_uv.c
index 9b5f3209f0..79ff910738 100644
--- a/src/core/lib/iomgr/resolve_address_uv.c
+++ b/src/core/lib/iomgr/resolve_address_uv.c
@@ -113,14 +113,15 @@ static grpc_error *try_split_host_port(const char *name,
   /* parse name, splitting it into host and port parts */
   grpc_error *error;
   gpr_split_host_port(name, host, port);
-  if (host == NULL) {
+  if (*host == NULL) {
     char *msg;
     gpr_asprintf(&msg, "unparseable host:port: '%s'", name);
     error = GRPC_ERROR_CREATE(msg);
     gpr_free(msg);
     return error;
   }
-  if (port == NULL) {
+  if (*port == NULL) {
+    // TODO(murgatroid99): add tests for this case
     if (default_port == NULL) {
       char *msg;
       gpr_asprintf(&msg, "no port in name '%s'", name);
diff --git a/src/core/lib/iomgr/socket_utils_windows.c b/src/core/lib/iomgr/socket_utils_windows.c
index 628ad4a45b..5bbe8fa34c 100644
--- a/src/core/lib/iomgr/socket_utils_windows.c
+++ b/src/core/lib/iomgr/socket_utils_windows.c
@@ -41,8 +41,12 @@
 #include <grpc/support/log.h>
 
 const char *grpc_inet_ntop(int af, const void *src, char *dst, size_t size) {
+#ifdef GPR_WIN_INET_NTOP
+  return inet_ntop(af, src, dst, size);
+#else
   /* Windows InetNtopA wants a mutable ip pointer */
   return InetNtopA(af, (void *)src, dst, size);
+#endif /* GPR_WIN_INET_NTOP */
 }
 
 #endif /* GRPC_WINDOWS_SOCKETUTILS */
diff --git a/src/core/lib/iomgr/tcp_client_uv.c b/src/core/lib/iomgr/tcp_client_uv.c
index 5225a5402b..3de0795187 100644
--- a/src/core/lib/iomgr/tcp_client_uv.c
+++ b/src/core/lib/iomgr/tcp_client_uv.c
@@ -46,6 +46,8 @@
 #include "src/core/lib/iomgr/tcp_uv.h"
 #include "src/core/lib/iomgr/timer.h"
 
+extern int grpc_tcp_trace;
+
 typedef struct grpc_uv_tcp_connect {
   uv_connect_t connect_req;
   grpc_timer alarm;
@@ -70,6 +72,12 @@ static void uv_tc_on_alarm(grpc_exec_ctx *exec_ctx, void *acp,
                            grpc_error *error) {
   int done;
   grpc_uv_tcp_connect *connect = acp;
+  if (grpc_tcp_trace) {
+    const char *str = grpc_error_string(error);
+    gpr_log(GPR_DEBUG, "CLIENT_CONNECT: %s: on_alarm: error=%s",
+            connect->addr_name, str);
+    grpc_error_free_string(str);
+  }
   if (error == GRPC_ERROR_NONE) {
     /* error == NONE implies that the timer ran out, and wasn't cancelled. If
        it was cancelled, then the handler that cancelled it also should close
@@ -145,6 +153,12 @@ static void tcp_client_connect_impl(grpc_exec_ctx *exec_ctx,
   connect->resource_quota = resource_quota;
   uv_tcp_init(uv_default_loop(), connect->tcp_handle);
   connect->connect_req.data = connect;
+
+  if (grpc_tcp_trace) {
+    gpr_log(GPR_DEBUG, "CLIENT_CONNECT: %s: asynchronously connecting",
+            connect->addr_name);
+  }
+
   // TODO(murgatroid99): figure out what the return value here means
   uv_tcp_connect(&connect->connect_req, connect->tcp_handle,
                  (const struct sockaddr *)resolved_addr->addr,
diff --git a/src/csharp/README.md b/src/csharp/README.md
index 8468eb991e..a21b72f225 100644
--- a/src/csharp/README.md
+++ b/src/csharp/README.md
@@ -7,19 +7,19 @@ A C# implementation of gRPC.
 SUPPORTED PLATFORMS
 ------------------
 
+- [.NET Core](https://dotnet.github.io/) on Linux, Windows and Mac OS X 
 - .NET Framework 4.5+ (Windows)
-- [.NET Core](https://dotnet.github.io/) on Linux, Windows and Mac OS X (starting from version 1.0.1)
 - Mono 4+ on Linux, Windows and Mac OS X
 
-
 PREREQUISITES
 --------------
 
+When using gRPC C# under .NET Core you only need to [install .NET Core](https://www.microsoft.com/net/core).
+
 - Windows: .NET Framework 4.5+, Visual Studio 2013 or 2015
 - Linux: Mono 4+, MonoDevelop 5.9+ (with NuGet add-in installed)
 - Mac OS X: Xamarin Studio 5.9+
 
-
 HOW TO USE
 --------------
 
@@ -27,7 +27,7 @@ HOW TO USE
 
 - Open Visual Studio / MonoDevelop / Xamarin Studio and start a new project/solution.
 
-- Add the [Grpc](https://www.nuget.org/packages/Grpc/) NuGet package as a dependency (Project options -> Manage NuGet Packages).
+- Add the [Grpc](https://www.nuget.org/packages/Grpc/) NuGet package as a dependency (Project options -> Manage NuGet Packages). 
 
 - To be able to generate code from Protocol Buffer (`.proto`) file definitions, add the [Grpc.Tools](https://www.nuget.org/packages/Grpc.Tools/) NuGet package that contains Protocol Buffers compiler (_protoc_) and the gRPC _protoc_ plugin.
 
@@ -71,23 +71,10 @@ DOCUMENTATION
 - [Helloworld Example][]
 - [RouteGuide Tutorial][]
 
-CONTENTS
---------
-
-- ext:
-  The extension library that wraps C API to be more digestible by C#.
-- Grpc.Auth:
-  gRPC OAuth2/JWT support.
-- Grpc.Core:
-  The main gRPC C# library.
-- Grpc.Examples:
-  API examples for math.proto
-- Grpc.Examples.MathClient:
-  An example client that sends requests to math server.
-- Grpc.Examples.MathServer:
-  An example server that implements a simple math service.
-- Grpc.IntegrationTesting:
-  Cross-language gRPC implementation testing (interop testing).
+PERFORMANCE
+-----------
+
+For best gRPC C# performance, use [.NET Core](https://dotnet.github.io/) and the Server GC mode `"System.GC.Server": true` for your applications.
 
 THE NATIVE DEPENDENCY
 ---------------
diff --git a/src/node/ext/completion_queue_threadpool.cc b/src/node/ext/completion_queue_threadpool.cc
index 6302e7a103..4881542f2d 100644
--- a/src/node/ext/completion_queue_threadpool.cc
+++ b/src/node/ext/completion_queue_threadpool.cc
@@ -78,6 +78,8 @@ class CompletionQueueAsyncWorker : public Nan::AsyncWorker {
   void HandleErrorCallback();
 
  private:
+  static void TryAddWorker();
+
   grpc_event result;
 
   static grpc_completion_queue *queue;
@@ -118,20 +120,21 @@ void CompletionQueueAsyncWorker::Execute() {
 
 grpc_completion_queue *CompletionQueueAsyncWorker::GetQueue() { return queue; }
 
-void CompletionQueueAsyncWorker::Next() {
-#ifndef GRPC_UV
-  Nan::HandleScope scope;
-  if (current_threads < max_queue_threads) {
+void CompletionQueueAsyncWorker::TryAddWorker() {
+  if (current_threads < max_queue_threads && waiting_next_calls > 0) {
     current_threads += 1;
+    waiting_next_calls -= 1;
     CompletionQueueAsyncWorker *worker = new CompletionQueueAsyncWorker();
     Nan::AsyncQueueWorker(worker);
-  } else {
-    waiting_next_calls += 1;
   }
   GPR_ASSERT(current_threads <= max_queue_threads);
   GPR_ASSERT((current_threads == max_queue_threads) ||
              (waiting_next_calls == 0));
-#endif
+}
+
+void CompletionQueueAsyncWorker::Next() {
+  waiting_next_calls += 1;
+  TryAddWorker();
 }
 
 void CompletionQueueAsyncWorker::Init(Local<Object> exports) {
@@ -143,17 +146,8 @@ void CompletionQueueAsyncWorker::Init(Local<Object> exports) {
 
 void CompletionQueueAsyncWorker::HandleOKCallback() {
   Nan::HandleScope scope;
-  if (waiting_next_calls > 0) {
-    waiting_next_calls -= 1;
-    // Old worker removed, new worker added. current_threads += 0
-    CompletionQueueAsyncWorker *worker = new CompletionQueueAsyncWorker();
-    Nan::AsyncQueueWorker(worker);
-  } else {
-    current_threads -= 1;
-  }
-  GPR_ASSERT(current_threads <= max_queue_threads);
-  GPR_ASSERT((current_threads == max_queue_threads) ||
-             (waiting_next_calls == 0));
+  current_threads -= 1;
+  TryAddWorker();
   Nan::Callback *callback = GetTagCallback(result.tag);
   Local<Value> argv[] = {Nan::Null(), GetTagNodeValue(result.tag)};
   callback->Call(2, argv);
@@ -162,18 +156,9 @@ void CompletionQueueAsyncWorker::HandleOKCallback() {
 }
 
 void CompletionQueueAsyncWorker::HandleErrorCallback() {
-  if (waiting_next_calls > 0) {
-    waiting_next_calls -= 1;
-    // Old worker removed, new worker added. current_threads += 0
-    CompletionQueueAsyncWorker *worker = new CompletionQueueAsyncWorker();
-    Nan::AsyncQueueWorker(worker);
-  } else {
-    current_threads -= 1;
-  }
-  GPR_ASSERT(current_threads <= max_queue_threads);
-  GPR_ASSERT((current_threads == max_queue_threads) ||
-             (waiting_next_calls == 0));
   Nan::HandleScope scope;
+  current_threads -= 1;
+  TryAddWorker();
   Nan::Callback *callback = GetTagCallback(result.tag);
   Local<Value> argv[] = {Nan::Error(ErrorMessage())};
 
@@ -189,6 +174,7 @@ grpc_completion_queue *GetCompletionQueue() {
 }
 
 void CompletionQueueNext() {
+  gpr_log(GPR_DEBUG, "Called CompletionQueueNext");
   CompletionQueueAsyncWorker::Next();
 }
 
diff --git a/src/node/src/client.js b/src/node/src/client.js
index 134ef239c2..44081a3a6c 100644
--- a/src/node/src/client.js
+++ b/src/node/src/client.js
@@ -108,7 +108,7 @@ function _write(chunk, encoding, callback) {
        but passing an object that causes a serialization failure is a misuse
        of the API anyway, so that's OK. The primary purpose here is to give the
        programmer a useful error and to stop the stream properly */
-    this.call.cancelWithStatus(grpc.status.INTERNAL, "Serialization failure");
+    this.call.cancelWithStatus(grpc.status.INTERNAL, 'Serialization failure');
     callback(e);
   }
   if (_.isFinite(encoding)) {
@@ -831,13 +831,12 @@ exports.waitForClientReady = function(client, deadline, callback) {
  */
 exports.makeProtobufClientConstructor =  function(service, options) {
   var method_attrs = common.getProtobufServiceAttrs(service, options);
-  var deprecatedArgumentOrder = false;
-  if (options) {
-    deprecatedArgumentOrder = options.deprecatedArgumentOrder;
+  if (!options) {
+    options = {deprecatedArgumentOrder: false};
   }
   var Client = exports.makeClientConstructor(
       method_attrs, common.fullyQualifiedName(service),
-      deprecatedArgumentOrder);
+      options);
   Client.service = service;
   Client.service.grpc_options = options;
   return Client;
diff --git a/src/node/src/server.js b/src/node/src/server.js
index da9c6b2d7f..8a7eff507d 100644
--- a/src/node/src/server.js
+++ b/src/node/src/server.js
@@ -121,20 +121,20 @@ function sendUnaryResponse(call, value, serialize, metadata, flags) {
   if (metadata) {
     statusMetadata = metadata;
   }
-  status.metadata = statusMetadata._getCoreRepresentation();
-  if (!call.metadataSent) {
-    end_batch[grpc.opType.SEND_INITIAL_METADATA] =
-        (new Metadata())._getCoreRepresentation();
-    call.metadataSent = true;
-  }
   var message;
   try {
     message = serialize(value);
   } catch (e) {
     e.code = grpc.status.INTERNAL;
-    handleError(e);
+    handleError(call, e);
     return;
   }
+  status.metadata = statusMetadata._getCoreRepresentation();
+  if (!call.metadataSent) {
+    end_batch[grpc.opType.SEND_INITIAL_METADATA] =
+        (new Metadata())._getCoreRepresentation();
+    call.metadataSent = true;
+  }
   message.grpcWriteFlags = flags;
   end_batch[grpc.opType.SEND_MESSAGE] = message;
   end_batch[grpc.opType.SEND_STATUS_FROM_SERVER] = status;
@@ -280,11 +280,6 @@ function _write(chunk, encoding, callback) {
   /* jshint validthis: true */
   var batch = {};
   var self = this;
-  if (!this.call.metadataSent) {
-    batch[grpc.opType.SEND_INITIAL_METADATA] =
-        (new Metadata())._getCoreRepresentation();
-    this.call.metadataSent = true;
-  }
   var message;
   try {
     message = this.serialize(chunk);
@@ -293,6 +288,11 @@ function _write(chunk, encoding, callback) {
     callback(e);
     return;
   }
+  if (!this.call.metadataSent) {
+    batch[grpc.opType.SEND_INITIAL_METADATA] =
+        (new Metadata())._getCoreRepresentation();
+    this.call.metadataSent = true;
+  }
   if (_.isFinite(encoding)) {
     /* Attach the encoding if it is a finite number. This is the closest we
      * can get to checking that it is valid flags */
@@ -728,11 +728,17 @@ var defaultHandler = {
  *     method implementation for the provided service.
  */
 Server.prototype.addService = function(service, implementation) {
+  if (!_.isObjectLike(service) || !_.isObjectLike(implementation)) {
+    throw new Error('addService requires two objects as arguments');
+  }
+  if (_.keys(service).length === 0) {
+    throw new Error('Cannot add an empty service to a server');
+  }
   if (this.started) {
     throw new Error('Can\'t add a service to a started server.');
   }
   var self = this;
-  _.each(service, function(attrs, name) {
+  _.forOwn(service, function(attrs, name) {
     var method_type;
     if (attrs.requestStream) {
       if (attrs.responseStream) {
diff --git a/src/node/test/surface_test.js b/src/node/test/surface_test.js
index e429a3648b..2636ea85ac 100644
--- a/src/node/test/surface_test.js
+++ b/src/node/test/surface_test.js
@@ -607,6 +607,113 @@ describe('Client malformed response handling', function() {
     call.end();
   });
 });
+describe('Server serialization failure handling', function() {
+  function serializeFail(obj) {
+    throw new Error('Serialization failed');
+  }
+  var client;
+  var server;
+  before(function() {
+    var test_proto = ProtoBuf.loadProtoFile(__dirname + '/test_service.proto');
+    var test_service = test_proto.lookup('TestService');
+    var malformed_test_service = {
+      unary: {
+        path: '/TestService/Unary',
+        requestStream: false,
+        responseStream: false,
+        requestDeserialize: _.identity,
+        responseSerialize: serializeFail
+      },
+      clientStream: {
+        path: '/TestService/ClientStream',
+        requestStream: true,
+        responseStream: false,
+        requestDeserialize: _.identity,
+        responseSerialize: serializeFail
+      },
+      serverStream: {
+        path: '/TestService/ServerStream',
+        requestStream: false,
+        responseStream: true,
+        requestDeserialize: _.identity,
+        responseSerialize: serializeFail
+      },
+      bidiStream: {
+        path: '/TestService/BidiStream',
+        requestStream: true,
+        responseStream: true,
+        requestDeserialize: _.identity,
+        responseSerialize: serializeFail
+      }
+    };
+    server = new grpc.Server();
+    server.addService(malformed_test_service, {
+      unary: function(call, cb) {
+        cb(null, {});
+      },
+      clientStream: function(stream, cb) {
+        stream.on('data', function() {/* Ignore requests */});
+        stream.on('end', function() {
+          cb(null, {});
+        });
+      },
+      serverStream: function(stream) {
+        stream.write({});
+        stream.end();
+      },
+      bidiStream: function(stream) {
+        stream.on('data', function() {
+          // Ignore requests
+          stream.write({});
+        });
+        stream.on('end', function() {
+          stream.end();
+        });
+      }
+    });
+    var port = server.bind('localhost:0', server_insecure_creds);
+    var Client = surface_client.makeProtobufClientConstructor(test_service);
+    client = new Client('localhost:' + port, grpc.credentials.createInsecure());
+    server.start();
+  });
+  after(function() {
+    server.forceShutdown();
+  });
+  it('should get an INTERNAL status with a unary call', function(done) {
+    client.unary({}, function(err, data) {
+      assert(err);
+      assert.strictEqual(err.code, grpc.status.INTERNAL);
+      done();
+    });
+  });
+  it('should get an INTERNAL status with a client stream call', function(done) {
+    var call = client.clientStream(function(err, data) {
+      assert(err);
+      assert.strictEqual(err.code, grpc.status.INTERNAL);
+      done();
+    });
+    call.write({});
+    call.end();
+  });
+  it('should get an INTERNAL status with a server stream call', function(done) {
+    var call = client.serverStream({});
+    call.on('data', function(){});
+    call.on('error', function(err) {
+      assert.strictEqual(err.code, grpc.status.INTERNAL);
+      done();
+    });
+  });
+  it('should get an INTERNAL status with a bidi stream call', function(done) {
+    var call = client.bidiStream();
+    call.on('data', function(){});
+    call.on('error', function(err) {
+      assert.strictEqual(err.code, grpc.status.INTERNAL);
+      done();
+    });
+    call.write({});
+    call.end();
+  });
+});
 describe('Other conditions', function() {
   var test_service;
   var Client;
diff --git a/src/python/grpcio/grpc/_channel.py b/src/python/grpcio/grpc/_channel.py
index 691e96a322..a1ac80651e 100644
--- a/src/python/grpcio/grpc/_channel.py
+++ b/src/python/grpcio/grpc/_channel.py
@@ -39,7 +39,7 @@ from grpc import _grpcio_metadata
 from grpc._cython import cygrpc
 from grpc.framework.foundation import callable_util
 
-_USER_AGENT = 'Python-gRPC-{}'.format(_grpcio_metadata.__version__)
+_USER_AGENT = 'grpc-python/{}'.format(_grpcio_metadata.__version__)
 
 _EMPTY_FLAGS = 0
 _INFINITE_FUTURE = cygrpc.Timespec(float('+inf'))
diff --git a/src/python/grpcio_tests/tests/unit/_metadata_test.py b/src/python/grpcio_tests/tests/unit/_metadata_test.py
index 53fe7ba8aa..035d87e3cf 100644
--- a/src/python/grpcio_tests/tests/unit/_metadata_test.py
+++ b/src/python/grpcio_tests/tests/unit/_metadata_test.py
@@ -32,7 +32,7 @@ import unittest
 import weakref
 
 import grpc
-from grpc import _grpcio_metadata
+from grpc import _channel
 from grpc.framework.foundation import logging_pool
 
 from tests.unit import test_common
@@ -49,8 +49,6 @@ _UNARY_STREAM = '/test/UnaryStream'
 _STREAM_UNARY = '/test/StreamUnary'
 _STREAM_STREAM = '/test/StreamStream'
 
-_USER_AGENT = 'Python-gRPC-{}'.format(_grpcio_metadata.__version__)
-
 _CLIENT_METADATA = (('client-md-key', 'client-md-key'),
                     ('client-md-key-bin', b'\x00\x01'))
 
@@ -76,7 +74,7 @@ def validate_client_metadata(test, servicer_context):
             _CLIENT_METADATA, servicer_context.invocation_metadata()))
     test.assertTrue(
         user_agent(servicer_context.invocation_metadata())
-        .startswith('primary-agent ' + _USER_AGENT))
+        .startswith('primary-agent ' + _channel._USER_AGENT))
     test.assertTrue(
         user_agent(servicer_context.invocation_metadata())
         .endswith('secondary-agent'))
diff --git a/src/ruby/ext/grpc/extconf.rb b/src/ruby/ext/grpc/extconf.rb
index 66c54aa3e0..b379664bab 100644
--- a/src/ruby/ext/grpc/extconf.rb
+++ b/src/ruby/ext/grpc/extconf.rb
@@ -102,7 +102,6 @@ $CFLAGS << ' -std=c99 '
 $CFLAGS << ' -Wall '
 $CFLAGS << ' -Wextra '
 $CFLAGS << ' -pedantic '
-$CFLAGS << ' -Werror '
 $CFLAGS << ' -Wno-format '
 
 output = File.join('grpc', 'grpc_c')