Removed duplicated refs

1 files changed, 67 insertions, 79 deletions

diff --git a/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.c b/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.c
index 3e34257ea7..def8d5d68a 100644
--- a/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.c
+++ b/src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.c
@@ -141,6 +141,21 @@ struct rr_subchannel_list {
   bool shutting_down;
 };
 
+static rr_subchannel_list *rr_subchannel_list_create(round_robin_lb_policy *p,
+                                                     size_t num_subchannels) {
+  rr_subchannel_list *subchannel_list = gpr_zalloc(sizeof(*subchannel_list));
+  subchannel_list->policy = p;
+  subchannel_list->subchannels =
+      gpr_zalloc(sizeof(subchannel_data) * num_subchannels);
+  subchannel_list->num_subchannels = num_subchannels;
+  gpr_ref_init(&subchannel_list->refcount, 1);
+  if (GRPC_TRACER_ON(grpc_lb_round_robin_trace)) {
+    gpr_log(GPR_INFO, "[RR %p] Created subchannel list %p for %lu subchannels",
+            (void *)p, (void *)subchannel_list, (unsigned long)num_subchannels);
+  }
+  return subchannel_list;
+}
+
 static void rr_subchannel_list_destroy(grpc_exec_ctx *exec_ctx,
                                        rr_subchannel_list *subchannel_list) {
   GPR_ASSERT(subchannel_list->shutting_down);
@@ -191,30 +206,15 @@ static void rr_subchannel_list_unref(grpc_exec_ctx *exec_ctx,
   }
 }
 
-static rr_subchannel_list *rr_subchannel_list_create(round_robin_lb_policy *p,
-                                                     size_t num_subchannels) {
-  rr_subchannel_list *subchannel_list = gpr_zalloc(sizeof(*subchannel_list));
-  subchannel_list->policy = p;
-  subchannel_list->subchannels =
-      gpr_zalloc(sizeof(subchannel_data) * num_subchannels);
-  subchannel_list->num_subchannels = num_subchannels;
-  gpr_ref_init(&subchannel_list->refcount, 1);
-  if (GRPC_TRACER_ON(grpc_lb_round_robin_trace)) {
-    gpr_log(GPR_DEBUG, "Created subchannel list %p for %lu subchannels",
-            (void *)subchannel_list, (unsigned long)num_subchannels);
-  }
-  return subchannel_list;
-}
-
 /** Mark \a subchannel_list as discarded. Unsubscribes all its subchannels. The
  * watcher's callback will ultimately unref \a subchannel_list.  */
