Merge pull request #13670 from apolcyn/fix_backup_poller_race

Unref global backup poller while under its lock

src/core/ext/filters/client_channel/backup_poller.cc

@@ -83,8 +83,8 @@ static void done_poller(grpc_exec_ctx* exec_ctx, void* arg, grpc_error* error) {
 }
 
 static void g_poller_unref(grpc_exec_ctx* exec_ctx) {
+  gpr_mu_lock(&g_poller_mu);
   if (gpr_unref(&g_poller->refs)) {
-    gpr_mu_lock(&g_poller_mu);
     backup_poller* p = g_poller;
     g_poller = nullptr;
     gpr_mu_unlock(&g_poller_mu);
@@ -95,6 +95,8 @@ static void g_poller_unref(grpc_exec_ctx* exec_ctx) {
                                             p, grpc_schedule_on_exec_ctx));
     gpr_mu_unlock(p->pollset_mu);
     grpc_timer_cancel(exec_ctx, &p->polling_timer);
+  } else {
+    gpr_mu_unlock(&g_poller_mu);
   }
 }
 

src/ruby/end2end/multiple_killed_watching_threads_driver.rb

@@ -20,7 +20,7 @@ Thread.abort_on_exception = true
 
 include GRPC::Core::ConnectivityStates
 
-def watch_state(ch)
+def watch_state(ch, sleep_time)
   thd = Thread.new do
     state = ch.connectivity_state(false)
     fail "non-idle state: #{state}" unless state == IDLE
@@ -28,23 +28,34 @@ def watch_state(ch)
   end
   # sleep to get the thread into the middle of a
   # "watch connectivity state" call
-  sleep 0.1
+  sleep sleep_time
   thd.kill
 end
 
-def main
+def run_multiple_killed_watches(num_threads, sleep_time)
   channels = []
-  10.times do
+  num_threads.times do
     ch = GRPC::Core::Channel.new('dummy_host',
                                  nil, :this_channel_is_insecure)
-    watch_state(ch)
+    watch_state(ch, sleep_time)
     channels << ch
   end
 
   # checking state should still be safe to call
   channels.each do |c|
-    fail unless c.connectivity_state(false) == FATAL_FAILURE
+    connectivity_state = c.connectivity_state(false)
+    # The state should be FATAL_FAILURE in the case that it was interrupted
+    # while watching connectivity state, and IDLE if it we never started
+    # watching the channel's connectivity state
+    unless [FATAL_FAILURE, IDLE].include?(connectivity_state)
+      fail "unexpected connectivity state: #{connectivity_state}"
+    end
   end
 end
 
+def main
+  run_multiple_killed_watches(10, 0.1)
+  run_multiple_killed_watches(1000, 0.001)
+end
+
 main