Fix polling_timer shutdown process

1 files changed, 23 insertions, 9 deletions

diff --git a/src/core/ext/filters/client_channel/backup_poller.cc b/src/core/ext/filters/client_channel/backup_poller.cc
index e9bae13056..5f5995a770 100644
--- a/src/core/ext/filters/client_channel/backup_poller.cc
+++ b/src/core/ext/filters/client_channel/backup_poller.cc
@@ -38,22 +38,31 @@ typedef struct backup_poller {
   grpc_closure run_poller_closure;
   grpc_closure shutdown_closure;
   gpr_mu* pollset_mu;
-  grpc_pollset* pollset;
+  grpc_pollset* pollset;  // guarded by pollset_mu
+  bool shutting_down;     // guarded by pollset_mu
   gpr_refcount refs;
   gpr_refcount shutdown_refs;
 } backup_poller;
 
 static gpr_once g_once = GPR_ONCE_INIT;
 static gpr_mu g_poller_mu;
-static backup_poller* g_poller = NULL;
+static backup_poller* g_poller = NULL;  // guarded by g_poller_mu
+// g_poll_interval_ms is set only once at the first time
+// grpc_client_channel_start_backup_polling() is call, after that it is treated
+// as const.
 static int g_poll_interval_ms = DEFAULT_POLL_INTERVAL_MS;
 
-static void init_g_poller_mu() {
+static void init_globals() {
   gpr_mu_init(&g_poller_mu);
   char* env = gpr_getenv("GRPC_CLIENT_CHANNEL_BACKUP_POLL_INTERVAL_MS");
   if (env != NULL) {
     int poll_interval_ms = gpr_parse_nonnegative_int(env);
-    if (poll_interval_ms != -1) {
+    if (poll_interval_ms == -1) {
+      gpr_log(GPR_ERROR,
+              "Invalid GRPC_CLIENT_CHANNEL_BACKUP_POLL_INTERVAL_MS: %s, "
+              "default value %d will be used.",
+              env, g_poll_interval_ms);
+    } else {
       g_poll_interval_ms = poll_interval_ms;
     }
   }
@@ -79,13 +88,13 @@ static void g_poller_unref(grpc_exec_ctx* exec_ctx) {
     backup_poller* p = g_poller;
     g_poller = NULL;
     gpr_mu_unlock(&g_poller_mu);
-
-    grpc_timer_cancel(exec_ctx, &p->polling_timer);
     gpr_mu_lock(p->pollset_mu);
+    p->shutting_down = true;
     grpc_pollset_shutdown(exec_ctx, p->pollset,
                           GRPC_CLOSURE_INIT(&p->shutdown_closure, done_poller,
                                             p, grpc_schedule_on_exec_ctx));
     gpr_mu_unlock(p->pollset_mu);
+    grpc_timer_cancel(exec_ctx, &p->polling_timer);
   }
 }
 
@@ -93,13 +102,18 @@ static void run_poller(grpc_exec_ctx* exec_ctx, void* arg, grpc_error* error) {
   backup_poller* p = (backup_poller*)arg;
   if (error != GRPC_ERROR_NONE) {
     if (error != GRPC_ERROR_CANCELLED) {
-      GRPC_LOG_IF_ERROR("check_connectivity_state", error);
+      GRPC_LOG_IF_ERROR("run_poller", error);
     }
     backup_poller_shutdown_unref(exec_ctx, p);
     return;
   }
   gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC);
   gpr_mu_lock(p->pollset_mu);
+  if (p->shutting_down) {
+    gpr_mu_unlock(p->pollset_mu);
+    backup_poller_shutdown_unref(exec_ctx, p);
+    return;
+  }
   grpc_error* err = grpc_pollset_work(exec_ctx, p->pollset, NULL, now,
                                       gpr_inf_past(GPR_CLOCK_MONOTONIC));
   gpr_mu_unlock(p->pollset_mu);
@@ -112,14 +126,15 @@ static void run_poller(grpc_exec_ctx* exec_ctx, void* arg, grpc_error* error) {
 
 void grpc_client_channel_start_backup_polling(
     grpc_exec_ctx* exec_ctx, grpc_pollset_set* interested_parties) {
+  gpr_once_init(&g_once, init_globals);
   if (g_poll_interval_ms == 0) {
     return;
   }
-  gpr_once_init(&g_once, init_g_poller_mu);
   gpr_mu_lock(&g_poller_mu);
   if (g_poller == NULL) {
     g_poller = (backup_poller*)gpr_zalloc(sizeof(backup_poller));
     g_poller->pollset = (grpc_pollset*)gpr_malloc(grpc_pollset_size());
+    g_poller->shutting_down = false;
     grpc_pollset_init(g_poller->pollset, &g_poller->pollset_mu);
     gpr_ref_init(&g_poller->refs, 0);
     // one for timer cancellation, one for pollset shutdown
@@ -134,7 +149,6 @@ void grpc_client_channel_start_backup_polling(
   }
   gpr_ref(&g_poller->refs);
   gpr_mu_unlock(&g_poller_mu);
-
   grpc_pollset_set_add_pollset(exec_ctx, interested_parties, g_poller->pollset);
 }