-static void rr_subchannel_list_shutdown(grpc_exec_ctx *exec_ctx,
-                                        rr_subchannel_list *subchannel_list,
-                                        const char *reason) {
+static void rr_subchannel_list_shutdown_and_unref(
+    grpc_exec_ctx *exec_ctx, rr_subchannel_list *subchannel_list,
+    const char *reason) {
   GPR_ASSERT(!subchannel_list->shutting_down);
   if (GRPC_TRACER_ON(grpc_lb_round_robin_trace)) {
-    gpr_log(GPR_DEBUG, "Shutting down subchannel_list %p (%s)",
-            (void *)subchannel_list, reason);
+    gpr_log(GPR_DEBUG, "[RR %p] Shutting down subchannel_list %p (%s)",
+            (void *)subchannel_list->policy, (void *)subchannel_list, reason);
   }
   GPR_ASSERT(!subchannel_list->shutting_down);
   subchannel_list->shutting_down = true;
@@ -222,18 +222,19 @@ static void rr_subchannel_list_shutdown(grpc_exec_ctx *exec_ctx,
     subchannel_data *sd = &subchannel_list->subchannels[i];
     if (sd->subchannel != NULL) {  // if subchannel isn't shutdown, unsubscribe.
       if (GRPC_TRACER_ON(grpc_lb_round_robin_trace)) {
-        gpr_log(GPR_DEBUG,
-                "Unsubscribing from subchannel %p as part of shutting down "
-                "subchannel_list %p",
-                (void *)sd->subchannel, (void *)subchannel_list);
+        gpr_log(
+            GPR_DEBUG,
+            "[RR %p] Unsubscribing from subchannel %p as part of shutting down "
+            "subchannel_list %p",
+            (void *)subchannel_list->policy, (void *)sd->subchannel,
+            (void *)subchannel_list);
       }
       grpc_subchannel_notify_on_state_change(exec_ctx, sd->subchannel, NULL,
                                              NULL,
                                              &sd->connectivity_changed_closure);
     }
   }
-  // Corresponds to the creation ref.
-  rr_subchannel_list_unref(exec_ctx, subchannel_list, "creation");
+  rr_subchannel_list_unref(exec_ctx, subchannel_list, reason);
 }
 
 /** Returns the index into p->subchannel_list->subchannels of the next
@@ -303,7 +304,8 @@ static void update_last_ready_subchannel_index_locked(round_robin_lb_policy *p,
 static void rr_destroy(grpc_exec_ctx *exec_ctx, grpc_lb_policy *pol) {
   round_robin_lb_policy *p = (round_robin_lb_policy *)pol;
   if (GRPC_TRACER_ON(grpc_lb_round_robin_trace)) {
-    gpr_log(GPR_DEBUG, "Destroying Round Robin policy at %p", (void *)pol);
+    gpr_log(GPR_DEBUG, "[RR %p] Destroying Round Robin policy at %p",
+            (void *)pol, (void *)pol);
   }
   grpc_connectivity_state_destroy(exec_ctx, &p->state_tracker);
   gpr_free(p);
@@ -312,7 +314,8 @@ static void rr_destroy(grpc_exec_ctx *exec_ctx, grpc_lb_policy *pol) {
 static void rr_shutdown_locked(grpc_exec_ctx *exec_ctx, grpc_lb_policy *pol) {
   round_robin_lb_policy *p = (round_robin_lb_policy *)pol;
   if (GRPC_TRACER_ON(grpc_lb_round_robin_trace)) {
-    gpr_log(GPR_DEBUG, "Shutting down Round Robin policy at %p", (void *)pol);
+    gpr_log(GPR_DEBUG, "[RR %p] Shutting down Round Robin policy at %p",
+            (void *)pol, (void *)pol);
   }
   p->shutdown = true;
   pending_pick *pp;
@@ -329,19 +332,16 @@ static void rr_shutdown_locked(grpc_exec_ctx *exec_ctx, grpc_lb_policy *pol) {
       GRPC_ERROR_CREATE_FROM_STATIC_STRING("Channel Shutdown"), "rr_shutdown");
   const bool latest_is_current =
       p->subchannel_list == p->latest_pending_subchannel_list;
-  rr_subchannel_list_shutdown(exec_ctx, p->subchannel_list,
-                              "sl_shutdown_rr_shutdown");
-  rr_subchannel_list_unref(exec_ctx, p->subchannel_list,
-                           "sl_shutdown_current+make_pending");
+  rr_subchannel_list_shutdown_and_unref(exec_ctx, p->subchannel_list,
+                                        "sl_shutdown_rr_shutdown");
+  p->subchannel_list = NULL;
   if (!latest_is_current && p->latest_pending_subchannel_list != NULL &&
       !p->latest_pending_subchannel_list->shutting_down) {
-    rr_subchannel_list_shutdown(exec_ctx, p->latest_pending_subchannel_list,
-                                "sl_shutdown_pending_rr_shutdown");
-    rr_subchannel_list_unref(exec_ctx, p->latest_pending_subchannel_list,
-                             "sl_shutdown_pending+make_pending");
+    rr_subchannel_list_shutdown_and_unref(exec_ctx,
+                                          p->latest_pending_subchannel_list,
+                                          "sl_shutdown_pending_rr_shutdown");
     p->latest_pending_subchannel_list = NULL;
   }
-  p->subchannel_list = NULL;
 }
 
 static void rr_cancel_pick_locked(grpc_exec_ctx *exec_ctx, grpc_lb_policy *pol,
@@ -420,7 +420,7 @@ static int rr_pick_locked(grpc_exec_ctx *exec_ctx, grpc_lb_policy *pol,
                           grpc_closure *on_complete) {
   round_robin_lb_policy *p = (round_robin_lb_policy *)pol;
   if (GRPC_TRACER_ON(grpc_lb_round_robin_trace)) {
-    gpr_log(GPR_INFO, "Round Robin %p trying to pick", (void *)pol);
+    gpr_log(GPR_INFO, "[RR %p] Trying to pick", (void *)pol);
   }
   if (p->subchannel_list != NULL) {
     const size_t next_ready_index = get_next_ready_subchannel_index_locked(p);
@@ -436,8 +436,8 @@ static int rr_pick_locked(grpc_exec_ctx *exec_ctx, grpc_lb_policy *pol,
       if (GRPC_TRACER_ON(grpc_lb_round_robin_trace)) {
         gpr_log(
             GPR_DEBUG,
-            "[RR %p] PICKED TARGET <-- SUBCHANNEL %p (CONNECTED %p) (SL %p, "
-            "INDEX %lu)",
+            "[RR %p] Picked target <-- Subchannel %p (connected %p) (sl %p, "
+            "index %lu)",
             (void *)p, (void *)sd->subchannel, (void *)*target,
             (void *)sd->subchannel_list, (unsigned long)next_ready_index);
       }
@@ -584,9 +584,8 @@ static void rr_connectivity_changed_locked(grpc_exec_ctx *exec_ctx, void *arg,
   if (sd->subchannel_list != p->subchannel_list &&
       sd->subchannel_list != p->latest_pending_subchannel_list) {
     // sd belongs to an outdated subchannel_list: get rid of it.
-    rr_subchannel_list_shutdown(exec_ctx, sd->subchannel_list, "sl_outdated");
-    rr_subchannel_list_unref(exec_ctx, sd->subchannel_list,
-                             "sl_outdated+started_picking");
+    rr_subchannel_list_shutdown_and_unref(exec_ctx, sd->subchannel_list,
+                                          "sl_outdated");
     GRPC_LB_POLICY_WEAK_UNREF(exec_ctx, &p->base, "sl_outdated");
     return;
   }
@@ -646,14 +645,9 @@ static void rr_connectivity_changed_locked(grpc_exec_ctx *exec_ctx, void *arg,
         }
         if (p->subchannel_list != NULL) {
           // dispose of the current subchannel_list
-          rr_subchannel_list_shutdown(exec_ctx, p->subchannel_list,
-                                      "sl_phase_out_shutdown");
-          rr_subchannel_list_unref(exec_ctx, p->subchannel_list,
-                                   "sl_phase_out_shutdown+started_picking");
+          rr_subchannel_list_shutdown_and_unref(exec_ctx, p->subchannel_list,
+                                                "sl_phase_out_shutdown");
         }
-        // Promote pending list: No need to take a ref on
-        // p->latest_pending_subchannel_list: reusing its "make_pending"
-        // one.
         p->subchannel_list = p->latest_pending_subchannel_list;
         p->latest_pending_subchannel_list = NULL;
       }
@@ -665,8 +659,8 @@ static void rr_connectivity_changed_locked(grpc_exec_ctx *exec_ctx, void *arg,
       subchannel_data *selected =
           &p->subchannel_list->subchannels[next_ready_index];
       if (p->pending_picks != NULL) {
-        /* if the selected subchannel is going to be used for the pending
-         * picks, update the last picked pointer */
+        // if the selected subchannel is going to be used for the pending
+        // picks, update the last picked pointer
         update_last_ready_subchannel_index_locked(p, next_ready_index);
       }
       pending_pick *pp;
