diff --git a/src/ruby/end2end/grpc_class_init_client.rb b/src/ruby/end2end/grpc_class_init_client.rb index 464c42d07a8..e73ca76850f 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 195da3cf9f8..c65ed547c54 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 748fe663ed9..4e59174c3b3 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 584b5dbc63d..2a3b3391027 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 =