From 19bf00b69b665523607b972a8ec86fda04bbd5f4 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Tue, 17 May 2016 18:22:01 -0700 Subject: Re-added signal handlers to cancel calls and shut down servers --- src/ruby/lib/grpc.rb | 3 ++ src/ruby/lib/grpc/generic/active_call.rb | 5 +++ src/ruby/lib/grpc/generic/rpc_server.rb | 4 ++ src/ruby/lib/grpc/signals.rb | 68 ++++++++++++++++++++++++++++++++ 4 files changed, 80 insertions(+) create mode 100644 src/ruby/lib/grpc/signals.rb diff --git a/src/ruby/lib/grpc.rb b/src/ruby/lib/grpc.rb index 79fa705b1c..ab21fc4772 100644 --- a/src/ruby/lib/grpc.rb +++ b/src/ruby/lib/grpc.rb @@ -33,6 +33,7 @@ require_relative 'grpc/errors' require_relative 'grpc/grpc' require_relative 'grpc/logconfig' require_relative 'grpc/notifier' +require_relative 'grpc/signals' require_relative 'grpc/version' require_relative 'grpc/core/time_consts' require_relative 'grpc/generic/active_call' @@ -47,3 +48,5 @@ begin ensure file.close end + +GRPC::Signals.init diff --git a/src/ruby/lib/grpc/generic/active_call.rb b/src/ruby/lib/grpc/generic/active_call.rb index 7fe588bd4c..f91970a923 100644 --- a/src/ruby/lib/grpc/generic/active_call.rb +++ b/src/ruby/lib/grpc/generic/active_call.rb @@ -30,6 +30,7 @@ require 'forwardable' require 'weakref' require_relative 'bidi_call' +require_relative '../signals' class Struct # BatchResult is the struct returned by calls to call#start_batch. @@ -122,6 +123,10 @@ module GRPC @unmarshal = unmarshal @metadata_tag = metadata_tag @op_notifier = nil + weak_self = WeakRef.new(self) + signal_handler = proc { weak_self.cancel if weak_self.weakref_alive? } + remove_handler = GRPC::Signals.register_handler(&signal_handler) + ObjectSpace.define_finalizer(self, remove_handler) end # output_metadata are provides access to hash that can be used to diff --git a/src/ruby/lib/grpc/generic/rpc_server.rb b/src/ruby/lib/grpc/generic/rpc_server.rb index e1496d491a..238aaa9656 100644 --- a/src/ruby/lib/grpc/generic/rpc_server.rb +++ b/src/ruby/lib/grpc/generic/rpc_server.rb @@ -28,6 +28,7 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. require_relative '../grpc' +require_relative '../signals' require_relative 'active_call' require_relative 'service' require 'thread' @@ -352,7 +353,10 @@ module GRPC transition_running_state(:running) @run_cond.broadcast end + remove_signal_handler = GRPC::Signals.register_handler { stop } loop_handle_server_calls + # Remove signal handler when server stops + remove_signal_handler.call end alias_method :run_till_terminated, :run diff --git a/src/ruby/lib/grpc/signals.rb b/src/ruby/lib/grpc/signals.rb new file mode 100644 index 0000000000..56bc9f32a0 --- /dev/null +++ b/src/ruby/lib/grpc/signals.rb @@ -0,0 +1,68 @@ +# 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 'thread' +require_relative 'grpc' + +# GRPC contains the General RPC module. +module GRPC + # Signals contains gRPC functions related to signal handling + module Signals + @signal_handlers = [] + @handlers_mutex = Mutex.new + @previous_handlers = {} + # @signal_received = false + + def register_handler(&handler) + @handlers_mutex.synchronize do + @signal_handlers.push(handler) + # handler.call if @signal_received + end + # Returns a function to remove the handler + lambda do + @handlers_mutex.synchronize { @signal_handlers.delete(handler) } + end + end + module_function :register_handler + + def run_handlers(signal) + # @signal_received = true + @signal_handlers.each(&:call) + @previous_handlers[signal].call + end + module_function :run_handlers + + def init + %w(INT TERM).each do |sig| + @previous_handlers[sig] = Signal.trap(sig) { run_handlers(sig) } + end + end + module_function :init + end +end -- cgit v1.2.3 From e68a71582f4f5dab8cb4823f8ce7f272ed54c7f5 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 18 May 2016 09:46:26 -0700 Subject: Fixed a couple of issues, removed ruby-level signal handling again --- src/ruby/ext/grpc/rb_completion_queue.c | 18 +++------ src/ruby/lib/grpc.rb | 3 -- src/ruby/lib/grpc/generic/active_call.rb | 4 -- src/ruby/lib/grpc/generic/rpc_server.rb | 3 -- src/ruby/lib/grpc/signals.rb | 68 -------------------------------- 5 files changed, 6 insertions(+), 90 deletions(-) delete mode 100644 src/ruby/lib/grpc/signals.rb diff --git a/src/ruby/ext/grpc/rb_completion_queue.c b/src/ruby/ext/grpc/rb_completion_queue.c index 605c7408b4..b6ddbe88dc 100644 --- a/src/ruby/ext/grpc/rb_completion_queue.c +++ b/src/ruby/ext/grpc/rb_completion_queue.c @@ -62,13 +62,10 @@ static void *grpc_rb_completion_queue_next_no_gil(void *param) { gpr_timespec deadline; do { deadline = gpr_time_add(gpr_now(GPR_CLOCK_REALTIME), increment); - if (gpr_time_cmp(deadline, next_call->timeout) > 0) { - // Then we have run out of time - break; - } next_call->event = grpc_completion_queue_next(next_call->cq, deadline, NULL); - if (next_call->event.success) { + if (next_call->event.type != GRPC_QUEUE_TIMEOUT || + gpr_time_cmp(deadline, next_call->timeout) > 0) { break; } } while (!next_call->interrupted); @@ -82,14 +79,11 @@ static void *grpc_rb_completion_queue_pluck_no_gil(void *param) { gpr_timespec deadline; do { deadline = gpr_time_add(gpr_now(GPR_CLOCK_REALTIME), increment); - if (gpr_time_cmp(deadline, next_call->timeout) > 0) { - // Then we have run out of time - break; - } next_call->event = grpc_completion_queue_pluck(next_call->cq, next_call->tag, deadline, NULL); - if (next_call->event.type != GRPC_QUEUE_TIMEOUT) { + if (next_call->event.type != GRPC_QUEUE_TIMEOUT || + gpr_time_cmp(deadline, next_call->timeout) > 0) { break; } } while (!next_call->interrupted); @@ -195,8 +189,8 @@ grpc_event grpc_rb_completion_queue_pluck_event(VALUE self, VALUE tag, The basic reason we need this relatively complicated construction is that we need to re-acquire the GVL when an interrupt comes in, so that the ruby - interpeter can do what it needs to do with the interrupt. But we also need - to get back to plucking when */ + interpreter can do what it needs to do with the interrupt. But we also need + to get back to plucking when the interrupt has been handled. */ do { next_call.interrupted = 0; rb_thread_call_without_gvl(grpc_rb_completion_queue_pluck_no_gil, diff --git a/src/ruby/lib/grpc.rb b/src/ruby/lib/grpc.rb index ab21fc4772..79fa705b1c 100644 --- a/src/ruby/lib/grpc.rb +++ b/src/ruby/lib/grpc.rb @@ -33,7 +33,6 @@ require_relative 'grpc/errors' require_relative 'grpc/grpc' require_relative 'grpc/logconfig' require_relative 'grpc/notifier' -require_relative 'grpc/signals' require_relative 'grpc/version' require_relative 'grpc/core/time_consts' require_relative 'grpc/generic/active_call' @@ -48,5 +47,3 @@ begin ensure file.close end - -GRPC::Signals.init diff --git a/src/ruby/lib/grpc/generic/active_call.rb b/src/ruby/lib/grpc/generic/active_call.rb index f91970a923..c982e0fcc5 100644 --- a/src/ruby/lib/grpc/generic/active_call.rb +++ b/src/ruby/lib/grpc/generic/active_call.rb @@ -123,10 +123,6 @@ module GRPC @unmarshal = unmarshal @metadata_tag = metadata_tag @op_notifier = nil - weak_self = WeakRef.new(self) - signal_handler = proc { weak_self.cancel if weak_self.weakref_alive? } - remove_handler = GRPC::Signals.register_handler(&signal_handler) - ObjectSpace.define_finalizer(self, remove_handler) end # output_metadata are provides access to hash that can be used to diff --git a/src/ruby/lib/grpc/generic/rpc_server.rb b/src/ruby/lib/grpc/generic/rpc_server.rb index 238aaa9656..b548d3cf26 100644 --- a/src/ruby/lib/grpc/generic/rpc_server.rb +++ b/src/ruby/lib/grpc/generic/rpc_server.rb @@ -353,10 +353,7 @@ module GRPC transition_running_state(:running) @run_cond.broadcast end - remove_signal_handler = GRPC::Signals.register_handler { stop } loop_handle_server_calls - # Remove signal handler when server stops - remove_signal_handler.call end alias_method :run_till_terminated, :run diff --git a/src/ruby/lib/grpc/signals.rb b/src/ruby/lib/grpc/signals.rb deleted file mode 100644 index 56bc9f32a0..0000000000 --- a/src/ruby/lib/grpc/signals.rb +++ /dev/null @@ -1,68 +0,0 @@ -# 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 'thread' -require_relative 'grpc' - -# GRPC contains the General RPC module. -module GRPC - # Signals contains gRPC functions related to signal handling - module Signals - @signal_handlers = [] - @handlers_mutex = Mutex.new - @previous_handlers = {} - # @signal_received = false - - def register_handler(&handler) - @handlers_mutex.synchronize do - @signal_handlers.push(handler) - # handler.call if @signal_received - end - # Returns a function to remove the handler - lambda do - @handlers_mutex.synchronize { @signal_handlers.delete(handler) } - end - end - module_function :register_handler - - def run_handlers(signal) - # @signal_received = true - @signal_handlers.each(&:call) - @previous_handlers[signal].call - end - module_function :run_handlers - - def init - %w(INT TERM).each do |sig| - @previous_handlers[sig] = Signal.trap(sig) { run_handlers(sig) } - end - end - module_function :init - end -end -- cgit v1.2.3 From 746ea97afdcdffa074d4710d6706e4e988d4ad8e Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 18 May 2016 09:49:10 -0700 Subject: Finished removing ruby-level signal handlers again --- src/ruby/lib/grpc/generic/active_call.rb | 1 - src/ruby/lib/grpc/generic/rpc_server.rb | 1 - 2 files changed, 2 deletions(-) diff --git a/src/ruby/lib/grpc/generic/active_call.rb b/src/ruby/lib/grpc/generic/active_call.rb index c982e0fcc5..7fe588bd4c 100644 --- a/src/ruby/lib/grpc/generic/active_call.rb +++ b/src/ruby/lib/grpc/generic/active_call.rb @@ -30,7 +30,6 @@ require 'forwardable' require 'weakref' require_relative 'bidi_call' -require_relative '../signals' class Struct # BatchResult is the struct returned by calls to call#start_batch. diff --git a/src/ruby/lib/grpc/generic/rpc_server.rb b/src/ruby/lib/grpc/generic/rpc_server.rb index b548d3cf26..e1496d491a 100644 --- a/src/ruby/lib/grpc/generic/rpc_server.rb +++ b/src/ruby/lib/grpc/generic/rpc_server.rb @@ -28,7 +28,6 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. require_relative '../grpc' -require_relative '../signals' require_relative 'active_call' require_relative 'service' require 'thread' -- cgit v1.2.3