Merge pull request #14792 from vjpai/desneak2

Eliminate existence of SneakyCallOpSet, some server cleanup

include/grpcpp/impl/codegen/call.h

@@ -656,21 +656,6 @@ class CallOpSet : public CallOpSetInterface,
   grpc_call* call_;
 };
 
-/// A CallOpSet that does not post completions to the completion queue.
-///
-/// Allows hiding some completions that the C core must generate from
-/// C++ users.
-template <class Op1 = CallNoOp<1>, class Op2 = CallNoOp<2>,
-          class Op3 = CallNoOp<3>, class Op4 = CallNoOp<4>,
-          class Op5 = CallNoOp<5>, class Op6 = CallNoOp<6>>
-class SneakyCallOpSet : public CallOpSet<Op1, Op2, Op3, Op4, Op5, Op6> {
- public:
-  bool FinalizeResult(void** tag, bool* status) override {
-    typedef CallOpSet<Op1, Op2, Op3, Op4, Op5, Op6> Base;
-    return Base::FinalizeResult(tag, status) && false;
-  }
-};
-
 /// Straightforward wrapping of the C call object
 class Call final {
  public:

include/grpcpp/server.h

@@ -162,8 +162,8 @@ class Server : public ServerInterface, private GrpcLibraryCodegen {
   friend class ServerInitializer;
 
   class SyncRequest;
-  class AsyncRequest;
-  class ShutdownRequest;
+  class UnimplementedAsyncRequest;
+  class UnimplementedAsyncResponse;
 
   /// SyncRequestThreadManager is an implementation of ThreadManager. This class
   /// is responsible for polling for incoming RPCs and calling the RPC handlers.
@@ -171,10 +171,6 @@ class Server : public ServerInterface, private GrpcLibraryCodegen {
   /// interface)
   class SyncRequestThreadManager;
 
-  class UnimplementedAsyncRequestContext;
-  class UnimplementedAsyncRequest;
-  class UnimplementedAsyncResponse;
-
   /// Register a generic service. This call does not take ownership of the
   /// service. The service must exist for the lifetime of the Server instance.
   void RegisterAsyncGenericService(AsyncGenericService* service) override;

src/cpp/server/server_cc.cc

@@ -45,6 +45,7 @@
 #include "src/cpp/thread_manager/thread_manager.h"
 
 namespace grpc {
+namespace {
 
 class DefaultGlobalCallbacks final : public Server::GlobalCallbacks {
  public:
@@ -53,16 +54,29 @@ class DefaultGlobalCallbacks final : public Server::GlobalCallbacks {
   void PostSynchronousRequest(ServerContext* context) override {}
 };
 
-static std::shared_ptr<Server::GlobalCallbacks> g_callbacks = nullptr;
-static gpr_once g_once_init_callbacks = GPR_ONCE_INIT;
+std::shared_ptr<Server::GlobalCallbacks> g_callbacks = nullptr;
+gpr_once g_once_init_callbacks = GPR_ONCE_INIT;
 
-static void InitGlobalCallbacks() {
+void InitGlobalCallbacks() {
   if (!g_callbacks) {
     g_callbacks.reset(new DefaultGlobalCallbacks());
   }
 }
 
-class Server::UnimplementedAsyncRequestContext {
+class ShutdownTag : public internal::CompletionQueueTag {
+ public:
+  bool FinalizeResult(void** tag, bool* status) { return false; }
+};
+
+class DummyTag : public internal::CompletionQueueTag {
+ public:
+  bool FinalizeResult(void** tag, bool* status) {
+    *status = true;
+    return true;
+  }
+};
+
+class UnimplementedAsyncRequestContext {
  protected:
   UnimplementedAsyncRequestContext() : generic_stream_(&server_context_) {}
 
@@ -70,8 +84,14 @@ class Server::UnimplementedAsyncRequestContext {
   GenericServerAsyncReaderWriter generic_stream_;
 };
 
+}  // namespace
+
+/// Use private inheritance rather than composition only to establish order
+/// of construction, since the public base class should be constructed after the
+/// elements belonging to the private base class are constructed. This is not
+/// possible using true composition.
 class Server::UnimplementedAsyncRequest final
-    : public UnimplementedAsyncRequestContext,
+    : private UnimplementedAsyncRequestContext,
       public GenericAsyncRequest {
  public:
   UnimplementedAsyncRequest(Server* server, ServerCompletionQueue* cq)
@@ -90,38 +110,27 @@ class Server::UnimplementedAsyncRequest final
   ServerCompletionQueue* const cq_;
 };
 
-typedef internal::SneakyCallOpSet<internal::CallOpSendInitialMetadata,
-                                  internal::CallOpServerSendStatus>
-    UnimplementedAsyncResponseOp;
+/// UnimplementedAsyncResponse should not post user-visible completions to the
+/// C++ completion queue, but is generated as a CQ event by the core
 class Server::UnimplementedAsyncResponse final
-    : public UnimplementedAsyncResponseOp {
+    : public internal::CallOpSet<internal::CallOpSendInitialMetadata,
+                                 internal::CallOpServerSendStatus> {
  public:
   UnimplementedAsyncResponse(UnimplementedAsyncRequest* request);
   ~UnimplementedAsyncResponse() { delete request_; }
 
   bool FinalizeResult(void** tag, bool* status) override {
-    bool r = UnimplementedAsyncResponseOp::FinalizeResult(tag, status);
+    internal::CallOpSet<
+        internal::CallOpSendInitialMetadata,
+        internal::CallOpServerSendStatus>::FinalizeResult(tag, status);
     delete this;
-    return r;
+    return false;
   }
 
  private:
   UnimplementedAsyncRequest* const request_;
 };
 
-class ShutdownTag : public internal::CompletionQueueTag {
- public:
-  bool FinalizeResult(void** tag, bool* status) { return false; }
-};
-
-class DummyTag : public internal::CompletionQueueTag {
- public:
-  bool FinalizeResult(void** tag, bool* status) {
-    *status = true;
-    return true;
-  }
-};
-
 class Server::SyncRequest final : public internal::CompletionQueueTag {
  public:
   SyncRequest(internal::RpcServiceMethod* method, void* tag)