From f7a869fdbed6051b64a25d5bfc7228a7f4711219 Mon Sep 17 00:00:00 2001
From: Alexander Polcyn <apolcyn@google.com>
Date: Wed, 6 Dec 2017 18:57:44 -0800
Subject: [PATCH] Unref global backup poller under its lock

---
 .../filters/client_channel/backup_poller.cc   |  4 +++-
 ...multiple_killed_watching_threads_driver.rb | 23 ++++++++++++++-----
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/src/core/ext/filters/client_channel/backup_poller.cc b/src/core/ext/filters/client_channel/backup_poller.cc
index ed437d255c9..5b86e8631f9 100644
--- a/src/core/ext/filters/client_channel/backup_poller.cc
+++ b/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);
   }
 }
 
diff --git a/src/ruby/end2end/multiple_killed_watching_threads_driver.rb b/src/ruby/end2end/multiple_killed_watching_threads_driver.rb
index 59f6f275e46..8f078cfbed7 100755
--- a/src/ruby/end2end/multiple_killed_watching_threads_driver.rb
+++ b/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