Merge pull request #17629 from AspirinSJL/clean_sc_args

Remove filters from subchannel args

5 files changed, 10 insertions, 35 deletions

diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc
index 9077aa9753..3abacf68ae 100644
--- a/src/core/ext/filters/client_channel/subchannel.cc
+++ b/src/core/ext/filters/client_channel/subchannel.cc
@@ -101,9 +101,6 @@ struct grpc_subchannel {
         keep the subchannel open */
   gpr_atm ref_pair;
 
-  /** non-transport related channel filters */
-  const grpc_channel_filter** filters;
-  size_t num_filters;
   /** channel arguments */
   grpc_channel_args* args;
 
@@ -384,7 +381,6 @@ static void subchannel_destroy(void* arg, grpc_error* error) {
     c->channelz_subchannel->MarkSubchannelDestroyed();
     c->channelz_subchannel.reset();
   }
-  gpr_free((void*)c->filters);
   c->health_check_service_name.reset();
   grpc_channel_args_destroy(c->args);
   grpc_connectivity_state_destroy(&c->state_tracker);
@@ -567,15 +563,6 @@ grpc_subchannel* grpc_subchannel_create(grpc_connector* connector,
   gpr_atm_no_barrier_store(&c->ref_pair, 1 << INTERNAL_REF_BITS);
   c->connector = connector;
   grpc_connector_ref(c->connector);
-  c->num_filters = args->filter_count;
-  if (c->num_filters > 0) {
-    c->filters = static_cast<const grpc_channel_filter**>(
-        gpr_malloc(sizeof(grpc_channel_filter*) * c->num_filters));
-    memcpy((void*)c->filters, args->filters,
-           sizeof(grpc_channel_filter*) * c->num_filters);
-  } else {
-    c->filters = nullptr;
-  }
   c->pollset_set = grpc_pollset_set_create();
   grpc_resolved_address* addr =
       static_cast<grpc_resolved_address*>(gpr_malloc(sizeof(*addr)));
diff --git a/src/core/ext/filters/client_channel/subchannel.h b/src/core/ext/filters/client_channel/subchannel.h
index 14f87f2c68..d0c0a672fa 100644
--- a/src/core/ext/filters/client_channel/subchannel.h
+++ b/src/core/ext/filters/client_channel/subchannel.h
@@ -189,11 +189,6 @@ grpc_call_stack* grpc_subchannel_call_get_call_stack(
 struct grpc_subchannel_args {
   /* When updating this struct, also update subchannel_index.c */
 
-  /** Channel filters for this channel - wrapped factories will likely
-      want to mutate this */
-  const grpc_channel_filter** filters;
-  /** The number of filters in the above array */
-  size_t filter_count;
   /** Channel arguments to be supplied to the newly created channel */
   const grpc_channel_args* args;
 };
diff --git a/src/core/ext/filters/client_channel/subchannel_index.cc b/src/core/ext/filters/client_channel/subchannel_index.cc
index aa8441f17b..0ae7898c5a 100644
--- a/src/core/ext/filters/client_channel/subchannel_index.cc
+++ b/src/core/ext/filters/client_channel/subchannel_index.cc
@@ -49,15 +49,6 @@ static grpc_subchannel_key* create_key(
     grpc_channel_args* (*copy_channel_args)(const grpc_channel_args* args)) {
   grpc_subchannel_key* k =
       static_cast<grpc_subchannel_key*>(gpr_malloc(sizeof(*k)));
-  k->args.filter_count = args->filter_count;
-  if (k->args.filter_count > 0) {
-    k->args.filters = static_cast<const grpc_channel_filter**>(
-        gpr_malloc(sizeof(*k->args.filters) * k->args.filter_count));
-    memcpy(reinterpret_cast<grpc_channel_filter*>(k->args.filters),
-           args->filters, sizeof(*k->args.filters) * k->args.filter_count);
-  } else {
-    k->args.filters = nullptr;
-  }
   k->args.args = copy_channel_args(args->args);
   return k;
 }
@@ -75,18 +66,10 @@ int grpc_subchannel_key_compare(const grpc_subchannel_key* a,
                                 const grpc_subchannel_key* b) {
   // To pretend the keys are different, return a non-zero value.
   if (GPR_UNLIKELY(g_force_creation)) return 1;
-  int c = GPR_ICMP(a->args.filter_count, b->args.filter_count);
-  if (c != 0) return c;
-  if (a->args.filter_count > 0) {
-    c = memcmp(a->args.filters, b->args.filters,
-               a->args.filter_count * sizeof(*a->args.filters));
-    if (c != 0) return c;
-  }
   return grpc_channel_args_compare(a->args.args, b->args.args);
 }
 
 void grpc_subchannel_key_destroy(grpc_subchannel_key* k) {
-  gpr_free(reinterpret_cast<grpc_channel_args*>(k->args.filters));
   grpc_channel_args_destroy(const_cast<grpc_channel_args*>(k->args.args));
   gpr_free(k);
 }
diff --git a/src/core/lib/surface/channel_init.h b/src/core/lib/surface/channel_init.h
index f01852473b..d17a721606 100644
--- a/src/core/lib/surface/channel_init.h
+++ b/src/core/lib/surface/channel_init.h
@@ -45,6 +45,11 @@ void grpc_channel_init_init(void);
 /// registration order (in the case of a tie).
 /// Stages are registered against one of the pre-determined channel stack
 /// types.
+/// If the channel stack type is GRPC_CLIENT_SUBCHANNEL, the caller should
+/// ensure that subchannels with different filter lists will always have
+/// different channel args. This requires setting a channel arg in case the
+/// registration function relies on some condition other than channel args to
+/// decide whether to add a filter or not.
 void grpc_channel_init_register_stage(grpc_channel_stack_type type,
                                       int priority,
                                       grpc_channel_init_stage stage_fn,
diff --git a/src/cpp/common/channel_filter.h b/src/cpp/common/channel_filter.h
index 5e569c97e6..1a3295fc80 100644
--- a/src/cpp/common/channel_filter.h
+++ b/src/cpp/common/channel_filter.h
@@ -366,6 +366,11 @@ void ChannelFilterPluginShutdown();
 /// The \a include_filter argument specifies a function that will be called
 /// to determine at run-time whether or not to add the filter. If the
 /// value is nullptr, the filter will be added unconditionally.
+/// If the channel stack type is GRPC_CLIENT_SUBCHANNEL, the caller should
+/// ensure that subchannels with different filter lists will always have
+/// different channel args. This requires setting a channel arg in case the
+/// registration function relies on some condition other than channel args to
+/// decide whether to add a filter or not.
 template <typename ChannelDataType, typename CallDataType>
 void RegisterChannelFilter(
     const char* name, grpc_channel_stack_type stack_type, int priority,