Merge pull request #10500 from jtattermusch/upmerge_v12x

Upmerge from upstream/v1.2.x to master

BUILD

@@ -41,7 +41,7 @@ g_stands_for = "green"
 
 core_version = "3.0.0-dev"
 
-version = "1.2.0"
+version = "1.3.0-dev"
 
 grpc_cc_library(
     name = "gpr",
@@ -67,6 +67,7 @@ grpc_cc_library(
         "grpc_lb_policy_pick_first",
         "grpc_lb_policy_round_robin",
         "grpc_load_reporting",
+        "grpc_max_age_filter",
         "grpc_resolver_dns_ares",
         "grpc_resolver_dns_native",
         "grpc_resolver_sockaddr",
@@ -75,7 +76,6 @@ grpc_cc_library(
         "grpc_transport_chttp2_client_secure",
         "grpc_transport_chttp2_server_insecure",
         "grpc_transport_chttp2_server_secure",
-        "grpc_max_age_filter",
     ],
 )
 
@@ -109,11 +109,11 @@ grpc_cc_library(
         "grpc_lb_policy_pick_first",
         "grpc_lb_policy_round_robin",
         "grpc_load_reporting",
+        "grpc_max_age_filter",
         "grpc_resolver_dns_native",
         "grpc_resolver_sockaddr",
         "grpc_transport_chttp2_client_insecure",
         "grpc_transport_chttp2_server_insecure",
-        "grpc_max_age_filter",
     ],
 )
 