@@ -680,9 +674,10 @@ static void rr_connectivity_changed_locked(grpc_exec_ctx *exec_ctx, void *arg,
         }
         if (GRPC_TRACER_ON(grpc_lb_round_robin_trace)) {
           gpr_log(GPR_DEBUG,
-                  "[RR CONN CHANGED] TARGET <-- SUBCHANNEL %p (INDEX %lu)",
-                  (void *)selected->subchannel,
-                  (unsigned long)next_ready_index);
+                  "[RR %p] Fulfilling pending pick. Target <-- subchannel %p "
+                  "(subchannel_list %p, index %lu)",
+                  (void *)p, (void *)selected->subchannel,
+                  (void *)p->subchannel_list, (unsigned long)next_ready_index);
         }
         GRPC_CLOSURE_SCHED(exec_ctx, pp->on_complete, GRPC_ERROR_NONE);
         gpr_free(pp);
@@ -747,8 +742,7 @@ static void rr_update_locked(grpc_exec_ctx *exec_ctx, grpc_lb_policy *policy,
     } else {
       // otherwise, keep using the current subchannel list (ignore this update).
       gpr_log(GPR_ERROR,
-              "No valid LB addresses channel arg for Round Robin %p update, "
-              "ignoring.",
+              "[RR %p] No valid LB addresses channel arg for update, ignoring.",
               (void *)p);
     }
     return;
