diff --git a/src/core/lib/channel/channel_stack.h b/src/core/lib/channel/channel_stack.h index 5d064c5695bbe5b925d404496481835a3a9fb5e4..d9d3a8523358aba2863878ddc23e853835953daa 100644 --- a/src/core/lib/channel/channel_stack.h +++ b/src/core/lib/channel/channel_stack.h @@ -34,6 +34,13 @@ #ifndef GRPC_CORE_LIB_CHANNEL_CHANNEL_STACK_H #define GRPC_CORE_LIB_CHANNEL_CHANNEL_STACK_H +////////////////////////////////////////////////////////////////////////////// +// IMPORTANT NOTE: +// +// When you update this API, please make the corresponding changes to +// the C++ API in src/cpp/common/channel_filter.{h,cc} +////////////////////////////////////////////////////////////////////////////// + /* A channel filter defines how operations on a channel are implemented. Channel filters are chained together to create full channels, and if those chains are linear, then channel stacks provide a mechanism to minimize diff --git a/src/cpp/common/channel_filter.h b/src/cpp/common/channel_filter.h index 107522ea04a946eaaac43694dd70ba160c5a3e02..c9f50df73211934d372e9b06741924df6b7d9c9d 100644 --- a/src/cpp/common/channel_filter.h +++ b/src/cpp/common/channel_filter.h @@ -216,15 +216,13 @@ class TransportStreamOp { /// Represents channel data. class ChannelData { public: - virtual ~ChannelData() { - if (peer_) gpr_free((void *)peer_); - } + virtual ~ChannelData() {} /// Initializes the call data. - virtual grpc_error *Init() { return GRPC_ERROR_NONE; } - - /// Caller does NOT take ownership of result. - const char *peer() const { return peer_; } + virtual grpc_error *Init(grpc_exec_ctx *exec_ctx, + grpc_channel_element_args *args) { + return GRPC_ERROR_NONE; + } // TODO(roth): Find a way to avoid passing elem into these methods. @@ -235,11 +233,7 @@ class ChannelData { const grpc_channel_info *channel_info); protected: - /// Takes ownership of \a peer. - ChannelData(const grpc_channel_args &args, const char *peer) : peer_(peer) {} - - private: - const char *peer_; + ChannelData() {} }; /// Represents call data. @@ -248,7 +242,10 @@ class CallData { virtual ~CallData() {} /// Initializes the call data. - virtual grpc_error *Init() { return GRPC_ERROR_NONE; } + virtual grpc_error *Init(grpc_exec_ctx *exec_ctx, ChannelData *channel_data, + grpc_call_element_args *args) { + return GRPC_ERROR_NONE; + } // TODO(roth): Find a way to avoid passing elem into these methods. @@ -266,7 +263,7 @@ class CallData { virtual char *GetPeer(grpc_exec_ctx *exec_ctx, grpc_call_element *elem); protected: - explicit CallData(const ChannelData &) {} + CallData() {} }; namespace internal { @@ -282,14 +279,8 @@ class ChannelFilter final { static grpc_error *InitChannelElement(grpc_exec_ctx *exec_ctx, grpc_channel_element *elem, grpc_channel_element_args *args) { - const char *peer = - args->optional_transport - ? grpc_transport_get_peer(exec_ctx, args->optional_transport) - : nullptr; - // Construct the object in the already-allocated memory. - ChannelDataType *channel_data = - new (elem->channel_data) ChannelDataType(*args->channel_args, peer); - return channel_data->Init(); + ChannelDataType *channel_data = new (elem->channel_data) ChannelDataType(); + return channel_data->Init(exec_ctx, args); } static void DestroyChannelElement(grpc_exec_ctx *exec_ctx, @@ -317,11 +308,10 @@ class ChannelFilter final { static grpc_error *InitCallElement(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, grpc_call_element_args *args) { - const ChannelDataType &channel_data = - *(ChannelDataType *)elem->channel_data; + ChannelDataType *channel_data = (ChannelDataType *)elem->channel_data; // Construct the object in the already-allocated memory. - CallDataType *call_data = new (elem->call_data) CallDataType(channel_data); - return call_data->Init(); + CallDataType *call_data = new (elem->call_data) CallDataType(); + return call_data->Init(exec_ctx, channel_data, args); } static void DestroyCallElement(grpc_exec_ctx *exec_ctx, diff --git a/test/cpp/common/channel_filter_test.cc b/test/cpp/common/channel_filter_test.cc index 600a953d828fb04fee7340497616124a46719b09..32246a4b765df895501d5728fd8b08f791a4978f 100644 --- a/test/cpp/common/channel_filter_test.cc +++ b/test/cpp/common/channel_filter_test.cc @@ -41,14 +41,24 @@ namespace testing { class MyChannelData : public ChannelData { public: - MyChannelData(const grpc_channel_args& args, const char* peer) - : ChannelData(args, peer) {} + MyChannelData() {} + + grpc_error* Init(grpc_exec_ctx* exec_ctx, + grpc_channel_element_args* args) override { + (void)args->channel_args; // Make sure field is available. + return GRPC_ERROR_NONE; + } }; class MyCallData : public CallData { public: - explicit MyCallData(const ChannelData& channel_data) - : CallData(channel_data) {} + MyCallData() {} + + grpc_error* Init(grpc_exec_ctx* exec_ctx, ChannelData* channel_data, + grpc_call_element_args* args) override { + (void)args->path; // Make sure field is available. + return GRPC_ERROR_NONE; + } }; // This test ensures that when we make changes to the filter API in diff --git a/test/cpp/end2end/filter_end2end_test.cc b/test/cpp/end2end/filter_end2end_test.cc index ab6ed46de564446b85f59c9d40455206d43990cf..bd384f68b40724e3dc996ccf03be4c5606aa75cd 100644 --- a/test/cpp/end2end/filter_end2end_test.cc +++ b/test/cpp/end2end/filter_end2end_test.cc @@ -114,20 +114,17 @@ int GetCallCounterValue() { class ChannelDataImpl : public ChannelData { public: - ChannelDataImpl(const grpc_channel_args& args, const char* peer) - : ChannelData(args, peer) { + grpc_error* Init(grpc_exec_ctx* exec_ctx, grpc_channel_element_args* args) { IncrementConnectionCounter(); + return GRPC_ERROR_NONE; } }; class CallDataImpl : public CallData { public: - explicit CallDataImpl(const ChannelDataImpl& channel_data) - : CallData(channel_data) {} - void StartTransportStreamOp(grpc_exec_ctx* exec_ctx, grpc_call_element* elem, TransportStreamOp* op) override { - // Incrementing the counter could be done from the ctor, but we want + // Incrementing the counter could be done from Init(), but we want // to test that the individual methods are actually called correctly. if (op->recv_initial_metadata() != nullptr) IncrementCallCounter(); grpc_call_next_op(exec_ctx, elem, op->op());