From 1f0cdd5aad5c80a5d13ff40450b8739c6702ea0c Mon Sep 17 00:00:00 2001
From: Vijay Pai <vpai@google.com>
Date: Thu, 23 Jan 2020 00:44:09 -0800
Subject: [PATCH 1/2] Construct DefaultReactor lazily since not always needed

---
 .../grpcpp/impl/codegen/server_context_impl.h | 19 ++++++++++++++++---
 .../grpcpp/test/default_reactor_test_peer.h   |  7 +++++--
 src/cpp/server/server_context.cc              |  3 +--
 3 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/include/grpcpp/impl/codegen/server_context_impl.h b/include/grpcpp/impl/codegen/server_context_impl.h
index 7458eb3aeae..37c03573b74 100644
--- a/include/grpcpp/impl/codegen/server_context_impl.h
+++ b/include/grpcpp/impl/codegen/server_context_impl.h
@@ -20,8 +20,10 @@
 #define GRPCPP_IMPL_CODEGEN_SERVER_CONTEXT_IMPL_H
 
 #include <atomic>
+#include <cassert>
 #include <map>
 #include <memory>
+#include <type_traits>
 #include <vector>
 
 #include <grpc/impl/codegen/port_platform.h>
@@ -301,8 +303,18 @@ class ServerContextBase {
   ///
   /// WARNING: This is experimental API and could be changed or removed.
   ::grpc_impl::ServerUnaryReactor* DefaultReactor() {
-    auto reactor = &default_reactor_;
+    Reactor* reactor = reinterpret_cast<Reactor*>(&default_reactor_);
+    if (test_unary_ != nullptr) {
+      return reactor;
+    }
+    new (reactor) Reactor;
+#ifndef NDEBUG
+    bool old = false;
+    assert(default_reactor_used_.compare_exchange_strong(
+        old, true, std::memory_order_relaxed));
+#else
     default_reactor_used_.store(true, std::memory_order_relaxed);
+#endif
     return reactor;
   }
 
@@ -445,7 +457,7 @@ class ServerContextBase {
    public:
     TestServerCallbackUnary(ServerContextBase* ctx,
                             std::function<void(::grpc::Status)> func)
-        : reactor_(&ctx->default_reactor_), func_(std::move(func)) {
+        : reactor_(ctx->DefaultReactor()), func_(std::move(func)) {
       this->BindReactor(reactor_);
     }
     void Finish(::grpc::Status s) override {
@@ -472,7 +484,8 @@ class ServerContextBase {
     const std::function<void(::grpc::Status s)> func_;
   };
 
-  Reactor default_reactor_;
+  typename std::aligned_storage<sizeof(Reactor), alignof(Reactor)>::type
+      default_reactor_;
   std::atomic_bool default_reactor_used_{false};
   std::unique_ptr<TestServerCallbackUnary> test_unary_;
 };
diff --git a/include/grpcpp/test/default_reactor_test_peer.h b/include/grpcpp/test/default_reactor_test_peer.h
index 6755c043333..a792e6f94ac 100644
--- a/include/grpcpp/test/default_reactor_test_peer.h
+++ b/include/grpcpp/test/default_reactor_test_peer.h
@@ -29,7 +29,9 @@ namespace testing {
 /// DefaultReactor. It is intended for allow unit-testing of a callback API
 /// service via direct invocation of the service methods rather than through
 /// RPCs. It is only applicable for unary RPC methods that use the
-/// DefaultReactor rather than any user-defined reactor.
+/// DefaultReactor rather than any user-defined reactor. If it is used, it must
+/// be created before the RPC is invoked so that it can bind the reactor into a
+/// test mode rather than letting it follow the normal paths.
 class DefaultReactorTestPeer {
  public:
   explicit DefaultReactorTestPeer(experimental::CallbackServerContext* ctx)
@@ -40,7 +42,8 @@ class DefaultReactorTestPeer {
     ctx->SetupTestDefaultReactor(std::move(finish_func));
   }
   ::grpc::experimental::ServerUnaryReactor* reactor() const {
-    return &ctx_->default_reactor_;
+    return reinterpret_cast<experimental::ServerUnaryReactor*>(
+        &ctx_->default_reactor_);
   }
   bool test_status_set() const { return ctx_->test_status_set(); }
   Status test_status() const { return ctx_->test_status(); }
diff --git a/src/cpp/server/server_context.cc b/src/cpp/server/server_context.cc
index 590a3d59c4d..1c8e1e76829 100644
--- a/src/cpp/server/server_context.cc
+++ b/src/cpp/server/server_context.cc
@@ -272,8 +272,7 @@ void ServerContextBase::Clear() {
     grpc_call_unref(call);
   }
   if (default_reactor_used_.load(std::memory_order_relaxed)) {
-    default_reactor_.~Reactor();
-    new (&default_reactor_) Reactor;
+    reinterpret_cast<Reactor*>(&default_reactor_)->~Reactor();
     default_reactor_used_.store(false, std::memory_order_relaxed);
   }
   test_unary_.reset();

From a801ccbdc81234640c89a7c64c2bcc9dafb396ef Mon Sep 17 00:00:00 2001
From: Vijay Pai <vpai@google.com>
Date: Fri, 24 Jan 2020 12:25:51 -0800
Subject: [PATCH 2/2] Reinterpret_cast after in-place new...

---
 include/grpcpp/impl/codegen/server_context_impl.h | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/grpcpp/impl/codegen/server_context_impl.h b/include/grpcpp/impl/codegen/server_context_impl.h
index 37c03573b74..02049360559 100644
--- a/include/grpcpp/impl/codegen/server_context_impl.h
+++ b/include/grpcpp/impl/codegen/server_context_impl.h
@@ -303,11 +303,12 @@ class ServerContextBase {
   ///
   /// WARNING: This is experimental API and could be changed or removed.
   ::grpc_impl::ServerUnaryReactor* DefaultReactor() {
-    Reactor* reactor = reinterpret_cast<Reactor*>(&default_reactor_);
+    // Short-circuit the case where a default reactor was already set up by
+    // the TestPeer.
     if (test_unary_ != nullptr) {
-      return reactor;
+      return reinterpret_cast<Reactor*>(&default_reactor_);
     }
-    new (reactor) Reactor;
+    new (&default_reactor_) Reactor;
 #ifndef NDEBUG
     bool old = false;
     assert(default_reactor_used_.compare_exchange_strong(
@@ -315,7 +316,7 @@ class ServerContextBase {
 #else
     default_reactor_used_.store(true, std::memory_order_relaxed);
 #endif
-    return reactor;
+    return reinterpret_cast<Reactor*>(&default_reactor_);
   }
 
   /// Constructors for use by derived classes