From 6dfe27ab0823a1dcac2953afdbf404095d7a3320 Mon Sep 17 00:00:00 2001
From: Hope Casey-Allen <hcaseyal@google.com>
Date: Tue, 27 Aug 2019 15:09:17 -0700
Subject: [PATCH] Fix race in bm_chttp2_transport

---
 test/cpp/microbenchmarks/BUILD                |  8 ++++
 .../microbenchmarks/bm_chttp2_transport.cc    | 40 +++++++++++++++++--
 2 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/test/cpp/microbenchmarks/BUILD b/test/cpp/microbenchmarks/BUILD
index b8e9b14d4b4..fdae8246c31 100644
--- a/test/cpp/microbenchmarks/BUILD
+++ b/test/cpp/microbenchmarks/BUILD
@@ -27,6 +27,14 @@ grpc_cc_test(
     deps = ["//test/core/util:grpc_test_util"],
 )
 
+grpc_cc_binary(
+    name = "bm_chttp2_transport",
+    testonly = 1,
+    srcs = ["bm_chttp2_transport.cc"],
+    tags = ["no_windows"],
+    deps = [":helpers"],
+)
+
 grpc_cc_library(
     name = "helpers",
     testonly = 1,
diff --git a/test/cpp/microbenchmarks/bm_chttp2_transport.cc b/test/cpp/microbenchmarks/bm_chttp2_transport.cc
index da3357304ba..dc3d76ee38e 100644
--- a/test/cpp/microbenchmarks/bm_chttp2_transport.cc
+++ b/test/cpp/microbenchmarks/bm_chttp2_transport.cc
@@ -239,7 +239,7 @@ class Stream {
     grpc_transport_destroy_stream(stream->f_->transport(),
                                   static_cast<grpc_stream*>(stream->stream_),
                                   stream->destroy_closure_);
-    gpr_event_set(&stream->done_, (void*)1);
+    gpr_event_set(&stream->done_, (void*)(1));
   }
 
   Fixture* f_;
@@ -254,6 +254,7 @@ class Stream {
 ////////////////////////////////////////////////////////////////////////////////
 // Benchmarks
 //
+std::vector<std::unique_ptr<gpr_event>> done_events;
 
 static void BM_StreamCreateDestroy(benchmark::State& state) {
   TrackCounters track_counters;
@@ -380,15 +381,24 @@ static void BM_TransportEmptyOp(benchmark::State& state) {
   reset_op();
   op.cancel_stream = true;
   op_payload.cancel_stream.cancel_error = GRPC_ERROR_CANCELLED;
+  gpr_event* stream_cancel_done = new gpr_event;
+  gpr_event_init(stream_cancel_done);
+  std::unique_ptr<Closure> stream_cancel_closure =
+      MakeClosure([&](grpc_error* error) {
+        GPR_ASSERT(error == GRPC_ERROR_NONE);
+        gpr_event_set(stream_cancel_done, (void*)(1));
+      });
+  op.on_complete = stream_cancel_closure.get();
   s->Op(&op);
+  f.FlushExecCtx();
+  gpr_event_wait(stream_cancel_done, gpr_inf_future(GPR_CLOCK_REALTIME));
+  done_events.emplace_back(stream_cancel_done);
   s->DestroyThen(MakeOnceClosure([s](grpc_error* error) { delete s; }));
   f.FlushExecCtx();
   track_counters.Finish(state);
 }
 BENCHMARK(BM_TransportEmptyOp);
 
-std::vector<std::unique_ptr<gpr_event>> done_events;
-
 static void BM_TransportStreamSend(benchmark::State& state) {
   TrackCounters track_counters;
   grpc_core::ExecCtx exec_ctx;
@@ -424,7 +434,7 @@ static void BM_TransportStreamSend(benchmark::State& state) {
 
   std::unique_ptr<Closure> c = MakeClosure([&](grpc_error* error) {
     if (!state.KeepRunning()) {
-      gpr_event_set(bm_done, (void*)1);
+      gpr_event_set(bm_done, (void*)(1));
       return;
     }
     grpc_slice_buffer send_buffer;
@@ -455,7 +465,18 @@ static void BM_TransportStreamSend(benchmark::State& state) {
   reset_op();
   op.cancel_stream = true;
   op.payload->cancel_stream.cancel_error = GRPC_ERROR_CANCELLED;
+  gpr_event* stream_cancel_done = new gpr_event;
+  gpr_event_init(stream_cancel_done);
+  std::unique_ptr<Closure> stream_cancel_closure =
+      MakeClosure([&](grpc_error* error) {
+        GPR_ASSERT(error == GRPC_ERROR_NONE);
+        gpr_event_set(stream_cancel_done, (void*)(1));
+      });
+  op.on_complete = stream_cancel_closure.get();
   s->Op(&op);
+  f.FlushExecCtx();
+  gpr_event_wait(stream_cancel_done, gpr_inf_future(GPR_CLOCK_REALTIME));
+  done_events.emplace_back(stream_cancel_done);
   s->DestroyThen(MakeOnceClosure([s](grpc_error* error) { delete s; }));
   f.FlushExecCtx();
   track_counters.Finish(state);
@@ -629,7 +650,18 @@ static void BM_TransportStreamRecv(benchmark::State& state) {
   reset_op();
   op.cancel_stream = true;
   op.payload->cancel_stream.cancel_error = GRPC_ERROR_CANCELLED;
+  gpr_event* stream_cancel_done = new gpr_event;
+  gpr_event_init(stream_cancel_done);
+  std::unique_ptr<Closure> stream_cancel_closure =
+      MakeClosure([&](grpc_error* error) {
+        GPR_ASSERT(error == GRPC_ERROR_NONE);
+        gpr_event_set(stream_cancel_done, (void*)(1));
+      });
+  op.on_complete = stream_cancel_closure.get();
   s->Op(&op);
+  f.FlushExecCtx();
+  gpr_event_wait(stream_cancel_done, gpr_inf_future(GPR_CLOCK_REALTIME));
+  done_events.emplace_back(stream_cancel_done);
   s->DestroyThen(MakeOnceClosure([s](grpc_error* error) { delete s; }));
   grpc_metadata_batch_destroy(&b);
   grpc_metadata_batch_destroy(&b_recv);