Address reviewer comments

This commit is contained in:
Vijay Pai 2019-12-13 15:17:34 -08:00
parent 9be35f74c0
commit 24483d2afd
4 changed files with 15 additions and 13 deletions

View File

@ -50,7 +50,7 @@ message RequestParams {
int32 server_sleep_us = 15; // sleep when invoking server for deadline tests
int32 backend_channel_idx = 16; // which backend to send request to
bool echo_metadata_initially = 17;
bool server_notify_started = 18;
bool server_notify_client_when_started = 18;
}
message EchoRequest {

View File

@ -1131,14 +1131,14 @@ TEST_P(End2endTest, CancelRpcBeforeStart) {
}
}
TEST_P(End2endTest, CancelDelayedRpc) {
TEST_P(End2endTest, CancelRpcAfterStart) {
MAYBE_SKIP_TEST;
ResetStub();
EchoRequest request;
EchoResponse response;
ClientContext context;
request.set_message("hello");
request.mutable_param()->set_server_notify_started(true);
request.mutable_param()->set_server_notify_client_when_started(true);
request.mutable_param()->set_skip_cancelled_check(true);
Status s;
std::thread echo_thread([this, &s, &context, &request, &response] {
@ -1146,9 +1146,9 @@ TEST_P(End2endTest, CancelDelayedRpc) {
EXPECT_EQ(StatusCode::CANCELLED, s.error_code());
});
if (!GetParam().callback_server) {
service_.ClientWaitRpcStarted();
service_.ClientWaitUntilRpcStarted();
} else {
callback_service_.ClientWaitRpcStarted();
callback_service_.ClientWaitUntilRpcStarted();
}
context.TryCancel();

View File

@ -127,8 +127,9 @@ void ServerTryCancelNonblocking(experimental::CallbackServerContext* context) {
Status TestServiceImpl::Echo(ServerContext* context, const EchoRequest* request,
EchoResponse* response) {
if (request->has_param() && request->param().server_notify_started()) {
signaller_.SignalClientRpcStarted();
if (request->has_param() &&
request->param().server_notify_client_when_started()) {
signaller_.SignalClientThatRpcStarted();
signaller_.ServerWaitToContinue();
}
@ -427,8 +428,9 @@ experimental::ServerUnaryReactor* CallbackTestServiceImpl::Echo(
started_ = true;
if (request->has_param() && request->param().server_notify_started()) {
service->signaller_.SignalClientRpcStarted();
if (request->has_param() &&
request->param().server_notify_client_when_started()) {
service->signaller_.SignalClientThatRpcStarted();
// Block on the "wait to continue" decision in a different thread since
// we can't tie up an EM thread with blocking events. We can join it in
// OnDone since it would definitely be done by then.

View File

@ -49,7 +49,7 @@ typedef enum {
class TestServiceSignaller {
public:
void ClientWaitRpcStarted() {
void ClientWaitUntilRpcStarted() {
std::unique_lock<std::mutex> lock(mu_);
cv_rpc_started_.wait(lock, [this] { return rpc_started_; });
}
@ -57,7 +57,7 @@ class TestServiceSignaller {
std::unique_lock<std::mutex> lock(mu_);
cv_server_continue_.wait(lock, [this] { return server_should_continue_; });
}
void SignalClientRpcStarted() {
void SignalClientThatRpcStarted() {
std::unique_lock<std::mutex> lock(mu_);
rpc_started_ = true;
cv_rpc_started_.notify_one();
@ -106,7 +106,7 @@ class TestServiceImpl : public ::grpc::testing::EchoTestService::Service {
std::unique_lock<std::mutex> lock(mu_);
return signal_client_;
}
void ClientWaitRpcStarted() { signaller_.ClientWaitRpcStarted(); }
void ClientWaitUntilRpcStarted() { signaller_.ClientWaitUntilRpcStarted(); }
void SignalServerToContinue() { signaller_.SignalServerToContinue(); }
private:
@ -147,7 +147,7 @@ class CallbackTestServiceImpl
std::unique_lock<std::mutex> lock(mu_);
return signal_client_;
}
void ClientWaitRpcStarted() { signaller_.ClientWaitRpcStarted(); }
void ClientWaitUntilRpcStarted() { signaller_.ClientWaitUntilRpcStarted(); }
void SignalServerToContinue() { signaller_.SignalServerToContinue(); }
private: