From b2c0b7bc7411c0914e2f65d56096ecde1a207b53 Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Thu, 27 Apr 2017 00:26:25 -0700 Subject: constant state watch without timeouts --- src/ruby/end2end/grpc_class_init_client.rb | 65 +++++++++++++++++++++++------- 1 file changed, 51 insertions(+), 14 deletions(-) (limited to 'src/ruby/end2end/grpc_class_init_client.rb') diff --git a/src/ruby/end2end/grpc_class_init_client.rb b/src/ruby/end2end/grpc_class_init_client.rb index ee79292119..8e46907368 100755 --- a/src/ruby/end2end/grpc_class_init_client.rb +++ b/src/ruby/end2end/grpc_class_init_client.rb @@ -34,44 +34,81 @@ require_relative './end2end_common' -def main - grpc_class = '' - OptionParser.new do |opts| - opts.on('--grpc_class=P', String) do |p| - grpc_class = p +def construct_many(test_proc) + thds = [] + 4.times do + thds << Thread.new do + 20.times do + test_proc.call + end end - end.parse! + end + 20.times do + test_proc.call + end + thds.each(&:join) +end + +def run_gc_stress_test(test_proc) + GC.disable + construct_many(test_proc) - test_proc = nil + GC.enable + construct_many(test_proc) + GC.start(full_mark: true, immediate_sweep: true) + construct_many(test_proc) +end + +def get_test_proc(grpc_class) case grpc_class when 'channel' - test_proc = proc do + return proc do GRPC::Core::Channel.new('dummy_host', nil, :this_channel_is_insecure) end when 'server' - test_proc = proc do + return proc do GRPC::Core::Server.new({}) end when 'channel_credentials' - test_proc = proc do + return proc do GRPC::Core::ChannelCredentials.new end when 'call_credentials' - test_proc = proc do + return proc do GRPC::Core::CallCredentials.new(proc { |noop| noop }) end when 'compression_options' - test_proc = proc do + return proc do GRPC::Core::CompressionOptions.new end else fail "bad --grpc_class=#{grpc_class} param" end +end + +def main + grpc_class = '' + gc_stress = false + OptionParser.new do |opts| + opts.on('--grpc_class=P', String) do |p| + grpc_class = p + end + opts.on('--gc_stress=P') do |p| + gc_stress = p + end + end.parse! + + test_proc = get_test_proc(grpc_class) + + if gc_stress == 'true' + run_gc_stress_test(test_proc) + return + end - th = Thread.new { test_proc.call } + thd = Thread.new { test_proc.call } test_proc.call - th.join + thd.join end main -- cgit v1.2.3 From 7b3629e6c2570686701b4bdb6b171b219cbad06e Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Wed, 17 May 2017 00:29:10 -0700 Subject: fix lack-of-abort bug --- src/ruby/end2end/grpc_class_init_client.rb | 16 +++++++++++++--- src/ruby/ext/grpc/rb_channel.c | 12 +----------- 2 files changed, 14 insertions(+), 14 deletions(-) (limited to 'src/ruby/end2end/grpc_class_init_client.rb') diff --git a/src/ruby/end2end/grpc_class_init_client.rb b/src/ruby/end2end/grpc_class_init_client.rb index 8e46907368..62afc85b1d 100755 --- a/src/ruby/end2end/grpc_class_init_client.rb +++ b/src/ruby/end2end/grpc_class_init_client.rb @@ -106,9 +106,19 @@ def main return end - thd = Thread.new { test_proc.call } - test_proc.call - thd.join +# test_proc.call + + thds = [] + 100.times do + thds << Thread.new do + test_proc.call + sleep 10 + end + end + + #test_proc.call + raise "something" + thds.each(&:join) end main diff --git a/src/ruby/ext/grpc/rb_channel.c b/src/ruby/ext/grpc/rb_channel.c index c96dc672ce..748fe663ed 100644 --- a/src/ruby/ext/grpc/rb_channel.c +++ b/src/ruby/ext/grpc/rb_channel.c @@ -107,7 +107,6 @@ bg_watched_channel *bg_watched_channel_list_head = NULL; void grpc_rb_channel_try_register_connection_polling(bg_watched_channel *bg); void *wait_until_channel_polling_thread_started_no_gil(void*); -void wait_until_channel_polling_thread_started_unblocking_func(void*); void *channel_init_try_register_connection_polling_without_gil(void *arg); typedef struct channel_init_try_register_stack { @@ -228,7 +227,7 @@ VALUE grpc_rb_channel_init(int argc, VALUE *argv, VALUE self) { grpc_ruby_once_init(); rb_thread_call_without_gvl(wait_until_channel_polling_thread_started_no_gil, NULL, - wait_until_channel_polling_thread_started_unblocking_func, NULL); + run_poll_channels_loop_unblocking_func, NULL); /* "3" == 3 mandatory args */ rb_scan_args(argc, argv, "3", &target, &channel_args, &credentials); @@ -685,15 +684,6 @@ void *wait_until_channel_polling_thread_started_no_gil(void *arg) { return NULL; } -void wait_until_channel_polling_thread_started_unblocking_func(void* arg) { - (void)arg; - gpr_mu_lock(&global_connection_polling_mu); - gpr_log(GPR_DEBUG, "GRPC_RUBY: wait_until_channel_polling_thread_started_unblocking_func - begin aborting connection polling"); - abort_channel_polling = 1; - gpr_cv_broadcast(&global_connection_polling_cv); - gpr_mu_unlock(&global_connection_polling_mu); -} - static void *set_abort_channel_polling_without_gil(void *arg) { (void)arg; gpr_mu_lock(&global_connection_polling_mu); -- cgit v1.2.3 From fd4cbb70774a943a790459c8ec54d8bb112a3ef2 Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Wed, 17 May 2017 09:57:25 -0700 Subject: cleanup test --- src/ruby/end2end/grpc_class_init_client.rb | 57 ++++++++++++++++++++---------- src/ruby/end2end/grpc_class_init_driver.rb | 14 ++++---- 2 files changed, 46 insertions(+), 25 deletions(-) (limited to 'src/ruby/end2end/grpc_class_init_client.rb') diff --git a/src/ruby/end2end/grpc_class_init_client.rb b/src/ruby/end2end/grpc_class_init_client.rb index 62afc85b1d..d9c23c3835 100755 --- a/src/ruby/end2end/grpc_class_init_client.rb +++ b/src/ruby/end2end/grpc_class_init_client.rb @@ -60,6 +60,27 @@ def run_gc_stress_test(test_proc) construct_many(test_proc) end +def run_concurrency_stress_test(test_proc) + test_proc.call + + thds = [] + 100.times do + thds << Thread.new do + test_proc.call + end + end + + raise "something" +end + +# default (no gc_stress and no concurrency_stress) +def run_default_test(test_proc) + thd = Thread.new do + test_proc.call + end + test_proc.call +end + def get_test_proc(grpc_class) case grpc_class when 'channel' @@ -89,36 +110,34 @@ end def main grpc_class = '' - gc_stress = false + stress_test = '' OptionParser.new do |opts| opts.on('--grpc_class=P', String) do |p| grpc_class = p end - opts.on('--gc_stress=P') do |p| - gc_stress = p + opts.on('--stress_test=P') do |p| + stress_test = p end end.parse! test_proc = get_test_proc(grpc_class) - if gc_stress == 'true' + # the different test configs need to be ran + # in separate processes, since each one tests + # clean shutdown in a different way + case stress_test + when 'gc' + p 'run gc stress' run_gc_stress_test(test_proc) - return - end - -# test_proc.call - - thds = [] - 100.times do - thds << Thread.new do - test_proc.call - sleep 10 - end + when 'concurrency' + p 'run concurrency stress' + run_concurrency_stress_test(test_proc) + when '' + p 'run default' + run_default_test(test_proc) + else + fail "bad --stress_test=#{stress_test} param" end - - #test_proc.call - raise "something" - thds.each(&:join) end main diff --git a/src/ruby/end2end/grpc_class_init_driver.rb b/src/ruby/end2end/grpc_class_init_driver.rb index 0e330a493f..195da3cf9f 100755 --- a/src/ruby/end2end/grpc_class_init_driver.rb +++ b/src/ruby/end2end/grpc_class_init_driver.rb @@ -39,17 +39,17 @@ def main compression_options ) # there is room for false positives in this test, - # do 10 runs for each config to reduce these. - [true, false].each do |gc_stress| - 10.times do - native_grpc_classes.each do |grpc_class| + # do a few runs for each config + 4.times do + native_grpc_classes.each do |grpc_class| + ['', 'gc', 'concurrency'].each do |stress_test_type| STDERR.puts 'start client' this_dir = File.expand_path(File.dirname(__FILE__)) client_path = File.join(this_dir, 'grpc_class_init_client.rb') client_pid = Process.spawn(RbConfig.ruby, client_path, "--grpc_class=#{grpc_class}", - "--gc_stress=#{gc_stress}") + "--stress_test=#{stress_test_type}") begin Timeout.timeout(10) do Process.wait(client_pid) @@ -65,7 +65,9 @@ def main end client_exit_code = $CHILD_STATUS - if client_exit_code != 0 + # concurrency stress test type is expected to exit with a + # non-zero status due to an exception being raised + if client_exit_code != 0 and stress_test_type != 'concurrency' fail "client failed, exit code #{client_exit_code}" end end -- cgit v1.2.3 From 916893b618d602fef2ec1dbf9821611f67a7a1d3 Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Wed, 17 May 2017 10:05:39 -0700 Subject: fix up the test --- src/ruby/end2end/grpc_class_init_client.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'src/ruby/end2end/grpc_class_init_client.rb') diff --git a/src/ruby/end2end/grpc_class_init_client.rb b/src/ruby/end2end/grpc_class_init_client.rb index d9c23c3835..464c42d07a 100755 --- a/src/ruby/end2end/grpc_class_init_client.rb +++ b/src/ruby/end2end/grpc_class_init_client.rb @@ -61,16 +61,15 @@ def run_gc_stress_test(test_proc) end def run_concurrency_stress_test(test_proc) - test_proc.call - - thds = [] 100.times do - thds << Thread.new do + Thread.new do test_proc.call end end - raise "something" + test_proc.call + + raise 'exception thrown while child thread initing class' end # default (no gc_stress and no concurrency_stress) -- cgit v1.2.3 From 5c6dda8639bd390565e794192ddfb15af0837c92 Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Wed, 17 May 2017 13:08:34 -0700 Subject: fix tentative startup bug --- src/ruby/end2end/grpc_class_init_client.rb | 3 ++- src/ruby/end2end/grpc_class_init_driver.rb | 6 +++--- src/ruby/ext/grpc/rb_channel.c | 5 ++--- src/ruby/ext/grpc/rb_grpc.c | 21 +++++++++++++++++++-- 4 files changed, 26 insertions(+), 9 deletions(-) (limited to 'src/ruby/end2end/grpc_class_init_client.rb') diff --git a/src/ruby/end2end/grpc_class_init_client.rb b/src/ruby/end2end/grpc_class_init_client.rb index 464c42d07a..e73ca76850 100755 --- a/src/ruby/end2end/grpc_class_init_client.rb +++ b/src/ruby/end2end/grpc_class_init_client.rb @@ -69,7 +69,7 @@ def run_concurrency_stress_test(test_proc) test_proc.call - raise 'exception thrown while child thread initing class' + fail 'exception thrown while child thread initing class' end # default (no gc_stress and no concurrency_stress) @@ -78,6 +78,7 @@ def run_default_test(test_proc) test_proc.call end test_proc.call + thd.join end def get_test_proc(grpc_class) diff --git a/src/ruby/end2end/grpc_class_init_driver.rb b/src/ruby/end2end/grpc_class_init_driver.rb index 195da3cf9f..c65ed547c5 100755 --- a/src/ruby/end2end/grpc_class_init_driver.rb +++ b/src/ruby/end2end/grpc_class_init_driver.rb @@ -65,9 +65,9 @@ def main end client_exit_code = $CHILD_STATUS - # concurrency stress test type is expected to exit with a - # non-zero status due to an exception being raised - if client_exit_code != 0 and stress_test_type != 'concurrency' + # concurrency stress test type is expected to exit with a + # non-zero status due to an exception being raised + if client_exit_code != 0 && stress_test_type != 'concurrency' fail "client failed, exit code #{client_exit_code}" end end diff --git a/src/ruby/ext/grpc/rb_channel.c b/src/ruby/ext/grpc/rb_channel.c index 748fe663ed..4e59174c3b 100644 --- a/src/ruby/ext/grpc/rb_channel.c +++ b/src/ruby/ext/grpc/rb_channel.c @@ -368,8 +368,8 @@ void *wait_for_watch_state_op_complete_without_gvl(void *arg) { * state changes from "last_state". * */ VALUE grpc_rb_channel_watch_connectivity_state(VALUE self, - VALUE last_state, - VALUE deadline) { + VALUE last_state, + VALUE deadline) { grpc_rb_channel *wrapper = NULL; watch_state_stack stack; void* op_success = 0; @@ -693,7 +693,6 @@ static void *set_abort_channel_polling_without_gil(void *arg) { return NULL; } - /* Temporary fix for * https://github.com/GoogleCloudPlatform/google-cloud-ruby/issues/899. * Transports in idle channels can get destroyed. Normally c-core re-connects, diff --git a/src/ruby/ext/grpc/rb_grpc.c b/src/ruby/ext/grpc/rb_grpc.c index 584b5dbc63..2a3b339102 100644 --- a/src/ruby/ext/grpc/rb_grpc.c +++ b/src/ruby/ext/grpc/rb_grpc.c @@ -42,6 +42,7 @@ #include #include +#include #include "rb_call.h" #include "rb_call_credentials.h" #include "rb_channel.h" @@ -295,11 +296,12 @@ static gpr_once g_once_init = GPR_ONCE_INIT; static void grpc_ruby_once_init_internal() { grpc_init(); - grpc_rb_event_queue_thread_start(); - grpc_rb_channel_polling_thread_start(); atexit(grpc_rb_shutdown); } +static VALUE bg_thread_init_rb_mu = Qundef; +static int bg_thread_init_done = 0; + void grpc_ruby_once_init() { /* ruby_vm_at_exit doesn't seem to be working. It would crash once every * blue moon, and some users are getting it repeatedly. See the discussions @@ -312,6 +314,18 @@ void grpc_ruby_once_init() { * schedule our initialization and destruction only once. */ gpr_once_init(&g_once_init, grpc_ruby_once_init_internal); + + // Avoid calling calling into ruby library (when creating threads here) + // in gpr_once_init. In general, it appears to be unsafe to call + // into the ruby library while holding a non-ruby mutex, because a gil yield + // could end up trying to lock onto that same mutex and deadlocking. + rb_mutex_lock(bg_thread_init_rb_mu); + if (!bg_thread_init_done) { + grpc_rb_event_queue_thread_start(); + grpc_rb_channel_polling_thread_start(); + bg_thread_init_done = 1; + } + rb_mutex_unlock(bg_thread_init_rb_mu); } void Init_grpc_c() { @@ -320,6 +334,9 @@ void Init_grpc_c() { return; } + bg_thread_init_rb_mu = rb_mutex_new(); + rb_global_variable(&bg_thread_init_rb_mu); + grpc_rb_mGRPC = rb_define_module("GRPC"); grpc_rb_mGrpcCore = rb_define_module_under(grpc_rb_mGRPC, "Core"); grpc_rb_sNewServerRpc = -- cgit v1.2.3