Address review comments

pull/19406/head
Karthik Ravi Shankar 6 years ago
parent 514413de70
commit 2602bdd3e4
  1. 4
      src/core/lib/surface/completion_queue.cc
  2. 28
      test/cpp/end2end/client_callback_end2end_test.cc

@ -864,7 +864,7 @@ static void cq_end_op_for_callback(
return; return;
} }
// Schedule the shutdown callback on a closure if not internal or triggered // Schedule the callback on a closure if not internal or triggered
// from a background poller thread. // from a background poller thread.
GRPC_CLOSURE_SCHED( GRPC_CLOSURE_SCHED(
GRPC_CLOSURE_CREATE( GRPC_CLOSURE_CREATE(
@ -1360,7 +1360,7 @@ static void cq_finish_shutdown_callback(grpc_completion_queue* cq) {
return; return;
} }
// Schedule the shutdown callback on a closure if not internal or triggered // Schedule the callback on a closure if not internal or triggered
// from a background poller thread. // from a background poller thread.
GRPC_CLOSURE_SCHED( GRPC_CLOSURE_SCHED(
GRPC_CLOSURE_CREATE( GRPC_CLOSURE_CREATE(

@ -378,8 +378,8 @@ TEST_P(ClientCallbackEnd2endTest, SimpleRpcUnderLockNested) {
MAYBE_SKIP_TEST; MAYBE_SKIP_TEST;
ResetStub(); ResetStub();
std::mutex mu1, mu2, mu3; std::mutex mu1, mu2, mu3;
std::condition_variable cv1, cv2, cv3; std::condition_variable cv;
bool done1 = false; bool done = false;
EchoRequest request1, request2, request3; EchoRequest request1, request2, request3;
request1.set_message("Hello locked world1."); request1.set_message("Hello locked world1.");
request2.set_message("Hello locked world2."); request2.set_message("Hello locked world2.");
@ -387,42 +387,42 @@ TEST_P(ClientCallbackEnd2endTest, SimpleRpcUnderLockNested) {
EchoResponse response1, response2, response3; EchoResponse response1, response2, response3;
ClientContext cli_ctx1, cli_ctx2, cli_ctx3; ClientContext cli_ctx1, cli_ctx2, cli_ctx3;
{ {
std::unique_lock<std::mutex> l(mu1); std::lock_guard<std::mutex> l(mu1);
stub_->experimental_async()->Echo( stub_->experimental_async()->Echo(
&cli_ctx1, &request1, &response1, &cli_ctx1, &request1, &response1,
[this, &mu1, &mu2, &mu3, &cv1, &done1, &request1, &request2, &request3, [this, &mu1, &mu2, &mu3, &cv, &done, &request1, &request2, &request3,
&response1, &response2, &response3, &cli_ctx1, &cli_ctx2, &response1, &response2, &response3, &cli_ctx1, &cli_ctx2,
&cli_ctx3](Status s1) { &cli_ctx3](Status s1) {
std::unique_lock<std::mutex> l1(mu1); std::lock_guard<std::mutex> l1(mu1);
EXPECT_TRUE(s1.ok()); EXPECT_TRUE(s1.ok());
EXPECT_EQ(request1.message(), response1.message()); EXPECT_EQ(request1.message(), response1.message());
// start the second level of nesting // start the second level of nesting
std::unique_lock<std::mutex> l2(mu2); std::unique_lock<std::mutex> l2(mu2);
this->stub_->experimental_async()->Echo( this->stub_->experimental_async()->Echo(
&cli_ctx2, &request2, &response2, &cli_ctx2, &request2, &response2,
[this, &mu2, &mu3, &cv1, &done1, &request2, &request3, &response2, [this, &mu2, &mu3, &cv, &done, &request2, &request3, &response2,
&response3, &cli_ctx3](Status s2) { &response3, &cli_ctx3](Status s2) {
std::unique_lock<std::mutex> l2(mu2); std::lock_guard<std::mutex> l2(mu2);
EXPECT_TRUE(s2.ok()); EXPECT_TRUE(s2.ok());
EXPECT_EQ(request2.message(), response2.message()); EXPECT_EQ(request2.message(), response2.message());
// start the third level of nesting // start the third level of nesting
std::unique_lock<std::mutex> l3(mu3); std::lock_guard<std::mutex> l3(mu3);
stub_->experimental_async()->Echo( stub_->experimental_async()->Echo(
&cli_ctx3, &request3, &response3, &cli_ctx3, &request3, &response3,
[&mu3, &cv1, &done1, &request3, &response3](Status s3) { [&mu3, &cv, &done, &request3, &response3](Status s3) {
std::lock_guard<std::mutex> l(mu3); std::lock_guard<std::mutex> l(mu3);
EXPECT_TRUE(s3.ok()); EXPECT_TRUE(s3.ok());
EXPECT_EQ(request3.message(), response3.message()); EXPECT_EQ(request3.message(), response3.message());
done1 = true; done = true;
cv1.notify_all(); cv.notify_all();
}); });
}); });
}); });
} }
std::unique_lock<std::mutex> l1(mu1); std::unique_lock<std::mutex> l(mu3);
while (!done1) { while (!done) {
cv1.wait(l1); cv.wait(l);
} }
} }

Loading…
Cancel
Save