diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc index 86560ae324c8d..4ec4ae40aafbf 100644 --- a/test/integration/fake_upstream.cc +++ b/test/integration/fake_upstream.cc @@ -119,6 +119,7 @@ void FakeStream::waitForData(Event::Dispatcher& client_dispatcher, uint64_t body } void FakeStream::waitForEndStream(Event::Dispatcher& client_dispatcher) { + ENVOY_LOG(trace, "FakeStream waiting for end stream"); Thread::LockGuard lock(lock_); while (!end_stream_) { decoder_event_.waitFor(lock_, std::chrono::milliseconds(5)); @@ -127,6 +128,7 @@ void FakeStream::waitForEndStream(Event::Dispatcher& client_dispatcher) { client_dispatcher.run(Event::Dispatcher::RunType::NonBlock); } } + ENVOY_LOG(trace, "FakeStream done waiting for end stream"); } void FakeStream::waitForReset() { @@ -185,6 +187,7 @@ Http::StreamDecoder& FakeHttpConnection::newStream(Http::StreamEncoder& encoder) } void FakeConnectionBase::waitForDisconnect(bool ignore_spurious_events) { + ENVOY_LOG(trace, "FakeConnectionBase waiting for disconnect"); Thread::LockGuard lock(lock_); while (shared_connection_.connected()) { connection_event_.wait(lock_); // Safe since CondVar::wait won't throw. @@ -199,6 +202,7 @@ void FakeConnectionBase::waitForDisconnect(bool ignore_spurious_events) { } ASSERT(!shared_connection_.connected()); + ENVOY_LOG(trace, "FakeConnectionBase done waiting for disconnect"); } void FakeConnectionBase::waitForHalfClose(bool ignore_spurious_events) { diff --git a/test/integration/fake_upstream.h b/test/integration/fake_upstream.h index ec82d2be25936..a95965e3454d9 100644 --- a/test/integration/fake_upstream.h +++ b/test/integration/fake_upstream.h @@ -263,7 +263,7 @@ class QueuedConnectionWrapper : public LinkedObject { /** * Base class for both fake raw connections and fake HTTP connections. */ -class FakeConnectionBase { +class FakeConnectionBase : public Logger::Loggable { public: virtual ~FakeConnectionBase() { ASSERT(initialized_); @@ -341,7 +341,7 @@ typedef std::unique_ptr FakeHttpConnectionPtr; /** * Fake raw connection for integration testing. */ -class FakeRawConnection : Logger::Loggable, public FakeConnectionBase { +class FakeRawConnection : public FakeConnectionBase { public: FakeRawConnection(SharedConnectionWrapper& shared_connection) : FakeConnectionBase(shared_connection) {} diff --git a/test/integration/hds_integration_test.cc b/test/integration/hds_integration_test.cc index 6e8235785048f..e81322cbb9f63 100644 --- a/test/integration/hds_integration_test.cc +++ b/test/integration/hds_integration_test.cc @@ -66,14 +66,6 @@ class HdsIntegrationTest : public HttpIntegrationTest, test_server_->waitForCounterGe("hds_delegate.requests", ++hds_requests_); } - void cleanupUpstreamConnection() { - codec_client_->close(); - if (fake_upstream_connection_ != nullptr) { - fake_upstream_connection_->close(); - fake_upstream_connection_->waitForDisconnect(); - } - } - void cleanupHdsConnection() { if (fake_hds_connection_ != nullptr) { fake_hds_connection_->close(); diff --git a/test/integration/http2_integration_test.cc b/test/integration/http2_integration_test.cc index bc46cbb0af719..ae04ba0db8291 100644 --- a/test/integration/http2_integration_test.cc +++ b/test/integration/http2_integration_test.cc @@ -368,11 +368,11 @@ void Http2IntegrationTest::simultaneousRequest(int32_t request1_bytes, int32_t r EXPECT_EQ(request2_bytes, response1->body().size()); // Cleanup both downstream and upstream - codec_client_->close(); fake_upstream_connection1->close(); fake_upstream_connection1->waitForDisconnect(); fake_upstream_connection2->close(); fake_upstream_connection2->waitForDisconnect(); + codec_client_->close(); } TEST_P(Http2IntegrationTest, SimultaneousRequest) { simultaneousRequest(1024, 512); } diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index 8524c71d736da..7275b0342591b 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -214,13 +214,17 @@ IntegrationStreamDecoderPtr HttpIntegrationTest::sendRequestAndWaitForResponse( } void HttpIntegrationTest::cleanupUpstreamAndDownstream() { - if (codec_client_) { - codec_client_->close(); - } + // Close the upstream connection first. If there's an outstanding request, + // closing the client may result in a FIN being sent upstream, and FakeConnectionBase::close + // will interpret that as an unexpected disconnect. The codec client is not + // subject to the same failure mode. if (fake_upstream_connection_) { fake_upstream_connection_->close(); fake_upstream_connection_->waitForDisconnect(); } + if (codec_client_) { + codec_client_->close(); + } } void HttpIntegrationTest::waitForNextUpstreamRequest(uint64_t upstream_index) { diff --git a/test/integration/integration_test.cc b/test/integration/integration_test.cc index 1d94b5021b84b..c704ebee07bb9 100644 --- a/test/integration/integration_test.cc +++ b/test/integration/integration_test.cc @@ -277,7 +277,6 @@ TEST_P(IntegrationTest, TestBind) { initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); - // Request 1. auto response = codec_client_->makeRequestWithBody(Http::TestHeaderMapImpl{{":method", "GET"}, @@ -285,17 +284,14 @@ TEST_P(IntegrationTest, TestBind) { {":scheme", "http"}, {":authority", "host"}}, 1024); - fake_upstream_connection_ = fake_upstreams_[0]->waitForHttpConnection(*dispatcher_); std::string address = fake_upstream_connection_->connection().remoteAddress()->ip()->addressAsString(); EXPECT_EQ(address, address_string); upstream_request_ = fake_upstream_connection_->waitForNewStream(*dispatcher_); upstream_request_->waitForEndStream(*dispatcher_); - // Cleanup both downstream and upstream - codec_client_->close(); - fake_upstream_connection_->close(); - fake_upstream_connection_->waitForDisconnect(); + + cleanupUpstreamAndDownstream(); } TEST_P(IntegrationTest, TestFailedBind) { diff --git a/test/integration/load_stats_integration_test.cc b/test/integration/load_stats_integration_test.cc index e0011f0c4aad5..ef451d5a572cf 100644 --- a/test/integration/load_stats_integration_test.cc +++ b/test/integration/load_stats_integration_test.cc @@ -277,14 +277,6 @@ class LoadStatsIntegrationTest : public HttpIntegrationTest, return locality_stats; } - void cleanupUpstreamConnection() { - codec_client_->close(); - if (fake_upstream_connection_ != nullptr) { - fake_upstream_connection_->close(); - fake_upstream_connection_->waitForDisconnect(); - } - } - void cleanupLoadStatsConnection() { if (fake_loadstats_connection_ != nullptr) { fake_loadstats_connection_->close(); @@ -295,7 +287,7 @@ class LoadStatsIntegrationTest : public HttpIntegrationTest, void sendAndReceiveUpstream(uint32_t endpoint_index, uint32_t response_code = 200) { initiateClientConnection(); waitForUpstreamResponse(endpoint_index, response_code); - cleanupUpstreamConnection(); + cleanupUpstreamAndDownstream(); } static constexpr uint32_t upstream_endpoints_ = 5; @@ -515,7 +507,7 @@ TEST_P(LoadStatsIntegrationTest, InProgress) { waitForLoadStatsRequest({localityStats("winter", 0, 0, 1)}); waitForUpstreamResponse(0, 503); - cleanupUpstreamConnection(); + cleanupUpstreamAndDownstream(); EXPECT_EQ(1, test_server_->counter("load_reporter.requests")->value()); EXPECT_LE(2, test_server_->counter("load_reporter.responses")->value()); @@ -544,7 +536,7 @@ TEST_P(LoadStatsIntegrationTest, Dropped) { response_->waitForEndStream(); ASSERT_TRUE(response_->complete()); EXPECT_STREQ("503", response_->headers().Status()->value().c_str()); - cleanupUpstreamConnection(); + cleanupUpstreamAndDownstream(); waitForLoadStatsRequest({}, 1); diff --git a/test/integration/ratelimit_integration_test.cc b/test/integration/ratelimit_integration_test.cc index 1e77ff2b74b6b..537e53ef5b6af 100644 --- a/test/integration/ratelimit_integration_test.cc +++ b/test/integration/ratelimit_integration_test.cc @@ -130,15 +130,11 @@ class RatelimitIntegrationTest : public HttpIntegrationTest, } void cleanup() { - codec_client_->close(); if (fake_ratelimit_connection_ != nullptr) { fake_ratelimit_connection_->close(); fake_ratelimit_connection_->waitForDisconnect(); } - if (fake_upstream_connection_ != nullptr) { - fake_upstream_connection_->close(); - fake_upstream_connection_->waitForDisconnect(); - } + cleanupUpstreamAndDownstream(); } FakeHttpConnectionPtr fake_ratelimit_connection_;