From fc539eb19334bdfde8bc798d94ccb56b686e5193 Mon Sep 17 00:00:00 2001
From: Vijay Pai <vpai@google.com>
Date: Wed, 21 Jun 2017 16:17:35 -0700
Subject: [PATCH] Re-enable public constructor for ClientAsyncResponseReader to
avoid busting client that bypassed code generator. This code is
deprecated-on-arrival as it is a performance pessimization. This code path
should not be used.
---
.../grpc++/impl/codegen/async_unary_call.h | 90 ++++++++++++++-----
include/grpc++/impl/codegen/call.h | 20 +++++
2 files changed, 89 insertions(+), 21 deletions(-)
diff --git a/include/grpc++/impl/codegen/async_unary_call.h b/include/grpc++/impl/codegen/async_unary_call.h
index 4aa4e25a9e..de0ab2372a 100644
--- a/include/grpc++/impl/codegen/async_unary_call.h
+++ b/include/grpc++/impl/codegen/async_unary_call.h
@@ -87,6 +87,28 @@ class ClientAsyncResponseReader final
ClientAsyncResponseReader(call, context, request);
}
+ /// TODO(vjpai): Delete the below constructor
+ /// PLEASE DO NOT USE THIS CONSTRUCTOR IN NEW CODE
+ /// This code is only present as a short-term workaround
+ /// for users that bypassed the code-generator and directly
+ /// created this struct rather than properly using a stub.
+ /// This code will not remain a valid public constructor for long.
+ template <class W>
+ ClientAsyncResponseReader(ChannelInterface* channel, CompletionQueue* cq,
+ const RpcMethod& method, ClientContext* context,
+ const W& request)
+ : context_(context),
+ call_(channel->CreateCall(method, context, cq)),
+ collection_(std::make_shared<Ops>()) {
+ collection_->init_buf_.SetCollection(collection_);
+ collection_->init_buf_.SendInitialMetadata(
+ context->send_initial_metadata_, context->initial_metadata_flags());
+ // TODO(ctiller): don't assert
+ GPR_CODEGEN_ASSERT(collection_->init_buf_.SendMessage(request).ok());
+ collection_->init_buf_.ClientSendClose();
+ call_.PerformOps(&collection_->init_buf_);
+ }
+
// always allocated against a call arena, no memory free required
static void operator delete(void* ptr, std::size_t size) {
assert(size == sizeof(ClientAsyncResponseReader));
@@ -101,9 +123,18 @@ class ClientAsyncResponseReader final
void ReadInitialMetadata(void* tag) {
GPR_CODEGEN_ASSERT(!context_->initial_metadata_received_);
- meta_buf_.set_output_tag(tag);
- meta_buf_.RecvInitialMetadata(context_);
- call_.PerformOps(&meta_buf_);
+ Ops& o = ops;
+
+ // TODO(vjpai): Remove the collection_ specialization as soon
+ // as the public constructor is deleted
+ if (collection_) {
+ o = *collection_;
+ collection_->meta_buf_.SetCollection(collection_);
+ }
+
+ o.meta_buf_.set_output_tag(tag);
+ o.meta_buf_.RecvInitialMetadata(context_);
+ call_.PerformOps(&o.meta_buf_);
}
/// See \a ClientAysncResponseReaderInterface::Finish for semantics.
@@ -112,14 +143,23 @@ class ClientAsyncResponseReader final
/// - the \a ClientContext associated with this call is updated with
/// possible initial and trailing metadata sent from the server.
void Finish(R* msg, Status* status, void* tag) {
- finish_buf_.set_output_tag(tag);
+ Ops& o = ops;
+
+ // TODO(vjpai): Remove the collection_ specialization as soon
+ // as the public constructor is deleted
+ if (collection_) {
+ o = *collection_;
+ collection_->finish_buf_.SetCollection(collection_);
+ }
+
+ o.finish_buf_.set_output_tag(tag);
if (!context_->initial_metadata_received_) {
- finish_buf_.RecvInitialMetadata(context_);
+ o.finish_buf_.RecvInitialMetadata(context_);
}
- finish_buf_.RecvMessage(msg);
- finish_buf_.AllowNoMessage();
- finish_buf_.ClientRecvStatus(context_, status);
- call_.PerformOps(&finish_buf_);
+ o.finish_buf_.RecvMessage(msg);
+ o.finish_buf_.AllowNoMessage();
+ o.finish_buf_.ClientRecvStatus(context_, status);
+ call_.PerformOps(&o.finish_buf_);
}
private:
@@ -129,25 +169,33 @@ class ClientAsyncResponseReader final
template <class W>
ClientAsyncResponseReader(Call call, ClientContext* context, const W& request)
: context_(context), call_(call) {
- init_buf_.SendInitialMetadata(context->send_initial_metadata_,
- context->initial_metadata_flags());
+ ops.init_buf_.SendInitialMetadata(context->send_initial_metadata_,
+ context->initial_metadata_flags());
// TODO(ctiller): don't assert
- GPR_CODEGEN_ASSERT(init_buf_.SendMessage(request).ok());
- init_buf_.ClientSendClose();
- call_.PerformOps(&init_buf_);
+ GPR_CODEGEN_ASSERT(ops.init_buf_.SendMessage(request).ok());
+ ops.init_buf_.ClientSendClose();
+ call_.PerformOps(&ops.init_buf_);
}
// disable operator new
static void* operator new(std::size_t size);
static void* operator new(std::size_t size, void* p) { return p; }
- SneakyCallOpSet<CallOpSendInitialMetadata, CallOpSendMessage,
- CallOpClientSendClose>
- init_buf_;
- CallOpSet<CallOpRecvInitialMetadata> meta_buf_;
- CallOpSet<CallOpRecvInitialMetadata, CallOpRecvMessage<R>,
- CallOpClientRecvStatus>
- finish_buf_;
+ // TODO(vjpai): Remove the reference to CallOpSetCollectionInterface
+ // as soon as the related workaround (public constructor) is deleted
+ struct Ops : public CallOpSetCollectionInterface {
+ SneakyCallOpSet<CallOpSendInitialMetadata, CallOpSendMessage,
+ CallOpClientSendClose>
+ init_buf_;
+ CallOpSet<CallOpRecvInitialMetadata> meta_buf_;
+ CallOpSet<CallOpRecvInitialMetadata, CallOpRecvMessage<R>,
+ CallOpClientRecvStatus>
+ finish_buf_;
+ } ops;
+
+ // TODO(vjpai): Remove the collection_ as soon as the related workaround
+ // (public constructor) is deleted
+ std::shared_ptr<Ops> collection_;
};
/// Async server-side API for handling unary calls, where the single
diff --git a/include/grpc++/impl/codegen/call.h b/include/grpc++/impl/codegen/call.h
index 2ded2d9776..dba53bc427 100644
--- a/include/grpc++/impl/codegen/call.h
+++ b/include/grpc++/impl/codegen/call.h
@@ -544,6 +544,11 @@ class CallOpClientRecvStatus {
grpc_slice error_message_;
};
+/// TODO(vjpai): Remove the existence of CallOpSetCollectionInterface
+/// and references to it. This code is deprecated-on-arrival and is
+/// only added for users that bypassed the code-generator.
+class CallOpSetCollectionInterface {};
+
/// An abstract collection of call ops, used to generate the
/// grpc_call_op structure to pass down to the lower layers,
/// and as it is-a CompletionQueueTag, also massages the final
@@ -554,6 +559,18 @@ class CallOpSetInterface : public CompletionQueueTag {
/// Fills in grpc_op, starting from ops[*nops] and moving
/// upwards.
virtual void FillOps(grpc_call* call, grpc_op* ops, size_t* nops) = 0;
+
+ /// TODO(vjpai): Remove the SetCollection method and comment. This is only
+ /// a short-term workaround for users that bypassed the code generator
+ /// Mark this as belonging to a collection if needed
+ void SetCollection(std::shared_ptr<CallOpSetCollectionInterface> collection) {
+ collection_ = collection;
+ }
+
+ protected:
+ /// TODO(vjpai): Remove the collection_ field once the idea of bypassing the
+ /// code generator is forbidden. This is already deprecated
+ std::shared_ptr<CallOpSetCollectionInterface> collection_;
};
/// Primary implementaiton of CallOpSetInterface.
@@ -594,6 +611,9 @@ class CallOpSet : public CallOpSetInterface,
this->Op6::FinishOp(status);
*tag = return_tag_;
g_core_codegen_interface->grpc_call_unref(call_);
+ // TODO(vjpai): Remove the reference to collection_ once the idea of
+ // bypassing the code generator is forbidden. It is already deprecated
+ collection_.reset();
return true;
}
--
GitLab