Code review changes.

src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc

@@ -44,9 +44,9 @@ typedef struct {
   grpc_lb_policy base;
   /** all our subchannels */
   grpc_lb_subchannel_list *subchannel_list;
-  /** Latest pending subchannel list. */
+  /** latest pending subchannel list */
   grpc_lb_subchannel_list *latest_pending_subchannel_list;
-  /** Selected subchannel in \a subchannel_list. */
+  /** selected subchannel in \a subchannel_list */
   grpc_lb_subchannel_data *selected;
   /** have we started picking? */
   bool started_picking;
@@ -351,7 +351,7 @@ static void pf_update_locked(grpc_exec_ctx *exec_ctx, grpc_lb_policy *policy,
   }
   // If we've started picking, start trying to connect to the first
   // subchannel in the new list.
-  if (p->started_picking && subchannel_list->num_subchannels > 0) {
+  if (p->started_picking) {
     grpc_lb_subchannel_list_ref_for_connectivity_watch(
         subchannel_list, "connectivity_watch+update");
     grpc_lb_subchannel_data_start_connectivity_watch(
@@ -396,7 +396,7 @@ static void pf_connectivity_changed_locked(grpc_exec_ctx *exec_ctx, void *arg,
   // either the current or latest pending subchannel lists.
   GPR_ASSERT(sd->subchannel_list == p->subchannel_list ||
              sd->subchannel_list == p->latest_pending_subchannel_list);
-  // Update state counters.
+  // Update state.
   sd->curr_connectivity_state = sd->pending_connectivity_state_unsafe;
   // Handle updates for the currently selected subchannel.
   if (p->selected == sd) {

src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc

@@ -436,7 +436,7 @@ static void rr_connectivity_changed_locked(grpc_exec_ctx *exec_ctx, void *arg,
     grpc_lb_subchannel_data_stop_connectivity_watch(exec_ctx, sd);
     grpc_lb_subchannel_data_unref_subchannel(exec_ctx, sd, "rr_sl_shutdown");
     grpc_lb_subchannel_list_unref_for_connectivity_watch(
-        exec_ctx, sd->subchannel_list, "rr_shutdown");
+        exec_ctx, sd->subchannel_list, "rr_sl_shutdown");
     return;
   }
   // If we're still here, the notification must be for a subchannel in
@@ -601,7 +601,7 @@ static void rr_update_locked(grpc_exec_ctx *exec_ctx, grpc_lb_policy *policy,
     return;
   }
   if (p->started_picking) {
-    if (p->latest_pending_subchannel_list != NULL && p->started_picking) {
+    if (p->latest_pending_subchannel_list != NULL) {
       if (GRPC_TRACER_ON(grpc_lb_round_robin_trace)) {
         gpr_log(GPR_DEBUG,
                 "[RR %p] Shutting down latest pending subchannel list %p, "
@@ -610,8 +610,7 @@ static void rr_update_locked(grpc_exec_ctx *exec_ctx, grpc_lb_policy *policy,
                 (void *)subchannel_list);
       }
       grpc_lb_subchannel_list_shutdown_and_unref(
-          exec_ctx, p->latest_pending_subchannel_list,
+          exec_ctx, p->latest_pending_subchannel_list, "sl_outdated");
-          "sl_outdated_dont_smash");
     }
     p->latest_pending_subchannel_list = subchannel_list;
     for (size_t i = 0; i < subchannel_list->num_subchannels; ++i) {

src/core/ext/filters/client_channel/lb_policy/subchannel_list.cc

@@ -103,11 +103,11 @@ grpc_lb_subchannel_list *grpc_lb_subchannel_list_create(
   gpr_ref_init(&subchannel_list->refcount, 1);
   subchannel_list->subchannels = (grpc_lb_subchannel_data *)gpr_zalloc(
       sizeof(grpc_lb_subchannel_data) * addresses->num_addresses);
-  /* We need to remove the LB addresses in order to be able to compare the
+  // 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. */
+  // subchannel keys of subchannels from a different batch of addresses.
   static const char *keys_to_remove[] = {GRPC_ARG_SUBCHANNEL_ADDRESS,
                                          GRPC_ARG_LB_ADDRESSES};
-  /* Create subchannels for addresses in the update. */
+  // Create a subchannels for each address.
   grpc_subchannel_args sc_args;
   size_t subchannel_index = 0;
   for (size_t i = 0; i < addresses->num_addresses; i++) {
@@ -174,10 +174,8 @@ grpc_lb_subchannel_list *grpc_lb_subchannel_list_create(
     GRPC_CLOSURE_INIT(&sd->connectivity_changed_closure,
                       connectivity_changed_cb, sd,
                       grpc_combiner_scheduler(args->combiner));
-    /* use some sentinel value outside of the range of
+    // Use some sentinel value outside of the range of
-     * grpc_connectivity_state to signal an undefined previous state. We
+    // grpc_connectivity_state to signal an undefined previous state.
-     * won't be referring to this value again and it'll be overwritten after
-     * the first call to rr_connectivity_changed_locked */
     sd->prev_connectivity_state = GRPC_CHANNEL_INIT;
     sd->curr_connectivity_state = subchannel_connectivity_state;
     sd->user_data_vtable = addresses->user_data_vtable;