@@ -764,10 +758,8 @@ static void rr_update_locked(grpc_exec_ctx *exec_ctx, grpc_lb_policy *policy,
         GRPC_ERROR_CREATE_FROM_STATIC_STRING("Empty update"),
         "rr_update_empty");
     if (p->subchannel_list != NULL) {
-      rr_subchannel_list_shutdown(exec_ctx, p->subchannel_list,
-                                  "sl_shutdown_empty_update");
-      rr_subchannel_list_unref(exec_ctx, p->subchannel_list,
-                               "sl_shutdown_empty_update+make_pending");
+      rr_subchannel_list_shutdown_and_unref(exec_ctx, p->subchannel_list,
+                                            "sl_shutdown_empty_update");
       p->subchannel_list = NULL;
     }
     return;
@@ -777,18 +769,16 @@ static void rr_update_locked(grpc_exec_ctx *exec_ctx, grpc_lb_policy *policy,
   if (p->latest_pending_subchannel_list != NULL && p->started_picking) {
     if (GRPC_TRACER_ON(grpc_lb_round_robin_trace)) {
       gpr_log(GPR_DEBUG,
-              "Shutting down latest pending subchannel list %p, about to be "
+              "[RR %p] Shutting down latest pending subchannel list %p, about "
+              "to be "
               "replaced by newer latest %p",
-              (void *)p->latest_pending_subchannel_list,
+              (void *)p, (void *)p->latest_pending_subchannel_list,
               (void *)subchannel_list);
     }
-    rr_subchannel_list_shutdown(exec_ctx, p->latest_pending_subchannel_list,
-                                "sl_outdated_dont_smash");
-    rr_subchannel_list_unref(exec_ctx, p->latest_pending_subchannel_list,
-                             "sl_outdated_dont_smash+make_pending");
+    rr_subchannel_list_shutdown_and_unref(
+        exec_ctx, p->latest_pending_subchannel_list, "sl_outdated_dont_smash");
   }
   p->latest_pending_subchannel_list = subchannel_list;
-  rr_subchannel_list_ref(p->latest_pending_subchannel_list, "make_pending");
   grpc_subchannel_args sc_args;
   /* We need to remove the LB addresses in order to be able to compare the
    * subchannel keys of subchannels from a different batch of addresses. */
@@ -812,11 +802,12 @@ static void rr_update_locked(grpc_exec_ctx *exec_ctx, grpc_lb_policy *policy,
     if (GRPC_TRACER_ON(grpc_lb_round_robin_trace)) {
       char *address_uri =
           grpc_sockaddr_to_uri(&addresses->addresses[i].address);
-      gpr_log(GPR_DEBUG,
-              "index %lu: Created subchannel %p for address uri %s into "
-              "subchannel_list %p",
-              (unsigned long)subchannel_index, (void *)subchannel, address_uri,
-              (void *)subchannel_list);
+      gpr_log(
+          GPR_DEBUG,
+          "[RR %p] index %lu: Created subchannel %p for address uri %s into "
+          "subchannel_list %p",
+          (void *)p, (unsigned long)subchannel_index, (void *)subchannel,
+          address_uri, (void *)subchannel_list);
       gpr_free(address_uri);
     }
     grpc_channel_args_destroy(exec_ctx, new_args);
@@ -855,12 +846,9 @@ static void rr_update_locked(grpc_exec_ctx *exec_ctx, grpc_lb_policy *policy,
     // The policy isn't picking yet. Save the update for later, disposing of
     // previous version if any.
     if (p->subchannel_list != NULL) {
-      rr_subchannel_list_shutdown(exec_ctx, p->subchannel_list,
-                                  "rr_update_before_started_picking");
-      rr_subchannel_list_unref(exec_ctx, subchannel_list,
-                               "rr_update_before_started_picking+make_pending");
+      rr_subchannel_list_shutdown_and_unref(exec_ctx, p->subchannel_list,
+                                            "rr_update_before_started_picking");
     }
-    // Recycles "make_pending" reference.
     p->subchannel_list = subchannel_list;
     p->latest_pending_subchannel_list = NULL;
   }
@@ -892,7 +880,7 @@ static grpc_lb_policy *round_robin_create(grpc_exec_ctx *exec_ctx,
   grpc_connectivity_state_init(&p->state_tracker, GRPC_CHANNEL_IDLE,
                                "round_robin");
   if (GRPC_TRACER_ON(grpc_lb_round_robin_trace)) {
-    gpr_log(GPR_DEBUG, "Created Round Robin %p with %lu subchannels", (void *)p,
+    gpr_log(GPR_DEBUG, "[RR %p] Created with %lu subchannels", (void *)p,
             (unsigned long)p->subchannel_list->num_subchannels);
   }
   return &p->base;