From cf4daebe2764cb5da664c7b2e406c48ee44dfeaa Mon Sep 17 00:00:00 2001 From: vjpai Date: Mon, 15 Feb 2016 02:33:54 -0800 Subject: Comment the requirements for changing grpc_poll_function and do poll overrides in such a way as to avoid polling races --- test/cpp/end2end/async_end2end_test.cc | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) (limited to 'test/cpp/end2end/async_end2end_test.cc') diff --git a/test/cpp/end2end/async_end2end_test.cc b/test/cpp/end2end/async_end2end_test.cc index a194c615cd..78417fc6d0 100644 --- a/test/cpp/end2end/async_end2end_test.cc +++ b/test/cpp/end2end/async_end2end_test.cc @@ -86,21 +86,21 @@ class PollOverride { grpc_poll_function_type prev_; }; -class PollingCheckRegion : public PollOverride { +class PollingOverrider : public PollOverride { public: - explicit PollingCheckRegion(bool allow_blocking) + explicit PollingOverrider(bool allow_blocking) : PollOverride(allow_blocking ? poll : assert_non_blocking_poll) {} }; #else -class PollingCheckRegion { +class PollingOverrider { public: - explicit PollingCheckRegion(bool allow_blocking) {} + explicit PollingOverrider(bool allow_blocking) {} }; #endif -class Verifier : public PollingCheckRegion { +class Verifier { public: - explicit Verifier(bool spin) : PollingCheckRegion(!spin), spin_(spin) {} + explicit Verifier(bool spin) : spin_(spin) {} Verifier& Expect(int i, bool expect_ok) { expectations_[tag(i)] = expect_ok; return *this; @@ -180,7 +180,7 @@ class Verifier : public PollingCheckRegion { class AsyncEnd2endTest : public ::testing::TestWithParam { protected: - AsyncEnd2endTest() {} + AsyncEnd2endTest(): poll_override_(GetParam()) {} void SetUp() GRPC_OVERRIDE { int port = grpc_pick_unused_port_or_die(); @@ -249,6 +249,8 @@ class AsyncEnd2endTest : public ::testing::TestWithParam { std::unique_ptr server_; grpc::testing::EchoTestService::AsyncService service_; std::ostringstream server_address_; + + PollingOverrider poll_override_; }; TEST_P(AsyncEnd2endTest, SimpleRpc) { -- cgit v1.2.3 From 018879aa9a37fa2351c71d6699e71743a92f6a18 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Tue, 16 Feb 2016 09:20:50 -0800 Subject: Set up poll overrides --- test/cpp/end2end/async_end2end_test.cc | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) (limited to 'test/cpp/end2end/async_end2end_test.cc') diff --git a/test/cpp/end2end/async_end2end_test.cc b/test/cpp/end2end/async_end2end_test.cc index 78417fc6d0..aa99061fe9 100644 --- a/test/cpp/end2end/async_end2end_test.cc +++ b/test/cpp/end2end/async_end2end_test.cc @@ -67,10 +67,10 @@ namespace { void* tag(int i) { return (void*)(intptr_t)i; } #ifdef GPR_POSIX_SOCKET -static int assert_non_blocking_poll(struct pollfd* pfds, nfds_t nfds, +static int non_blocking_poll(struct pollfd* pfds, nfds_t nfds, int timeout) { - GPR_ASSERT(timeout == 0); - return poll(pfds, nfds, timeout); + /* ignore timeout and always use timeout 0 */ + return poll(pfds, nfds, 0); } class PollOverride { @@ -89,7 +89,7 @@ class PollOverride { class PollingOverrider : public PollOverride { public: explicit PollingOverrider(bool allow_blocking) - : PollOverride(allow_blocking ? poll : assert_non_blocking_poll) {} + : PollOverride(allow_blocking ? poll : non_blocking_poll) {} }; #else class PollingOverrider { @@ -180,9 +180,11 @@ class Verifier { class AsyncEnd2endTest : public ::testing::TestWithParam { protected: - AsyncEnd2endTest(): poll_override_(GetParam()) {} + AsyncEnd2endTest() {} void SetUp() GRPC_OVERRIDE { + poll_overrider_.reset(new PollingOverrider(!GetParam())); + int port = grpc_pick_unused_port_or_die(); server_address_ << "localhost:" << port; @@ -202,6 +204,7 @@ class AsyncEnd2endTest : public ::testing::TestWithParam { cq_->Shutdown(); while (cq_->Next(&ignored_tag, &ignored_ok)) ; + poll_overrider_.reset(); } void ResetStub() { @@ -250,7 +253,7 @@ class AsyncEnd2endTest : public ::testing::TestWithParam { grpc::testing::EchoTestService::AsyncService service_; std::ostringstream server_address_; - PollingOverrider poll_override_; + std::unique_ptr poll_overrider_; }; TEST_P(AsyncEnd2endTest, SimpleRpc) { @@ -1089,7 +1092,7 @@ class AsyncEnd2endServerTryCancelTest : public AsyncEnd2endTest { Verifier(GetParam()).Expect(7, true).Verify(cq_.get()); // This is expected to fail in all cases i.e for all values of - // server_try_cancel. This is becasue at this point, either there are no + // server_try_cancel. This is because at this point, either there are no // more msgs from the client (because client called WritesDone) or the RPC // is cancelled on the server srv_stream.Read(&recv_request, tag(8)); -- cgit v1.2.3 From b65eda4c8844f98dcf2d0616406b746ac08e861d Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Tue, 16 Feb 2016 13:48:05 -0800 Subject: Make the poll override function assert on non-zero timeout if invoked from the end2end test thread itself (done by adding a TLS on that thread) Also clang-format --- test/cpp/end2end/async_end2end_test.cc | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) (limited to 'test/cpp/end2end/async_end2end_test.cc') diff --git a/test/cpp/end2end/async_end2end_test.cc b/test/cpp/end2end/async_end2end_test.cc index aa99061fe9..a15cbd7ee2 100644 --- a/test/cpp/end2end/async_end2end_test.cc +++ b/test/cpp/end2end/async_end2end_test.cc @@ -43,6 +43,7 @@ #include #include #include +#include #include #include "src/proto/grpc/testing/duplicate/echo_duplicate.grpc.pb.h" @@ -59,6 +60,8 @@ using grpc::testing::EchoRequest; using grpc::testing::EchoResponse; using std::chrono::system_clock; +GPR_TLS_DECL(g_is_async_end2end_test); + namespace grpc { namespace testing { @@ -67,10 +70,12 @@ namespace { void* tag(int i) { return (void*)(intptr_t)i; } #ifdef GPR_POSIX_SOCKET -static int non_blocking_poll(struct pollfd* pfds, nfds_t nfds, - int timeout) { - /* ignore timeout and always use timeout 0 */ - return poll(pfds, nfds, 0); +static int maybe_assert_non_blocking_poll(struct pollfd* pfds, nfds_t nfds, + int timeout) { + if (gpr_tls_get(&g_is_async_end2end_test)) { + GPR_ASSERT(timeout == 0); + } + return poll(pfds, nfds, timeout); } class PollOverride { @@ -89,7 +94,7 @@ class PollOverride { class PollingOverrider : public PollOverride { public: explicit PollingOverrider(bool allow_blocking) - : PollOverride(allow_blocking ? poll : non_blocking_poll) {} + : PollOverride(allow_blocking ? poll : maybe_assert_non_blocking_poll) {} }; #else class PollingOverrider { @@ -195,6 +200,8 @@ class AsyncEnd2endTest : public ::testing::TestWithParam { builder.RegisterService(&service_); cq_ = builder.AddCompletionQueue(); server_ = builder.BuildAndStart(); + + gpr_tls_set(&g_is_async_end2end_test, 1); } void TearDown() GRPC_OVERRIDE { @@ -205,6 +212,7 @@ class AsyncEnd2endTest : public ::testing::TestWithParam { while (cq_->Next(&ignored_tag, &ignored_ok)) ; poll_overrider_.reset(); + gpr_tls_set(&g_is_async_end2end_test, 0); } void ResetStub() { @@ -1169,6 +1177,9 @@ INSTANTIATE_TEST_CASE_P(AsyncEnd2endServerTryCancel, int main(int argc, char** argv) { grpc_test_init(argc, argv); + gpr_tls_init(&g_is_async_end2end_test); ::testing::InitGoogleTest(&argc, argv); - return RUN_ALL_TESTS(); + int ret = RUN_ALL_TESTS(); + gpr_tls_destroy(&g_is_async_end2end_test); + return ret; } -- cgit v1.2.3 From ad0df7bf1fa9a2ad6302016cfbe0b86793f810c5 Mon Sep 17 00:00:00 2001 From: yang-g Date: Mon, 22 Feb 2016 10:00:20 -0800 Subject: Discard the read buffer on stream error --- src/core/transport/chttp2_transport.c | 5 +++++ test/cpp/end2end/async_end2end_test.cc | 4 ++++ 2 files changed, 9 insertions(+) (limited to 'test/cpp/end2end/async_end2end_test.cc') diff --git a/src/core/transport/chttp2_transport.c b/src/core/transport/chttp2_transport.c index 617d98875c..c3efc36cc5 100644 --- a/src/core/transport/chttp2_transport.c +++ b/src/core/transport/chttp2_transport.c @@ -1019,6 +1019,11 @@ static void check_read_ops(grpc_exec_ctx *exec_ctx, stream_global->recv_initial_metadata_ready = NULL; } if (stream_global->recv_message_ready != NULL) { + while (stream_global->seen_error && + (bs = grpc_chttp2_incoming_frame_queue_pop( + &stream_global->incoming_frames)) != NULL) { + grpc_byte_stream_destroy(exec_ctx, bs); + } if (stream_global->incoming_frames.head != NULL) { *stream_global->recv_message = grpc_chttp2_incoming_frame_queue_pop( &stream_global->incoming_frames); diff --git a/test/cpp/end2end/async_end2end_test.cc b/test/cpp/end2end/async_end2end_test.cc index a15cbd7ee2..9ca3bf98f8 100644 --- a/test/cpp/end2end/async_end2end_test.cc +++ b/test/cpp/end2end/async_end2end_test.cc @@ -989,6 +989,10 @@ class AsyncEnd2endServerTryCancelTest : public AsyncEnd2endTest { if (server_try_cancel == CANCEL_AFTER_PROCESSING) { ServerTryCancel(&srv_ctx); + + // Client reads may fail bacause it is notified that the stream is + // cancelled. + ignore_cq_result = true; } // Client attemts to read the three messages from the server -- cgit v1.2.3