[channel-stack] Eliminate post-init in channel stack builder (#29514)

* Eliminate post-init in channel stack builder

We've had a post init function on channel stack builder for a very long
time, an it serves to run some code after initialization completes.

We need the functionality for a few things, but the function passed in
is intimately tied to the filter in use - we never vary it between
multiple functions for the same filter... which means it makes more
sense to locate this functionality as part of the filter interface.

* fix

* Automated change: Fix sanity tests

* fix

Co-authored-by: ctiller <ctiller@users.noreply.github.com>
pull/29273/head^2
Craig Tiller 3 years ago committed by GitHub
parent 697c438df6
commit 18bec00b58
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 10
      src/core/ext/filters/channel_idle/channel_idle_filter.cc
  2. 2
      src/core/ext/filters/channel_idle/channel_idle_filter.h
  3. 2
      src/core/ext/filters/client_channel/client_channel.cc
  4. 2
      src/core/ext/filters/client_channel/client_channel_plugin.cc
  5. 1
      src/core/ext/filters/client_channel/lb_policy/grpclb/client_load_reporting_filter.cc
  6. 2
      src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc
  7. 1
      src/core/ext/filters/client_channel/retry_filter.cc
  8. 3
      src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc
  9. 4
      src/core/ext/filters/deadline/deadline_filter.cc
  10. 2
      src/core/ext/filters/http/client_authority_filter.cc
  11. 4
      src/core/ext/filters/http/http_filters_plugin.cc
  12. 1
      src/core/ext/filters/http/message_compress/message_compress_filter.cc
  13. 1
      src/core/ext/filters/http/message_compress/message_decompress_filter.cc
  14. 1
      src/core/ext/filters/http/server/http_server_filter.cc
  15. 2
      src/core/ext/filters/load_reporting/server_load_reporting_filter.cc
  16. 5
      src/core/ext/filters/message_size/message_size_filter.cc
  17. 1
      src/core/ext/filters/rbac/rbac_filter.cc
  18. 5
      src/core/ext/xds/xds_channel_stack_modifier.cc
  19. 3
      src/core/lib/channel/channel_stack.cc
  20. 7
      src/core/lib/channel/channel_stack.h
  21. 10
      src/core/lib/channel/channel_stack_builder.cc
  22. 19
      src/core/lib/channel/channel_stack_builder.h
  23. 19
      src/core/lib/channel/channel_stack_builder_impl.cc
  24. 37
      src/core/lib/channel/connected_channel.cc
  25. 7
      src/core/lib/channel/promise_based_filter.h
  26. 1
      src/core/lib/security/transport/server_auth_filter.cc
  27. 4
      src/core/lib/surface/builtins.cc
  28. 7
      src/core/lib/surface/init.cc
  29. 1
      src/core/lib/surface/lame_client.cc
  30. 1
      src/core/lib/surface/server.cc
  31. 2
      src/cpp/common/channel_filter.cc
  32. 1
      src/cpp/common/channel_filter.h
  33. 57
      test/core/channel/channel_stack_builder_test.cc
  34. 1
      test/core/channel/channel_stack_test.cc
  35. 2
      test/core/channel/minimal_stack_is_minimal_test.cc
  36. 3
      test/core/end2end/tests/filter_causes_close.cc
  37. 3
      test/core/end2end/tests/filter_context.cc
  38. 19
      test/core/end2end/tests/filter_init_fails.cc
  39. 36
      test/core/end2end/tests/filter_latency.cc
  40. 4
      test/core/end2end/tests/filter_status_code.cc
  41. 3
      test/core/end2end/tests/retry_cancel_with_multiple_send_batches.cc
  42. 4
      test/core/end2end/tests/retry_recv_message_replay.cc
  43. 3
      test/core/end2end/tests/retry_recv_trailing_metadata_error.cc
  44. 4
      test/core/end2end/tests/retry_send_op_fails.cc
  45. 4
      test/core/end2end/tests/retry_transparent_goaway.cc
  46. 4
      test/core/end2end/tests/retry_transparent_not_sent_on_wire.cc
  47. 3
      test/core/transport/chttp2/streams_not_seen_test.cc
  48. 20
      test/core/xds/xds_channel_stack_modifier_test.cc
  49. 12
      test/cpp/microbenchmarks/bm_call_create.cc

@ -124,7 +124,7 @@ void MaxAgeFilter::Shutdown() {
ChannelIdleFilter::Shutdown();
}
void MaxAgeFilter::Start() {
void MaxAgeFilter::PostInit() {
struct StartupClosure {
RefCountedPtr<grpc_channel_stack> channel_stack;
MaxAgeFilter* filter;
@ -276,7 +276,7 @@ void RegisterChannelIdleFilters(CoreConfiguration::Builder* builder) {
auto channel_args = builder->channel_args();
if (!channel_args.WantMinimalStack() &&
GetClientIdleTimeout(channel_args) != Duration::Infinity()) {
builder->PrependFilter(&grpc_client_idle_filter, nullptr);
builder->PrependFilter(&grpc_client_idle_filter);
}
return true;
});
@ -286,11 +286,7 @@ void RegisterChannelIdleFilters(CoreConfiguration::Builder* builder) {
auto channel_args = builder->channel_args();
if (!channel_args.WantMinimalStack() &&
MaxAgeFilter::Config::FromChannelArgs(channel_args).enable()) {
builder->PrependFilter(
&grpc_max_age_filter,
[](grpc_channel_stack*, grpc_channel_element* elem) {
static_cast<MaxAgeFilter*>(elem->channel_data)->Start();
});
builder->PrependFilter(&grpc_max_age_filter);
}
return true;
});

@ -89,7 +89,7 @@ class MaxAgeFilter final : public ChannelIdleFilter {
static absl::StatusOr<MaxAgeFilter> Create(ChannelArgs args,
ChannelFilter::Args filter_args);
void Start();
void PostInit() override;
private:
class ConnectivityWatcher : public AsyncConnectivityStateWatcherInterface {

@ -243,6 +243,7 @@ const grpc_channel_filter ClientChannel::kFilterVtable = {
ClientChannel::CallData::Destroy,
sizeof(ClientChannel),
ClientChannel::Init,
grpc_channel_stack_no_post_init,
ClientChannel::Destroy,
ClientChannel::GetChannelInfo,
"client-channel",
@ -394,6 +395,7 @@ const grpc_channel_filter DynamicTerminationFilter::kFilterVtable = {
DynamicTerminationFilter::CallData::Destroy,
sizeof(DynamicTerminationFilter),
DynamicTerminationFilter::Init,
grpc_channel_stack_no_post_init,
DynamicTerminationFilter::Destroy,
DynamicTerminationFilter::GetChannelInfo,
"dynamic_filter_termination",

@ -59,7 +59,7 @@ void BuildClientChannelConfiguration(CoreConfiguration::Builder* builder) {
builder->channel_init()->RegisterStage(
GRPC_CLIENT_CHANNEL, GRPC_CHANNEL_INIT_BUILTIN_PRIORITY,
[](ChannelStackBuilder* builder) {
builder->AppendFilter(&ClientChannel::kFilterVtable, nullptr);
builder->AppendFilter(&ClientChannel::kFilterVtable);
return true;
});
}

@ -140,6 +140,7 @@ const grpc_channel_filter grpc_client_load_reporting_filter = {
clr_destroy_call_elem,
0, // sizeof(channel_data)
clr_init_channel_elem,
grpc_channel_stack_no_post_init,
clr_destroy_channel_elem,
grpc_channel_next_get_info,
"client_load_reporting"};

@ -1879,7 +1879,7 @@ void RegisterGrpcLbLoadReportingFilter(CoreConfiguration::Builder* builder) {
// this filter at the very top of the subchannel stack, since that
// will minimize the number of metadata elements that the filter
// needs to iterate through to find the ClientStats object.
builder->PrependFilter(&grpc_client_load_reporting_filter, nullptr);
builder->PrependFilter(&grpc_client_load_reporting_filter);
}
return true;
});

@ -2654,6 +2654,7 @@ const grpc_channel_filter kRetryFilterVtable = {
RetryFilter::CallData::Destroy,
sizeof(RetryFilter),
RetryFilter::Init,
grpc_channel_stack_no_post_init,
RetryFilter::Destroy,
RetryFilter::GetChannelInfo,
"retry_filter",

@ -132,6 +132,7 @@ const grpc_channel_filter ServiceConfigChannelArgFilter = {
ServiceConfigChannelArgDestroyCallElem,
sizeof(ServiceConfigChannelArgChannelData),
ServiceConfigChannelArgInitChannelElem,
grpc_channel_stack_no_post_init,
ServiceConfigChannelArgDestroyChannelElem,
grpc_channel_next_get_info,
"service_config_channel_arg"};
@ -148,7 +149,7 @@ void RegisterServiceConfigChannelArgFilter(
!channel_args.GetString(GRPC_ARG_SERVICE_CONFIG).has_value()) {
return true;
}
builder->PrependFilter(&ServiceConfigChannelArgFilter, nullptr);
builder->PrependFilter(&ServiceConfigChannelArgFilter);
return true;
});
}

@ -347,6 +347,7 @@ const grpc_channel_filter grpc_client_deadline_filter = {
deadline_destroy_call_elem,
0, // sizeof(channel_data)
deadline_init_channel_elem,
grpc_channel_stack_no_post_init,
deadline_destroy_channel_elem,
grpc_channel_next_get_info,
"deadline",
@ -362,6 +363,7 @@ const grpc_channel_filter grpc_server_deadline_filter = {
deadline_destroy_call_elem,
0, // sizeof(channel_data)
deadline_init_channel_elem,
grpc_channel_stack_no_post_init,
deadline_destroy_channel_elem,
grpc_channel_next_get_info,
"deadline",
@ -383,7 +385,7 @@ void RegisterDeadlineFilter(CoreConfiguration::Builder* builder) {
auto args = builder->channel_args();
if (args.GetBool(GRPC_ARG_ENABLE_DEADLINE_CHECKS)
.value_or(!args.WantMinimalStack())) {
builder->PrependFilter(filter, nullptr);
builder->PrependFilter(filter);
}
return true;
});

@ -75,7 +75,7 @@ bool add_client_authority_filter(ChannelStackBuilder* builder) {
.value_or(false)) {
return true;
}
builder->PrependFilter(&ClientAuthorityFilter::kFilter, nullptr);
builder->PrependFilter(&ClientAuthorityFilter::kFilter);
return true;
}
} // namespace

@ -50,7 +50,7 @@ void RegisterHttpFilters(CoreConfiguration::Builder* builder) {
const bool enable = args.GetBool(control_channel_arg)
.value_or(enable_in_minimal_stack ||
!args.WantMinimalStack());
if (enable) builder->PrependFilter(filter, nullptr);
if (enable) builder->PrependFilter(filter);
return true;
});
};
@ -60,7 +60,7 @@ void RegisterHttpFilters(CoreConfiguration::Builder* builder) {
channel_type, GRPC_CHANNEL_INIT_BUILTIN_PRIORITY,
[filter](ChannelStackBuilder* builder) {
if (is_building_http_like_transport(builder)) {
builder->PrependFilter(filter, nullptr);
builder->PrependFilter(filter);
}
return true;
});

