From 5c6dda8639bd390565e794192ddfb15af0837c92 Mon Sep 17 00:00:00 2001
From: Alexander Polcyn <apolcyn@google.com>
Date: Wed, 17 May 2017 13:08:34 -0700
Subject: [PATCH] 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(-)

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 <grpc/grpc.h>
 #include <grpc/support/time.h>
+#include <grpc/support/log.h>
 #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 =
-- 
GitLab