diff --git a/source/server/connection_handler_impl.cc b/source/server/connection_handler_impl.cc index b30ed366ed0d4..ed342c64379bb 100644 --- a/source/server/connection_handler_impl.cc +++ b/source/server/connection_handler_impl.cc @@ -264,6 +264,11 @@ void ConnectionHandlerImpl::ActiveTcpSocket::newConnection() { socket_->setDetectedTransportProtocol( Extensions::TransportSockets::TransportProtocolNames::get().RawBuffer); } + // TODO(lambdai): add integration test + // TODO: Address issues in wider scope. See https://github.com/envoyproxy/envoy/issues/8925 + // Erase accept filter states because accept filters may not get the opportunity to clean up. + // Particularly the assigned events need to reset before assigning new events in the follow up. + accept_filters_.clear(); // Create a new connection on this listener. listener_.newConnection(std::move(socket_)); } diff --git a/test/mocks/network/mocks.cc b/test/mocks/network/mocks.cc index 7e3799968be62..fec947cc549f9 100644 --- a/test/mocks/network/mocks.cc +++ b/test/mocks/network/mocks.cc @@ -98,7 +98,7 @@ MockDrainDecision::MockDrainDecision() = default; MockDrainDecision::~MockDrainDecision() = default; MockListenerFilter::MockListenerFilter() = default; -MockListenerFilter::~MockListenerFilter() = default; +MockListenerFilter::~MockListenerFilter() { destroy_(); } MockListenerFilterCallbacks::MockListenerFilterCallbacks() { ON_CALL(*this, socket()).WillByDefault(ReturnRef(socket_)); diff --git a/test/mocks/network/mocks.h b/test/mocks/network/mocks.h index ee7b392e40bee..2387e363db9d1 100644 --- a/test/mocks/network/mocks.h +++ b/test/mocks/network/mocks.h @@ -164,6 +164,7 @@ class MockListenerFilter : public ListenerFilter { MockListenerFilter(); ~MockListenerFilter() override; + MOCK_METHOD0(destroy_, void()); MOCK_METHOD1(onAccept, Network::FilterStatus(ListenerFilterCallbacks&)); }; diff --git a/test/server/connection_handler_test.cc b/test/server/connection_handler_test.cc index 6ff0301a26713..4ce3f337b79cb 100644 --- a/test/server/connection_handler_test.cc +++ b/test/server/connection_handler_test.cc @@ -361,7 +361,8 @@ TEST_F(ConnectionHandlerTest, NormalRedirect) { EXPECT_CALL(test_listener2->socket_, localAddress()).WillRepeatedly(ReturnRef(alt_address)); handler_->addListener(*test_listener2); - Network::MockListenerFilter* test_filter = new Network::MockListenerFilter(); + auto* test_filter = new NiceMock(); + EXPECT_CALL(*test_filter, destroy_()); Network::MockConnectionSocket* accepted_socket = new NiceMock(); bool redirected = false; EXPECT_CALL(factory_, createListenerFilterChain(_)) @@ -432,6 +433,7 @@ TEST_F(ConnectionHandlerTest, FallbackToWildcardListener) { handler_->addListener(*test_listener2); Network::MockListenerFilter* test_filter = new Network::MockListenerFilter(); + EXPECT_CALL(*test_filter, destroy_()); Network::MockConnectionSocket* accepted_socket = new NiceMock(); bool redirected = false; EXPECT_CALL(factory_, createListenerFilterChain(_)) @@ -498,6 +500,7 @@ TEST_F(ConnectionHandlerTest, WildcardListenerWithOriginalDst) { cb.socket().restoreLocalAddress(original_dst_address); return Network::FilterStatus::Continue; })); + EXPECT_CALL(*test_filter, destroy_()); EXPECT_CALL(*accepted_socket, restoreLocalAddress(original_dst_address)); EXPECT_CALL(*accepted_socket, localAddressRestored()).WillOnce(Return(true)); EXPECT_CALL(*accepted_socket, localAddress()).WillRepeatedly(ReturnRef(original_dst_address)); @@ -529,6 +532,7 @@ TEST_F(ConnectionHandlerTest, WildcardListenerWithNoOriginalDst) { handler_->addListener(*test_listener1); Network::MockListenerFilter* test_filter = new Network::MockListenerFilter(); + EXPECT_CALL(*test_filter, destroy_()); Network::MockConnectionSocket* accepted_socket = new NiceMock(); EXPECT_CALL(factory_, createListenerFilterChain(_)) .WillRepeatedly(Invoke([&](Network::ListenerFilterManager& manager) -> bool { @@ -586,6 +590,7 @@ TEST_F(ConnectionHandlerTest, TransportProtocolCustom) { handler_->addListener(*test_listener); Network::MockListenerFilter* test_filter = new Network::MockListenerFilter(); + EXPECT_CALL(*test_filter, destroy_()); EXPECT_CALL(factory_, createListenerFilterChain(_)) .WillRepeatedly(Invoke([&](Network::ListenerFilterManager& manager) -> bool { manager.addAcceptFilter(Network::ListenerFilterPtr{test_filter}); @@ -644,6 +649,7 @@ TEST_F(ConnectionHandlerTest, ListenerFilterTimeout) { EXPECT_CALL(*timeout, disableTimer()); timeout->invokeCallback(); + EXPECT_CALL(*test_filter, destroy_()); dispatcher_.clearDeferredDeleteList(); EXPECT_EQ(0UL, downstream_pre_cx_active.value()); EXPECT_EQ(1UL, stats_store_.counter("downstream_pre_cx_timeout").value()); @@ -672,7 +678,7 @@ TEST_F(ConnectionHandlerTest, ContinueOnListenerFilterTimeout) { EXPECT_CALL(test_listener->socket_, localAddress()); handler_->addListener(*test_listener); - Network::MockListenerFilter* test_filter = new Network::MockListenerFilter(); + Network::MockListenerFilter* test_filter = new NiceMock(); EXPECT_CALL(factory_, createListenerFilterChain(_)) .WillRepeatedly(Invoke([&](Network::ListenerFilterManager& manager) -> bool { manager.addAcceptFilter(Network::ListenerFilterPtr{test_filter}); @@ -691,7 +697,8 @@ TEST_F(ConnectionHandlerTest, ContinueOnListenerFilterTimeout) { Stats::Gauge& downstream_pre_cx_active = stats_store_.gauge("downstream_pre_cx_active", Stats::Gauge::ImportMode::Accumulate); EXPECT_EQ(1UL, downstream_pre_cx_active.value()); - + EXPECT_CALL(*test_filter, destroy_()); + // Barrier: test_filter must be destructed before findFilterChain EXPECT_CALL(manager_, findFilterChain(_)).WillOnce(Return(nullptr)); EXPECT_CALL(*timeout, disableTimer()); timeout->invokeCallback(); @@ -708,7 +715,6 @@ TEST_F(ConnectionHandlerTest, ContinueOnListenerFilterTimeout) { // Timeout is disabled once the listener filters complete. TEST_F(ConnectionHandlerTest, ListenerFilterTimeoutResetOnSuccess) { InSequence s; - TestListener* test_listener = addListener(1, true, false, "test_listener"); Network::MockListener* listener = new Network::MockListener(); Network::ListenerCallbacks* listener_callbacks; @@ -740,7 +746,7 @@ TEST_F(ConnectionHandlerTest, ListenerFilterTimeoutResetOnSuccess) { Event::MockTimer* timeout = new Event::MockTimer(&dispatcher_); EXPECT_CALL(*timeout, enableTimer(std::chrono::milliseconds(15000), _)); listener_callbacks->onAccept(Network::ConnectionSocketPtr{accepted_socket}); - + EXPECT_CALL(*test_filter, destroy_()); EXPECT_CALL(manager_, findFilterChain(_)).WillOnce(Return(nullptr)); EXPECT_CALL(*timeout, disableTimer()); listener_filter_cb->continueFilterChain(true); @@ -777,6 +783,7 @@ TEST_F(ConnectionHandlerTest, ListenerFilterDisabledTimeout) { return Network::FilterStatus::StopIteration; })); EXPECT_CALL(dispatcher_, createTimer_(_)).Times(0); + EXPECT_CALL(*test_filter, destroy_()); Network::MockConnectionSocket* accepted_socket = new NiceMock(); listener_callbacks->onAccept(Network::ConnectionSocketPtr{accepted_socket}); @@ -801,7 +808,6 @@ TEST_F(ConnectionHandlerTest, ListenerFilterReportError) { Network::MockListenerFilter* first_filter = new Network::MockListenerFilter(); Network::MockListenerFilter* last_filter = new Network::MockListenerFilter(); - EXPECT_CALL(factory_, createListenerFilterChain(_)) .WillRepeatedly(Invoke([&](Network::ListenerFilterManager& manager) -> bool { manager.addAcceptFilter(Network::ListenerFilterPtr{first_filter}); @@ -816,6 +822,8 @@ TEST_F(ConnectionHandlerTest, ListenerFilterReportError) { })); // The last filter won't be invoked EXPECT_CALL(*last_filter, onAccept(_)).Times(0); + EXPECT_CALL(*first_filter, destroy_()); + EXPECT_CALL(*last_filter, destroy_()); Network::MockConnectionSocket* accepted_socket = new NiceMock(); listener_callbacks->onAccept(Network::ConnectionSocketPtr{accepted_socket});