@ -451,6 +451,7 @@ const grpc_channel_filter grpc_message_compress_filter = {
CompressDestroyCallElem,
sizeof(ChannelData),
CompressInitChannelElem,
grpc_channel_stack_no_post_init,
CompressDestroyChannelElem,
grpc_channel_next_get_info,
"message_compress"};

@ -381,6 +381,7 @@ const grpc_channel_filter MessageDecompressFilter = {
DecompressDestroyCallElem,
sizeof(ChannelData),
DecompressInitChannelElem,
grpc_channel_stack_no_post_init,
DecompressDestroyChannelElem,
grpc_channel_next_get_info,
"message_decompress"};

@ -328,6 +328,7 @@ const grpc_channel_filter grpc_http_server_filter = {
hs_destroy_call_elem,
sizeof(channel_data),
hs_init_channel_elem,
grpc_channel_stack_no_post_init,
hs_destroy_channel_elem,
grpc_channel_next_get_info,
"http-server"};

@ -264,7 +264,7 @@ struct ServerLoadReportingFilterStaticRegistrar {
builder->channel_init()->RegisterStage(
GRPC_SERVER_CHANNEL, INT_MAX, [](ChannelStackBuilder* cs_builder) {
if (MaybeAddServerLoadReportingFilter(cs_builder->channel_args())) {
cs_builder->PrependFilter(&kFilter, nullptr);
cs_builder->PrependFilter(&kFilter);
}
return true;
});

@ -340,6 +340,7 @@ const grpc_channel_filter grpc_message_size_filter = {
message_size_destroy_call_elem,
sizeof(channel_data),
message_size_init_channel_elem,
grpc_channel_stack_no_post_init,
message_size_destroy_channel_elem,
grpc_channel_next_get_info,
"message_size"};
@ -350,7 +351,7 @@ static bool maybe_add_message_size_filter_subchannel(
if (builder->channel_args().WantMinimalStack()) {
return true;
}
builder->PrependFilter(&grpc_message_size_filter, nullptr);
builder->PrependFilter(&grpc_message_size_filter);
return true;
}
@ -367,7 +368,7 @@ static bool maybe_add_message_size_filter(
const bool enable =
lim.max_send_size != -1 || lim.max_recv_size != -1 ||
channel_args.GetString(GRPC_ARG_SERVICE_CONFIG).has_value();
if (enable) builder->PrependFilter(&grpc_message_size_filter, nullptr);
if (enable) builder->PrependFilter(&grpc_message_size_filter);
return true;
}

@ -118,6 +118,7 @@ const grpc_channel_filter RbacFilter::kFilterVtable = {
RbacFilter::CallData::Destroy,
sizeof(RbacFilter),
RbacFilter::Init,
grpc_channel_stack_no_post_init,
RbacFilter::Destroy,
grpc_channel_next_get_info,
"rbac_filter",

@ -53,7 +53,7 @@ bool XdsChannelStackModifier::ModifyChannelStack(ChannelStackBuilder* builder) {
// Insert the filters after the census filter if present.
auto it = builder->mutable_stack()->begin();
while (it != builder->mutable_stack()->end()) {
const char* filter_name_at_it = it->filter->name;
const char* filter_name_at_it = (*it)->name;
if (strcmp("census_server", filter_name_at_it) == 0 ||
strcmp("opencensus_server", filter_name_at_it) == 0) {
break;
@ -71,8 +71,7 @@ bool XdsChannelStackModifier::ModifyChannelStack(ChannelStackBuilder* builder) {
++it;
}
for (const grpc_channel_filter* filter : filters_) {
it = builder->mutable_stack()->insert(
it, ChannelStackBuilder::StackEntry{filter, nullptr});
it = builder->mutable_stack()->insert(it, filter);
++it;
}
return true;

@ -277,3 +277,6 @@ grpc_call_stack* grpc_call_stack_from_top_element(grpc_call_element* elem) {
reinterpret_cast<char*>(elem) -
GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(grpc_call_stack)));
}
void grpc_channel_stack_no_post_init(grpc_channel_stack*,
grpc_channel_element*) {}

@ -164,6 +164,10 @@ struct grpc_channel_filter {
Implementations may assume that elem->channel_data is all zeros. */
grpc_error_handle (*init_channel_elem)(grpc_channel_element* elem,
grpc_channel_element_args* args);
/* Post init per-channel data.
Called after all channel elements have been successfully created. */
void (*post_init_channel_elem)(grpc_channel_stack* stk,
grpc_channel_element* elem);
/* Destroy per channel data.
The filter does not need to do any chaining */
void (*destroy_channel_elem)(grpc_channel_element* elem);
@ -356,6 +360,9 @@ void grpc_call_log_op(const char* file, int line, gpr_log_severity severity,
grpc_call_element* elem,
grpc_transport_stream_op_batch* op);
void grpc_channel_stack_no_post_init(grpc_channel_stack* stk,
grpc_channel_element* elem);
extern grpc_core::TraceFlag grpc_trace_channel;
#define GRPC_CALL_LOG_OP(sev, elem, op) \

@ -47,14 +47,12 @@ ChannelStackBuilder& ChannelStackBuilder::SetChannelArgs(ChannelArgs args) {
return *this;
}
void ChannelStackBuilder::PrependFilter(const grpc_channel_filter* filter,
PostInitFunc post_init) {
stack_.insert(stack_.begin(), {filter, std::move(post_init)});
void ChannelStackBuilder::PrependFilter(const grpc_channel_filter* filter) {
stack_.insert(stack_.begin(), filter);
}
void ChannelStackBuilder::AppendFilter(const grpc_channel_filter* filter,
PostInitFunc post_init) {
stack_.push_back({filter, std::move(post_init)});
void ChannelStackBuilder::AppendFilter(const grpc_channel_filter* filter) {
stack_.push_back(filter);
}
} // namespace grpc_core

@ -44,17 +44,6 @@ namespace grpc_core {
// and a transport.
class ChannelStackBuilder {
public:
// A function that will be called after the channel stack is successfully
// built.
using PostInitFunc = std::function<void(grpc_channel_stack* channel_stack,
grpc_channel_element* elem)>;
// One filter in the currently building stack.
struct StackEntry {
const grpc_channel_filter* filter;
PostInitFunc post_init;
};
// Initialize with a name.
ChannelStackBuilder(const char* name, grpc_channel_stack_type type)
: name_(name), type_(type) {}
@ -84,16 +73,16 @@ class ChannelStackBuilder {
const ChannelArgs& channel_args() const { return args_; }
// Mutable vector of proposed stack entries.
std::vector<StackEntry>* mutable_stack() { return &stack_; }
std::vector<const grpc_channel_filter*>* mutable_stack() { return &stack_; }
// The type of channel stack being built.
grpc_channel_stack_type channel_stack_type() const { return type_; }
// Helper to add a filter to the front of the stack.
void PrependFilter(const grpc_channel_filter* filter, PostInitFunc post_init);
void PrependFilter(const grpc_channel_filter* filter);
// Helper to add a filter to the end of the stack.
void AppendFilter(const grpc_channel_filter* filter, PostInitFunc post_init);
void AppendFilter(const grpc_channel_filter* filter);
// Build the channel stack.
// After success, *result holds the new channel stack,
@ -119,7 +108,7 @@ class ChannelStackBuilder {
// Channel args
ChannelArgs args_;
// The in-progress stack
std::vector<StackEntry> stack_;
std::vector<const grpc_channel_filter*> stack_;
};
} // namespace grpc_core

@ -36,16 +36,9 @@ absl::StatusOr<RefCountedPtr<grpc_channel_stack>>
ChannelStackBuilderImpl::Build() {
auto* stack = mutable_stack();
// create an array of filters
std::vector<const grpc_channel_filter*> filters;
filters.reserve(stack->size());
for (const auto& elem : *stack) {
filters.push_back(elem.filter);
}
// calculate the size of the channel stack
size_t channel_stack_size =
grpc_channel_stack_size(filters.data(), filters.size());
grpc_channel_stack_size(stack->data(), stack->size());
// allocate memory
auto* channel_stack =
@ -74,7 +67,7 @@ ChannelStackBuilderImpl::Build() {
grpc_channel_stack_destroy(stk);
gpr_free(stk);
},
channel_stack, filters.data(), filters.size(), c_args, name(),
channel_stack, stack->data(), stack->size(), c_args, name(),
channel_stack);
grpc_channel_args_destroy(c_args);
@ -87,11 +80,9 @@ ChannelStackBuilderImpl::Build() {
}
// run post-initialization functions
for (size_t i = 0; i < filters.size(); i++) {
if ((*stack)[i].post_init != nullptr) {
(*stack)[i].post_init(channel_stack,
grpc_channel_stack_element(channel_stack, i));
}
for (size_t i = 0; i < stack->size(); i++) {
auto* elem = grpc_channel_stack_element(channel_stack, i);
elem->filter->post_init_channel_elem(channel_stack, elem);
}
return RefCountedPtr<grpc_channel_stack>(channel_stack);

@ -29,6 +29,7 @@
#include <grpc/support/alloc.h>
#include <grpc/support/log.h>
#include "src/core/lib/channel/channel_stack.h"
#include "src/core/lib/gpr/string.h"
#include "src/core/lib/profiling/timers.h"
#include "src/core/lib/transport/transport.h"
@ -184,7 +185,8 @@ static grpc_error_handle connected_channel_init_channel_elem(
grpc_channel_element* elem, grpc_channel_element_args* args) {
channel_data* cd = static_cast<channel_data*>(elem->channel_data);
GPR_ASSERT(args->is_last);
cd->transport = nullptr;
cd->transport = grpc_channel_args_find_pointer<grpc_transport>(
args->channel_args, GRPC_ARG_TRANSPORT);
return GRPC_ERROR_NONE;
}
@ -211,36 +213,25 @@ const grpc_channel_filter grpc_connected_filter = {
connected_channel_destroy_call_elem,
sizeof(channel_data),
connected_channel_init_channel_elem,
[](grpc_channel_stack* channel_stack, grpc_channel_element* elem) {
/* HACK(ctiller): increase call stack size for the channel to make space
for channel data. We need a cleaner (but performant) way to do this,
and I'm not sure what that is yet.
This is only "safe" because call stacks place no additional data after
the last call element, and the last call element MUST be the connected
channel. */
channel_stack->call_stack_size += grpc_transport_stream_size(
static_cast<channel_data*>(elem->channel_data)->transport);
},
connected_channel_destroy_channel_elem,
connected_channel_get_channel_info,
"connected",
};
static void bind_transport(grpc_channel_stack* channel_stack,
grpc_channel_element* elem, void* t) {
channel_data* cd = static_cast<channel_data*>(elem->channel_data);
GPR_ASSERT(elem->filter == &grpc_connected_filter);
GPR_ASSERT(cd->transport == nullptr);
cd->transport = static_cast<grpc_transport*>(t);
/* HACK(ctiller): increase call stack size for the channel to make space
for channel data. We need a cleaner (but performant) way to do this,
and I'm not sure what that is yet.
This is only "safe" because call stacks place no additional data after
the last call element, and the last call element MUST be the connected
channel. */
channel_stack->call_stack_size +=
grpc_transport_stream_size(static_cast<grpc_transport*>(t));
}
bool grpc_add_connected_filter(grpc_core::ChannelStackBuilder* builder) {
grpc_transport* t = builder->transport();
GPR_ASSERT(t != nullptr);
builder->AppendFilter(
&grpc_connected_filter,
[t](grpc_channel_stack* stk, grpc_channel_element* elem) {
bind_transport(stk, elem, t);
});
builder->AppendFilter(&grpc_connected_filter);
return true;
}

@ -59,6 +59,9 @@ class ChannelFilter {
grpc_channel_element* channel_element_;
};
// Perform post-initialization step (if any).
virtual void PostInit() {}
// Construct a promise for one call.
virtual ArenaPromise<ServerMetadataHandle> MakeCallPromise(
CallArgs call_args, NextPromiseFactory next_promise_factory) = 0;
@ -474,6 +477,10 @@ MakePromiseBasedFilter(const char* name) {
new (elem->channel_data) F(std::move(*status));
return GRPC_ERROR_NONE;
},
// post_init_channel_elem
[](grpc_channel_stack*, grpc_channel_element* elem) {
static_cast<F*>(elem->channel_data)->PostInit();
},
// destroy_channel_elem
[](grpc_channel_element* elem) {
static_cast<F*>(elem->channel_data)->~F();

@ -332,6 +332,7 @@ const grpc_channel_filter grpc_server_auth_filter = {
server_auth_destroy_call_elem,
sizeof(channel_data),
server_auth_init_channel_elem,
grpc_channel_stack_no_post_init,
server_auth_destroy_channel_elem,
grpc_channel_next_get_info,
"server-auth"};

@ -36,12 +36,12 @@ void RegisterBuiltins(CoreConfiguration::Builder* builder) {
builder->channel_init()->RegisterStage(
GRPC_CLIENT_LAME_CHANNEL, GRPC_CHANNEL_INIT_BUILTIN_PRIORITY,
[](ChannelStackBuilder* builder) {
builder->AppendFilter(&grpc_lame_filter, nullptr);
builder->AppendFilter(&grpc_lame_filter);
return true;
});
builder->channel_init()->RegisterStage(
GRPC_SERVER_CHANNEL, INT_MAX, [](ChannelStackBuilder* builder) {
builder->PrependFilter(&Server::kServerTopFilter, nullptr);
builder->PrependFilter(&Server::kServerTopFilter);
return true;
});
}

@ -83,7 +83,7 @@ static bool g_shutting_down ABSL_GUARDED_BY(g_init_mu) = false;
static bool maybe_prepend_client_auth_filter(
grpc_core::ChannelStackBuilder* builder) {
if (builder->channel_args().Contains(GRPC_ARG_SECURITY_CONNECTOR)) {
builder->PrependFilter(&grpc_core::ClientAuthFilter::kFilter, nullptr);
builder->PrependFilter(&grpc_core::ClientAuthFilter::kFilter);
}
return true;
}
@ -91,7 +91,7 @@ static bool maybe_prepend_client_auth_filter(
static bool maybe_prepend_server_auth_filter(
grpc_core::ChannelStackBuilder* builder) {
if (builder->channel_args().Contains(GRPC_SERVER_CREDENTIALS_ARG)) {
builder->PrependFilter(&grpc_server_auth_filter, nullptr);
builder->PrependFilter(&grpc_server_auth_filter);
}
return true;
}
@ -100,8 +100,7 @@ static bool maybe_prepend_grpc_server_authz_filter(
grpc_core::ChannelStackBuilder* builder) {
if (builder->channel_args().GetPointer<grpc_authorization_policy_provider>(
GRPC_ARG_AUTHORIZATION_POLICY_PROVIDER) != nullptr) {
builder->PrependFilter(&grpc_core::GrpcServerAuthzFilter::kFilterVtable,
nullptr);
builder->PrependFilter(&grpc_core::GrpcServerAuthzFilter::kFilterVtable);
}
return true;
}

@ -164,6 +164,7 @@ const grpc_channel_filter grpc_lame_filter = {
grpc_core::lame_destroy_call_elem,
sizeof(grpc_core::ChannelData),
grpc_core::lame_init_channel_elem,
grpc_channel_stack_no_post_init,
grpc_core::lame_destroy_channel_elem,
grpc_core::lame_get_channel_info,
"lame-client",

@ -506,6 +506,7 @@ const grpc_channel_filter Server::kServerTopFilter = {
Server::CallData::DestroyCallElement,
sizeof(Server::ChannelData),
Server::ChannelData::InitChannelElement,
grpc_channel_stack_no_post_init,
Server::ChannelData::DestroyChannelElement,
grpc_channel_next_get_info,
"server",

@ -80,7 +80,7 @@ void RegisterChannelFilter(
}
grpc_channel_args_destroy(args);
}
builder->PrependFilter(filter, nullptr);
builder->PrependFilter(filter);
return true;
};
grpc_core::CoreConfiguration::RegisterBuilder(

@ -336,6 +336,7 @@ void RegisterChannelFilter(
FilterType::DestroyCallElement,
FilterType::channel_data_size,
FilterType::InitChannelElement,
grpc_channel_stack_no_post_init,
FilterType::DestroyChannelElement,
FilterType::GetChannelInfo,
name};

@ -58,6 +58,14 @@ void CallDestroyFunc(grpc_call_element* /*elem*/,
bool g_replacement_fn_called = false;
bool g_original_fn_called = false;
void SetReplacementFnCalled(grpc_channel_stack*, grpc_channel_element*) {
g_replacement_fn_called = true;
}
void SetOriginalFnCalled(grpc_channel_stack*, grpc_channel_element*) {
g_original_fn_called = true;
}
TEST(ChannelStackBuilderTest, ReplaceFilter) {
grpc_channel_credentials* creds = grpc_insecure_credentials_create();
grpc_channel* channel =
@ -72,31 +80,21 @@ TEST(ChannelStackBuilderTest, ReplaceFilter) {
}
const grpc_channel_filter replacement_filter = {
grpc_call_next_op,
nullptr,
grpc_channel_next_op,
0,
CallInitFunc,
grpc_call_stack_ignore_set_pollset_or_pollset_set,
CallDestroyFunc,
0,
ChannelInitFunc,
ChannelDestroyFunc,
grpc_channel_next_get_info,
grpc_call_next_op, nullptr,
grpc_channel_next_op, 0,
CallInitFunc, grpc_call_stack_ignore_set_pollset_or_pollset_set,
CallDestroyFunc, 0,
ChannelInitFunc, SetReplacementFnCalled,
ChannelDestroyFunc, grpc_channel_next_get_info,
"filter_name"};
const grpc_channel_filter original_filter = {
grpc_call_next_op,
nullptr,
grpc_channel_next_op,
0,
CallInitFunc,
grpc_call_stack_ignore_set_pollset_or_pollset_set,
CallDestroyFunc,
0,
ChannelInitFunc,
ChannelDestroyFunc,
grpc_channel_next_get_info,
grpc_call_next_op, nullptr,
grpc_channel_next_op, 0,
CallInitFunc, grpc_call_stack_ignore_set_pollset_or_pollset_set,
CallDestroyFunc, 0,
ChannelInitFunc, SetOriginalFnCalled,
ChannelDestroyFunc, grpc_channel_next_get_info,
"filter_name"};
bool AddReplacementFilter(ChannelStackBuilder* builder) {
@ -104,23 +102,16 @@ bool AddReplacementFilter(ChannelStackBuilder* builder) {
// same name.
auto* stk = builder->mutable_stack();
stk->erase(std::remove_if(stk->begin(), stk->end(),
[](const ChannelStackBuilder::StackEntry& entry) {
return strcmp(entry.filter->name,
"filter_name") == 0;
[](const grpc_channel_filter* entry) {
return strcmp(entry->name, "filter_name") == 0;
}),
stk->end());
builder->PrependFilter(&replacement_filter,
[](grpc_channel_stack*, grpc_channel_element*) {
g_replacement_fn_called = true;
});
builder->PrependFilter(&replacement_filter);
return true;
}
bool AddOriginalFilter(ChannelStackBuilder* builder) {
builder->PrependFilter(&original_filter,
[](grpc_channel_stack*, grpc_channel_element*) {
g_original_fn_called = true;
});
builder->PrependFilter(&original_filter);
return true;
}

@ -85,6 +85,7 @@ static void test_create_channel_stack(void) {
call_destroy_func,
sizeof(int),
channel_init_func,
grpc_channel_stack_no_post_init,
channel_destroy_func,
grpc_channel_next_get_info,
"some_test_filter"};

@ -157,7 +157,7 @@ static int check_stack(const char* file, int line, const char* transport_name,
// build up our "got" list
parts.clear();
for (const auto& entry : *builder.mutable_stack()) {
const char* name = entry.filter->name;
const char* name = entry->name;
if (name == nullptr) continue;
parts.push_back(name);
}

@ -244,6 +244,7 @@ static const grpc_channel_filter test_filter = {
destroy_call_elem,
sizeof(channel_data),
init_channel_elem,
grpc_channel_stack_no_post_init,
destroy_channel_elem,
grpc_channel_next_get_info,
"filter_causes_close"};
@ -259,7 +260,7 @@ void filter_causes_close(grpc_end2end_test_config config) {
builder->channel_init()->RegisterStage(
GRPC_SERVER_CHANNEL, 0,
[](grpc_core::ChannelStackBuilder* builder) {
builder->PrependFilter(&test_filter, nullptr);
builder->PrependFilter(&test_filter);
return true;
});
},

@ -267,6 +267,7 @@ static const grpc_channel_filter test_filter = {
destroy_call_elem,
0,
init_channel_elem,
grpc_channel_stack_no_post_init,
destroy_channel_elem,
grpc_channel_next_get_info,
"filter_context"};
@ -290,7 +291,7 @@ void filter_context(grpc_end2end_test_config config) {
// right before the last one.
auto it = builder->mutable_stack()->end();
--it;
builder->mutable_stack()->insert(it, {&test_filter, nullptr});
builder->mutable_stack()->insert(it, &test_filter);
return true;
});
}

@ -442,17 +442,12 @@ static grpc_error_handle init_channel_elem(
static void destroy_channel_elem(grpc_channel_element* /*elem*/) {}
static const grpc_channel_filter test_filter = {
grpc_call_next_op,
nullptr,
grpc_channel_next_op,
0,
init_call_elem,
grpc_call_stack_ignore_set_pollset_or_pollset_set,
destroy_call_elem,
0,
init_channel_elem,
destroy_channel_elem,
grpc_channel_next_get_info,
grpc_call_next_op, nullptr,
grpc_channel_next_op, 0,
init_call_elem, grpc_call_stack_ignore_set_pollset_or_pollset_set,
destroy_call_elem, 0,
init_channel_elem, grpc_channel_stack_no_post_init,
destroy_channel_elem, grpc_channel_next_get_info,
"filter_init_fails"};
/*******************************************************************************
@ -500,7 +495,7 @@ void filter_init_fails(grpc_end2end_test_config config) {
// last one. So we add it right before the last one.
auto it = builder->mutable_stack()->end();
--it;
builder->mutable_stack()->insert(it, {&test_filter, nullptr});
builder->mutable_stack()->insert(it, &test_filter);
return true;
});
};

@ -276,31 +276,21 @@ static grpc_error_handle init_channel_elem(
static void destroy_channel_elem(grpc_channel_element* /*elem*/) {}
static const grpc_channel_filter test_client_filter = {
grpc_call_next_op,
nullptr,
grpc_channel_next_op,
0,
init_call_elem,
grpc_call_stack_ignore_set_pollset_or_pollset_set,
client_destroy_call_elem,
0,
init_channel_elem,
destroy_channel_elem,
grpc_channel_next_get_info,
grpc_call_next_op, nullptr,
grpc_channel_next_op, 0,
init_call_elem, grpc_call_stack_ignore_set_pollset_or_pollset_set,
client_destroy_call_elem, 0,
init_channel_elem, grpc_channel_stack_no_post_init,
destroy_channel_elem, grpc_channel_next_get_info,
"client_filter_latency"};
static const grpc_channel_filter test_server_filter = {
grpc_call_next_op,
nullptr,
grpc_channel_next_op,
0,
init_call_elem,
grpc_call_stack_ignore_set_pollset_or_pollset_set,
server_destroy_call_elem,
0,
init_channel_elem,
destroy_channel_elem,
grpc_channel_next_get_info,
grpc_call_next_op, nullptr,
grpc_channel_next_op, 0,
init_call_elem, grpc_call_stack_ignore_set_pollset_or_pollset_set,
server_destroy_call_elem, 0,
init_channel_elem, grpc_channel_stack_no_post_init,
destroy_channel_elem, grpc_channel_next_get_info,
"server_filter_latency"};
/*******************************************************************************
@ -322,7 +312,7 @@ void filter_latency(grpc_end2end_test_config config) {
// right before the last one.
auto it = builder->mutable_stack()->end();
--it;
builder->mutable_stack()->insert(it, {filter, nullptr});
builder->mutable_stack()->insert(it, filter);
return true;
});
};

@ -331,6 +331,7 @@ static const grpc_channel_filter test_client_filter = {
client_destroy_call_elem,
0,
init_channel_elem,
grpc_channel_stack_no_post_init,
destroy_channel_elem,
grpc_channel_next_get_info,
"client_filter_status_code"};
@ -345,6 +346,7 @@ static const grpc_channel_filter test_server_filter = {
server_destroy_call_elem,
0,
init_channel_elem,
grpc_channel_stack_no_post_init,
destroy_channel_elem,
grpc_channel_next_get_info,
"server_filter_status_code"};
@ -368,7 +370,7 @@ void filter_status_code(grpc_end2end_test_config config) {
// So we add it right before the last one.
auto it = builder->mutable_stack()->end();
--it;
builder->mutable_stack()->insert(it, {filter, nullptr});
builder->mutable_stack()->insert(it, filter);
return true;
});
};

@ -301,6 +301,7 @@ grpc_channel_filter FailSendOpsFilter::kFilterVtable = {
CallData::Destroy,
sizeof(FailSendOpsFilter),
Init,
grpc_channel_stack_no_post_init,
Destroy,
grpc_channel_next_get_info,
"FailSendOpsFilter",
@ -314,7 +315,7 @@ bool MaybeAddFilter(grpc_core::ChannelStackBuilder* builder) {
return true;
}
// Install filter.
builder->PrependFilter(&FailSendOpsFilter::kFilterVtable, nullptr);
builder->PrependFilter(&FailSendOpsFilter::kFilterVtable);
return true;
}

@ -337,6 +337,7 @@ grpc_channel_filter FailFirstSendOpFilter::kFilterVtable = {
CallData::Destroy,
sizeof(FailFirstSendOpFilter),
Init,
grpc_channel_stack_no_post_init,
Destroy,
grpc_channel_next_get_info,
"FailFirstSendOpFilter",
@ -359,8 +360,7 @@ void retry_recv_message_replay(grpc_end2end_test_config config) {
return true;
}
// Install filter.
builder->PrependFilter(&FailFirstSendOpFilter::kFilterVtable,
nullptr);
builder->PrependFilter(&FailFirstSendOpFilter::kFilterVtable);
return true;
});
},

@ -333,6 +333,7 @@ grpc_channel_filter InjectStatusFilter::kFilterVtable = {
CallData::Destroy,
0,
Init,
grpc_channel_stack_no_post_init,
Destroy,
grpc_channel_next_get_info,
"InjectStatusFilter",
@ -346,7 +347,7 @@ bool AddFilter(grpc_core::ChannelStackBuilder* builder) {
return true;
}
// Install filter.
builder->PrependFilter(&InjectStatusFilter::kFilterVtable, nullptr);
builder->PrependFilter(&InjectStatusFilter::kFilterVtable);
return true;
}

@ -352,6 +352,7 @@ grpc_channel_filter FailFirstCallFilter::kFilterVtable = {
CallData::Destroy,
sizeof(FailFirstCallFilter),
Init,
grpc_channel_stack_no_post_init,
Destroy,
grpc_channel_next_get_info,
"FailFirstCallFilter",
@ -374,8 +375,7 @@ void retry_send_op_fails(grpc_end2end_test_config config) {
return true;
}
// Install filter.
builder->PrependFilter(&FailFirstCallFilter::kFilterVtable,
nullptr);
builder->PrependFilter(&FailFirstCallFilter::kFilterVtable);
return true;
});
},

@ -346,6 +346,7 @@ grpc_channel_filter FailFirstCallFilter::kFilterVtable = {
CallData::Destroy,
sizeof(FailFirstCallFilter),
Init,
grpc_channel_stack_no_post_init,
Destroy,
grpc_channel_next_get_info,
"FailFirstCallFilter",
@ -368,8 +369,7 @@ void retry_transparent_goaway(grpc_end2end_test_config config) {
return true;
}
// Install filter.
builder->PrependFilter(&FailFirstCallFilter::kFilterVtable,
nullptr);
builder->PrependFilter(&FailFirstCallFilter::kFilterVtable);
return true;
});
},

@ -345,6 +345,7 @@ grpc_channel_filter FailFirstTenCallsFilter::kFilterVtable = {
CallData::Destroy,
sizeof(FailFirstTenCallsFilter),
Init,
grpc_channel_stack_no_post_init,
Destroy,
grpc_channel_next_get_info,
"FailFirstTenCallsFilter",
@ -367,8 +368,7 @@ void retry_transparent_not_sent_on_wire(grpc_end2end_test_config config) {
return true;
}
// Install filter.
builder->PrependFilter(&FailFirstTenCallsFilter::kFilterVtable,
nullptr);
builder->PrependFilter(&FailFirstTenCallsFilter::kFilterVtable);
return true;
});
},

@ -173,6 +173,7 @@ grpc_channel_filter TrailingMetadataRecordingFilter::kFilterVtable = {
CallData::Destroy,
sizeof(TrailingMetadataRecordingFilter),
Init,
grpc_channel_stack_no_post_init,
Destroy,
grpc_channel_next_get_info,
"trailing-metadata-recording-filter",
@ -770,7 +771,7 @@ int main(int argc, char** argv) {
// right before the last one.
auto it = builder->mutable_stack()->end();
--it;
builder->mutable_stack()->insert(it, {filter, nullptr});
builder->mutable_stack()->insert(it, filter);
return true;
});
};

@ -70,11 +70,11 @@ TEST(XdsChannelStackModifierTest, XdsHttpFiltersInsertion) {
grpc_init();
// Add 2 test filters to XdsChannelStackModifier
const grpc_channel_filter test_filter_1 = {
nullptr, nullptr, nullptr, 0, nullptr, nullptr,
nullptr, 0, nullptr, nullptr, nullptr, kTestFilter1};
nullptr, nullptr, nullptr, 0, nullptr, nullptr, nullptr,
0, nullptr, nullptr, nullptr, nullptr, kTestFilter1};
const grpc_channel_filter test_filter_2 = {
nullptr, nullptr, nullptr, 0, nullptr, nullptr,
nullptr, 0, nullptr, nullptr, nullptr, kTestFilter2};
nullptr, nullptr, nullptr, 0, nullptr, nullptr, nullptr,
0, nullptr, nullptr, nullptr, nullptr, kTestFilter2};
auto channel_stack_modifier = MakeRefCounted<XdsChannelStackModifier>(
std::vector<const grpc_channel_filter*>{&test_filter_1, &test_filter_2});
grpc_arg arg = channel_stack_modifier->MakeChannelArg();
@ -93,7 +93,7 @@ TEST(XdsChannelStackModifierTest, XdsHttpFiltersInsertion) {
ASSERT_TRUE(CoreConfiguration::Get().channel_init().CreateStack(&builder));
std::vector<std::string> filters;
for (const auto& entry : *builder.mutable_stack()) {
filters.push_back(entry.filter->name);
filters.push_back(entry->name);
}
filters.resize(3);
EXPECT_EQ(filters,
@ -108,11 +108,11 @@ TEST(XdsChannelStackModifierTest, XdsHttpFiltersInsertionAfterCensus) {
grpc_init();
// Add 2 test filters to XdsChannelStackModifier
const grpc_channel_filter test_filter_1 = {
nullptr, nullptr, nullptr, 0, nullptr, nullptr,
nullptr, 0, nullptr, nullptr, nullptr, kTestFilter1};
nullptr, nullptr, nullptr, 0, nullptr, nullptr, nullptr,
0, nullptr, nullptr, nullptr, nullptr, kTestFilter1};
const grpc_channel_filter test_filter_2 = {
nullptr, nullptr, nullptr, 0, nullptr, nullptr,
nullptr, 0, nullptr, nullptr, nullptr, kTestFilter2};
nullptr, nullptr, nullptr, 0, nullptr, nullptr, nullptr,
0, nullptr, nullptr, nullptr, nullptr, kTestFilter2};
auto channel_stack_modifier = MakeRefCounted<XdsChannelStackModifier>(
std::vector<const grpc_channel_filter*>{&test_filter_1, &test_filter_2});
grpc_arg arg = channel_stack_modifier->MakeChannelArg();
@ -131,7 +131,7 @@ TEST(XdsChannelStackModifierTest, XdsHttpFiltersInsertionAfterCensus) {
ASSERT_TRUE(CoreConfiguration::Get().channel_init().CreateStack(&builder));
std::vector<std::string> filters;
for (const auto& entry : *builder.mutable_stack()) {
filters.push_back(entry.filter->name);
filters.push_back(entry->name);
}
filters.resize(4);
EXPECT_EQ(filters, std::vector<std::string>({"server", "opencensus_server",

@ -396,8 +396,9 @@ static const grpc_channel_filter phony_filter = {
StartTransportOp, 0,
InitCallElem, SetPollsetOrPollsetSet,
DestroyCallElem, 0,
InitChannelElem, DestroyChannelElem,
GetChannelInfo, "phony_filter"};
InitChannelElem, grpc_channel_stack_no_post_init,
DestroyChannelElem, GetChannelInfo,
"phony_filter"};
} // namespace phony_filter
@ -709,8 +710,9 @@ static const grpc_channel_filter isolated_call_filter = {
StartTransportOp, sizeof(call_data),
InitCallElem, SetPollsetOrPollsetSet,
DestroyCallElem, 0,
InitChannelElem, DestroyChannelElem,
GetChannelInfo, "isolated_call_filter"};
InitChannelElem, grpc_channel_stack_no_post_init,
DestroyChannelElem, GetChannelInfo,
"isolated_call_filter"};
} // namespace isolated_call_filter
class IsolatedCallFixture : public TrackCounters {
@ -728,7 +730,7 @@ class IsolatedCallFixture : public TrackCounters {
grpc_core::ChannelStackBuilderImpl builder("phony", GRPC_CLIENT_CHANNEL);
builder.SetTarget("phony_target");
builder.SetChannelArgs(args);
builder.AppendFilter(&isolated_call_filter::isolated_call_filter, nullptr);
builder.AppendFilter(&isolated_call_filter::isolated_call_filter);
{
grpc_core::ExecCtx exec_ctx;
channel_ =

Loading…
Cancel
Save