@@ -177,8 +177,6 @@ grpc_cc_library(
     ],
     hdrs = [
         "src/compiler/config.h",
-        "src/compiler/schema_interface.h",
-        "src/compiler/protobuf_plugin.h",
         "src/compiler/cpp_generator.h",
         "src/compiler/cpp_generator_helpers.h",
         "src/compiler/csharp_generator.h",
@@ -190,6 +188,7 @@ grpc_cc_library(
         "src/compiler/objective_c_generator_helpers.h",
         "src/compiler/php_generator.h",
         "src/compiler/php_generator_helpers.h",
+        "src/compiler/protobuf_plugin.h",
         "src/compiler/python_generator.h",
         "src/compiler/python_generator_helpers.h",
         "src/compiler/python_private_generator.h",
@@ -197,6 +196,7 @@ grpc_cc_library(
         "src/compiler/ruby_generator_helpers-inl.h",
         "src/compiler/ruby_generator_map-inl.h",
         "src/compiler/ruby_generator_string-inl.h",
+        "src/compiler/schema_interface.h",
     ],
     external_deps = [
         "protobuf_clib",
@@ -884,14 +884,14 @@ grpc_cc_library(
         "src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver.h",
         "src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.h",
     ],
+    external_deps = [
+        "cares",
+    ],
     language = "c",
     deps = [
         "grpc_base",
         "grpc_client_channel",
     ],
-    external_deps = [
-        "cares",
-    ],
 )
 
 grpc_cc_library(

cmake/msvc_static_runtime.cmake

@@ -3,6 +3,8 @@ option(gRPC_MSVC_STATIC_RUNTIME "Link with static msvc runtime libraries" OFF)
 if(gRPC_MSVC_STATIC_RUNTIME)
   # switch from dynamic to static linking of msvcrt
   foreach(flag_var
+    CMAKE_C_FLAGS CMAKE_C_FLAGS_DEBUG CMAKE_C_FLAGS_RELEASE
+    CMAKE_C_FLAGS_MINSIZEREL CMAKE_C_FLAGS_RELWITHDEBINFO
     CMAKE_CXX_FLAGS CMAKE_CXX_FLAGS_DEBUG CMAKE_CXX_FLAGS_RELEASE
     CMAKE_CXX_FLAGS_MINSIZEREL CMAKE_CXX_FLAGS_RELWITHDEBINFO)
 

grpc.gemspec

@@ -24,7 +24,7 @@ Gem::Specification.new do |s|
   s.files += Dir.glob('include/grpc/**/*')
   s.test_files = Dir.glob('src/ruby/spec/**/*')
   s.bindir = 'src/ruby/bin'
-  s.require_paths = %w( src/ruby/bin src/ruby/lib src/ruby/pb )
+  s.require_paths = %w( src/ruby/lib src/ruby/bin src/ruby/pb )
   s.platform      = Gem::Platform::RUBY
 
   s.add_dependency 'google-protobuf', '~> 3.1'

src/python/grpcio/grpc/_server.py

@@ -705,6 +705,10 @@ def _serve(state):
                     state.rpc_states.remove(rpc_state)
                     if _stop_serving(state):
                         return
+        # We want to force the deletion of the previous event
+        # ~before~ we poll again; if the event has a reference
+        # to a shutdown Call object, this can induce spinlock.
+        event = None
 
 
 def _stop(state, grace):

src/ruby/end2end/channel_closing_driver.rb

@@ -36,7 +36,7 @@ require_relative './end2end_common'
 
 def main
   STDERR.puts 'start server'
-  server_runner = ServerRunner.new
+  server_runner = ServerRunner.new(EchoServerImpl)
   server_port = server_runner.run
 
   sleep 1

src/ruby/end2end/channel_state_driver.rb

@@ -35,7 +35,7 @@ require_relative './end2end_common'
 
 def main
   STDERR.puts 'start server'
-  server_runner = ServerRunner.new
+  server_runner = ServerRunner.new(EchoServerImpl)
   server_port = server_runner.run
 
   sleep 1

src/ruby/end2end/end2end_common.rb

@@ -55,13 +55,14 @@ end
 
 # ServerRunner starts an "echo server" that test clients can make calls to
 class ServerRunner
-  def initialize
+  def initialize(service_impl)
+    @service_impl = service_impl
   end
 
   def run
     @srv = GRPC::RpcServer.new
     port = @srv.add_http2_port('0.0.0.0:0', :this_port_is_insecure)
-    @srv.handle(EchoServerImpl)
+    @srv.handle(@service_impl)
 
     @thd = Thread.new do
       @srv.run

src/ruby/end2end/killed_client_thread_client.rb

@@ -0,0 +1,58 @@
+#!/usr/bin/env ruby
+
+# Copyright 2015, 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.
+
+# Attempt to reproduce
+# https://github.com/GoogleCloudPlatform/google-cloud-ruby/issues/1327
+
+require_relative './end2end_common'
+
+def main
+  server_port = ''
+  OptionParser.new do |opts|
+    opts.on('--client_control_port=P', String) do
+      STDERR.puts 'client control port not used'
+    end
+    opts.on('--server_port=P', String) do |p|
+      server_port = p
+    end
+  end.parse!
+
+  thd = Thread.new do
+    stub = Echo::EchoServer::Stub.new("localhost:#{server_port}",
+                                      :this_channel_is_insecure)
+    stub.echo(Echo::EchoRequest.new(request: 'hello'))
+    fail 'the clients rpc in this test shouldnt complete. ' \
+      'expecting SIGINT to happen in the middle of the call'
+  end
+  thd.join
+end
+
+main

src/ruby/end2end/killed_client_thread_driver.rb

@@ -0,0 +1,114 @@
+#!/usr/bin/env ruby
+
+# Copyright 2016, 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.
+
+require_relative './end2end_common'
+
+# Service that sleeps for a long time upon receiving an 'echo request'
+# Also, this notifies @call_started_cv once it has received a request.
+class SleepingEchoServerImpl < Echo::EchoServer::Service
+  def initialize(call_started, call_started_mu, call_started_cv)
+    @call_started = call_started
+    @call_started_mu = call_started_mu
+    @call_started_cv = call_started_cv
+  end
+
+  def echo(echo_req, _)
+    @call_started_mu.synchronize do
+      @call_started.set_true
+      @call_started_cv.signal
+    end
+    sleep 1000
+    Echo::EchoReply.new(response: echo_req.request)
+  end
+end
+
+# Mutable boolean
+class BoolHolder
+  attr_reader :val
+
+  def init
+    @val = false
+  end
+
+  def set_true
+    @val = true
+  end
+end
+
+def main
+  STDERR.puts 'start server'
+
+  call_started = BoolHolder.new
+  call_started_mu = Mutex.new
+  call_started_cv = ConditionVariable.new
+
+  service_impl = SleepingEchoServerImpl.new(call_started,
+                                            call_started_mu,
+                                            call_started_cv)
+  server_runner = ServerRunner.new(service_impl)
+  server_port = server_runner.run
+
+  STDERR.puts 'start client'
+  _, client_pid = start_client('killed_client_thread_client.rb',
+                               server_port)
+
+  call_started_mu.synchronize do
+    call_started_cv.wait(call_started_mu) until call_started.val
+  end
+
+  # SIGINT the child process now that it's
+  # in the middle of an RPC (happening on a non-main thread)
+  Process.kill('SIGINT', client_pid)
+  STDERR.puts 'sent shutdown'
+
+  begin
+    Timeout.timeout(10) do
+      Process.wait(client_pid)
+    end
+  rescue Timeout::Error
+    STDERR.puts "timeout wait for client pid #{client_pid}"
+    Process.kill('SIGKILL', client_pid)
+    Process.wait(client_pid)
+    STDERR.puts 'killed client child'
+    raise 'Timed out waiting for client process. ' \
+      'It likely hangs when killed while in the middle of an rpc'
+  end
+
+  client_exit_code = $CHILD_STATUS
+  if client_exit_code.termsig != 2 # SIGINT
+    fail 'expected client exit from SIGINT ' \
+      "but got child status: #{client_exit_code}"
+  end
+
+  server_runner.stop
+end
+
+main

src/ruby/end2end/sig_handling_driver.rb

@@ -36,7 +36,7 @@ require_relative './end2end_common'
 
 def main
   STDERR.puts 'start server'
-  server_runner = ServerRunner.new
+  server_runner = ServerRunner.new(EchoServerImpl)
   server_port = server_runner.run
 
   sleep 1

src/ruby/end2end/sig_int_during_channel_watch_driver.rb

@@ -36,7 +36,7 @@ require_relative './end2end_common'
 
 def main
   STDERR.puts 'start server'
-  server_runner = ServerRunner.new
+  server_runner = ServerRunner.new(EchoServerImpl)
   server_port = server_runner.run
 
   sleep 1

src/ruby/ext/grpc/rb_call.c

@@ -784,7 +784,7 @@ static VALUE grpc_run_batch_stack_build_result(run_batch_stack *st) {
    Only one operation of each type can be active at once in any given
    batch */
 static VALUE grpc_rb_call_run_batch(VALUE self, VALUE ops_hash) {
-  run_batch_stack st;
+  run_batch_stack *st = NULL;
   grpc_rb_call *call = NULL;
   grpc_event ev;
   grpc_call_error err;
@@ -792,6 +792,7 @@ static VALUE grpc_rb_call_run_batch(VALUE self, VALUE ops_hash) {
   VALUE rb_write_flag = rb_ivar_get(self, id_write_flag);
   unsigned write_flag = 0;
   void *tag = (void*)&st;
+
   if (RTYPEDDATA_DATA(self) == NULL) {
     rb_raise(grpc_rb_eCallError, "Cannot run batch on closed call");
     return Qnil;
@@ -806,14 +807,16 @@ static VALUE grpc_rb_call_run_batch(VALUE self, VALUE ops_hash) {
   if (rb_write_flag != Qnil) {
     write_flag = NUM2UINT(rb_write_flag);
   }
-  grpc_run_batch_stack_init(&st, write_flag);
-  grpc_run_batch_stack_fill_ops(&st, ops_hash);
+  st = gpr_malloc(sizeof(run_batch_stack));
+  grpc_run_batch_stack_init(st, write_flag);
+  grpc_run_batch_stack_fill_ops(st, ops_hash);
 
   /* call grpc_call_start_batch, then wait for it to complete using
    * pluck_event */
-  err = grpc_call_start_batch(call->wrapped, st.ops, st.op_num, tag, NULL);
+  err = grpc_call_start_batch(call->wrapped, st->ops, st->op_num, tag, NULL);
   if (err != GRPC_CALL_OK) {
-    grpc_run_batch_stack_cleanup(&st);
+    grpc_run_batch_stack_cleanup(st);
+    gpr_free(st);
     rb_raise(grpc_rb_eCallError,
              "grpc_call_start_batch failed with %s (code=%d)",
              grpc_call_error_detail_of(err), err);
@@ -826,8 +829,9 @@ static VALUE grpc_rb_call_run_batch(VALUE self, VALUE ops_hash) {
   }
   /* Build and return the BatchResult struct result,
      if there is an error, it's reflected in the status */
-  result = grpc_run_batch_stack_build_result(&st);
-  grpc_run_batch_stack_cleanup(&st);
+  result = grpc_run_batch_stack_build_result(st);
+  grpc_run_batch_stack_cleanup(st);
+  gpr_free(st);
   return result;
 }
 

templates/grpc.gemspec.template

@@ -26,7 +26,7 @@
     s.files += Dir.glob('include/grpc/**/*')
     s.test_files = Dir.glob('src/ruby/spec/**/*')
     s.bindir = 'src/ruby/bin'
-    s.require_paths = %w( src/ruby/bin src/ruby/lib src/ruby/pb )
+    s.require_paths = %w( src/ruby/lib src/ruby/bin src/ruby/pb )
     s.platform      = Gem::Platform::RUBY
 
     s.add_dependency 'google-protobuf', '~> 3.1'

tools/run_tests/helper_scripts/pre_build_csharp.bat

@@ -43,7 +43,7 @@ cd build
 mkdir %ARCHITECTURE%
 cd %ARCHITECTURE%
 @rem TODO(jtattermusch): Stop hardcoding path to yasm once Jenkins workers can locate yasm correctly
-cmake -G "Visual Studio 14 2015" -A %ARCHITECTURE% -DgRPC_BUILD_TESTS=OFF -DCMAKE_ASM_NASM_COMPILER="C:/Program Files (x86)/yasm/yasm.exe" ../../.. || goto :error
+cmake -G "Visual Studio 14 2015" -A %ARCHITECTURE% -DgRPC_BUILD_TESTS=OFF -DgRPC_MSVC_STATIC_RUNTIME=ON -DCMAKE_ASM_NASM_COMPILER="C:/Program Files (x86)/yasm/yasm.exe" ../../.. || goto :error
 cd ..\..\..
 
 @rem Location of nuget.exe

tools/run_tests/helper_scripts/run_ruby_end2end_tests.sh

@@ -38,4 +38,5 @@ ruby src/ruby/end2end/sig_handling_driver.rb || EXIT_CODE=1
 ruby src/ruby/end2end/channel_state_driver.rb || EXIT_CODE=1
 ruby src/ruby/end2end/channel_closing_driver.rb || EXIT_CODE=1
 ruby src/ruby/end2end/sig_int_during_channel_watch_driver.rb || EXIT_CODE=1
+ruby src/ruby/end2end/killed_client_thread_driver.rb || EXIT_CODE=1
 exit $EXIT_CODE