fix race whereby we can call watch_connectivity_state on a destroyed channel

3 files changed, 13 insertions, 7 deletions

diff --git a/src/ruby/end2end/multiple_killed_watching_threads_driver.rb b/src/ruby/end2end/multiple_killed_watching_threads_driver.rb
index 8f078cfbed..8ec2073d98 100755
--- a/src/ruby/end2end/multiple_killed_watching_threads_driver.rb
+++ b/src/ruby/end2end/multiple_killed_watching_threads_driver.rb
@@ -54,8 +54,14 @@ def run_multiple_killed_watches(num_threads, sleep_time)
 end
 
 def main
+  STDERR.puts '10 iterations, sleep 0.1 before killing thread'
   run_multiple_killed_watches(10, 0.1)
+  STDERR.puts '1000 iterations, sleep 0.001 before killing thread'
   run_multiple_killed_watches(1000, 0.001)
+  STDERR.puts '10000 iterations, sleep 0.00001 before killing thread'
+  run_multiple_killed_watches(10_000, 0.00001)
+  STDERR.puts '20000 iterations, sleep 0.00001 before killing thread'
+  run_multiple_killed_watches(20_000, 0.00001)
 end
 
 main
diff --git a/src/ruby/ext/grpc/extconf.rb b/src/ruby/ext/grpc/extconf.rb
index e8e87e4f15..4760f33e38 100644
--- a/src/ruby/ext/grpc/extconf.rb
+++ b/src/ruby/ext/grpc/extconf.rb
@@ -84,7 +84,7 @@ if grpc_config == 'gcov'
 end
 
 if grpc_config == 'dbg'
-  $CFLAGS << ' -O0'
+  $CFLAGS << ' -O0 -ggdb3'
 end
 
 $LDFLAGS << ' -Wl,-wrap,memcpy' if RUBY_PLATFORM =~ /linux/
diff --git a/src/ruby/ext/grpc/rb_channel.c b/src/ruby/ext/grpc/rb_channel.c
index e8bfeb32a0..3f0dc530cf 100644
--- a/src/ruby/ext/grpc/rb_channel.c
+++ b/src/ruby/ext/grpc/rb_channel.c
@@ -315,7 +315,7 @@ static VALUE grpc_rb_channel_get_connectivity_state(int argc, VALUE* argv,
 }
 
 typedef struct watch_state_stack {
-  grpc_channel* channel;
+  bg_watched_channel* bg_wrapped;
   gpr_timespec deadline;
   int last_state;
 } watch_state_stack;
@@ -328,15 +328,15 @@ static void* wait_for_watch_state_op_complete_without_gvl(void* arg) {
   gpr_mu_lock(&global_connection_polling_mu);
   // its unsafe to do a "watch" after "channel polling abort" because the cq has
   // been shut down.
-  if (abort_channel_polling) {
+  if (abort_channel_polling || stack->bg_wrapped->channel_destroyed) {
     gpr_mu_unlock(&global_connection_polling_mu);
     return (void*)0;
   }
   op = gpr_zalloc(sizeof(watch_state_op));
   op->op_type = WATCH_STATE_API;
-  grpc_channel_watch_connectivity_state(stack->channel, stack->last_state,
-                                        stack->deadline, channel_polling_cq,
-                                        op);
+  grpc_channel_watch_connectivity_state(stack->bg_wrapped->channel,
+                                        stack->last_state, stack->deadline,
+                                        channel_polling_cq, op);
 
   while (!op->op.api_callback_args.called_back) {
     gpr_cv_wait(&global_connection_polling_cv, &global_connection_polling_mu,
@@ -388,7 +388,7 @@ static VALUE grpc_rb_channel_watch_connectivity_state(VALUE self,
     return Qnil;
   }
 
-  stack.channel = wrapper->bg_wrapped->channel;
+  stack.bg_wrapped = wrapper->bg_wrapped;
   stack.deadline = grpc_rb_time_timeval(deadline, 0),
   stack.last_state = NUM2LONG(last_state);