diff --git a/source/common/event/dispatcher_impl.cc b/source/common/event/dispatcher_impl.cc index d5fe2b18c83b6..8c9a3367e62d3 100644 --- a/source/common/event/dispatcher_impl.cc +++ b/source/common/event/dispatcher_impl.cc @@ -77,8 +77,8 @@ Network::ConnectionPtr DispatcherImpl::createServerConnection(Network::ConnectionSocketPtr&& socket, Network::TransportSocketPtr&& transport_socket) { ASSERT(isThreadSafe()); - return std::make_unique(*this, std::move(socket), - std::move(transport_socket), true); + return Network::ConnectionImpl::createServerConnection(*this, std::move(socket), + std::move(transport_socket)); } Network::ClientConnectionPtr @@ -87,8 +87,8 @@ DispatcherImpl::createClientConnection(Network::Address::InstanceConstSharedPtr Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options) { ASSERT(isThreadSafe()); - return std::make_unique(*this, address, source_address, - std::move(transport_socket), options); + return Network::ClientConnectionImpl::createClientConnection( + *this, address, source_address, std::move(transport_socket), options); } Network::DnsResolverSharedPtr DispatcherImpl::createDnsResolver( diff --git a/source/common/network/connection_impl.cc b/source/common/network/connection_impl.cc index 355ae09f39a2f..777b54e54ad51 100644 --- a/source/common/network/connection_impl.cc +++ b/source/common/network/connection_impl.cc @@ -44,6 +44,16 @@ void ConnectionImplUtility::updateBufferStats(uint64_t delta, uint64_t new_total std::atomic ConnectionImpl::next_global_id_; +std::unique_ptr +ConnectionImpl::createServerConnection(Event::Dispatcher& dispatcher, ConnectionSocketPtr&& socket, + TransportSocketPtr&& transport_socket) { + // make_shared<> requires a public constructor so we can't use it here. + std::unique_ptr conn(new Network::ConnectionImpl( + dispatcher, std::move(socket), std::move(transport_socket), true)); + conn->transport_socket_->setTransportSocketCallbacks(*conn); + return conn; +} + ConnectionImpl::ConnectionImpl(Event::Dispatcher& dispatcher, ConnectionSocketPtr&& socket, TransportSocketPtr&& transport_socket, bool connected) : transport_socket_(std::move(transport_socket)), filter_manager_(*this, *this), @@ -65,8 +75,6 @@ ConnectionImpl::ConnectionImpl(Event::Dispatcher& dispatcher, ConnectionSocketPt file_event_ = dispatcher_.createFileEvent( fd(), [this](uint32_t events) -> void { onFileEvent(events); }, Event::FileTriggerType::Edge, Event::FileReadyType::Read | Event::FileReadyType::Write); - - transport_socket_->setTransportSocketCallbacks(*this); } ConnectionImpl::~ConnectionImpl() { @@ -593,6 +601,18 @@ void ConnectionImpl::onDelayedCloseTimeout() { closeSocket(ConnectionEvent::LocalClose); } +std::unique_ptr ClientConnectionImpl::createClientConnection( + Event::Dispatcher& dispatcher, const Address::InstanceConstSharedPtr& remote_address, + const Address::InstanceConstSharedPtr& source_address, + Network::TransportSocketPtr&& transport_socket, + const Network::ConnectionSocket::OptionsSharedPtr& options) { + // make_shared<> requires a public constructor so we can't use it here. + std::unique_ptr conn(new Network::ClientConnectionImpl( + dispatcher, remote_address, source_address, std::move(transport_socket), options)); + conn->transport_socket_->setTransportSocketCallbacks(*conn); + return conn; +} + ClientConnectionImpl::ClientConnectionImpl( Event::Dispatcher& dispatcher, const Address::InstanceConstSharedPtr& remote_address, const Network::Address::InstanceConstSharedPtr& source_address, diff --git a/source/common/network/connection_impl.h b/source/common/network/connection_impl.h index 66ca6b7cbd09b..7a06e448a1a0c 100644 --- a/source/common/network/connection_impl.h +++ b/source/common/network/connection_impl.h @@ -51,10 +51,14 @@ class ConnectionImpl : public virtual Connection, public BufferSource, public TransportSocketCallbacks, protected Logger::Loggable { -public: +protected: ConnectionImpl(Event::Dispatcher& dispatcher, ConnectionSocketPtr&& socket, TransportSocketPtr&& transport_socket, bool connected); +public: + static std::unique_ptr + createServerConnection(Event::Dispatcher& dispatcher, ConnectionSocketPtr&& socket, + TransportSocketPtr&& transport_socket); ~ConnectionImpl(); // Network::FilterManager @@ -200,13 +204,21 @@ class ConnectionImpl : public virtual Connection, * libevent implementation of Network::ClientConnection. */ class ClientConnectionImpl : public ConnectionImpl, virtual public ClientConnection { -public: +protected: ClientConnectionImpl(Event::Dispatcher& dispatcher, const Address::InstanceConstSharedPtr& remote_address, const Address::InstanceConstSharedPtr& source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options); +public: + static std::unique_ptr + createClientConnection(Event::Dispatcher& dispatcher, + const Address::InstanceConstSharedPtr& remote_address, + const Address::InstanceConstSharedPtr& source_address, + Network::TransportSocketPtr&& transport_socket, + const Network::ConnectionSocket::OptionsSharedPtr& options); + // Network::ClientConnection void connect() override; }; diff --git a/source/server/config_validation/BUILD b/source/server/config_validation/BUILD index c0ac95f25d60a..825ead8769477 100644 --- a/source/server/config_validation/BUILD +++ b/source/server/config_validation/BUILD @@ -54,7 +54,10 @@ envoy_cc_library( envoy_cc_library( name = "dispatcher_lib", - srcs = ["dispatcher.cc"], + srcs = [ + "connection.cc", + "dispatcher.cc", + ], hdrs = [ "connection.h", "dispatcher.h", diff --git a/source/server/config_validation/connection.cc b/source/server/config_validation/connection.cc new file mode 100644 index 0000000000000..6ceb9e6896ac3 --- /dev/null +++ b/source/server/config_validation/connection.cc @@ -0,0 +1,20 @@ +#include "server/config_validation/connection.h" + +namespace Envoy { +namespace Network { + +std::unique_ptr +ConfigValidateConnection::create(Event::ValidationDispatcher& dispatcher, + const Address::InstanceConstSharedPtr& remote_address, + const Address::InstanceConstSharedPtr& source_address, + Network::TransportSocketPtr&& transport_socket, + const Network::ConnectionSocket::OptionsSharedPtr& options) { + // make_shared<> requires a public constructor so we can't use it here. + std::unique_ptr conn(new ConfigValidateConnection( + dispatcher, remote_address, source_address, std::move(transport_socket), options)); + conn->transport_socket_->setTransportSocketCallbacks(*conn); + return conn; +} + +} // namespace Network +} // namespace Envoy diff --git a/source/server/config_validation/connection.h b/source/server/config_validation/connection.h index 085067c16f62b..f25d7be49bb1a 100644 --- a/source/server/config_validation/connection.h +++ b/source/server/config_validation/connection.h @@ -13,7 +13,7 @@ namespace Network { overridden with no-op implementations. */ class ConfigValidateConnection : public Network::ClientConnectionImpl { -public: +private: ConfigValidateConnection(Event::ValidationDispatcher& dispatcher, Network::Address::InstanceConstSharedPtr remote_address, Network::Address::InstanceConstSharedPtr source_address, @@ -22,6 +22,14 @@ class ConfigValidateConnection : public Network::ClientConnectionImpl { : Network::ClientConnectionImpl(dispatcher, remote_address, source_address, std::move(transport_socket), options) {} +public: + static std::unique_ptr + create(Event::ValidationDispatcher& dispatcher, + const Address::InstanceConstSharedPtr& remote_address, + const Address::InstanceConstSharedPtr& source_address, + Network::TransportSocketPtr&& transport_socket, + const Network::ConnectionSocket::OptionsSharedPtr& options); + // Unit tests may instantiate it without proper event machine and leave opened sockets. // Do some cleanup before invoking base class's destructor. virtual ~ConfigValidateConnection() { close(ConnectionCloseType::NoFlush); } diff --git a/source/server/config_validation/dispatcher.cc b/source/server/config_validation/dispatcher.cc index a224605428ef6..b5fde2b1149e5 100644 --- a/source/server/config_validation/dispatcher.cc +++ b/source/server/config_validation/dispatcher.cc @@ -12,8 +12,8 @@ Network::ClientConnectionPtr ValidationDispatcher::createClientConnection( Network::Address::InstanceConstSharedPtr source_address, Network::TransportSocketPtr&& transport_socket, const Network::ConnectionSocket::OptionsSharedPtr& options) { - return std::make_unique(*this, remote_address, source_address, - std::move(transport_socket), options); + return Network::ConfigValidateConnection::create(*this, remote_address, source_address, + std::move(transport_socket), options); } Network::DnsResolverSharedPtr ValidationDispatcher::createDnsResolver( diff --git a/test/common/grpc/grpc_client_integration_test_harness.h b/test/common/grpc/grpc_client_integration_test_harness.h index d953842c365b6..c858fb8668d72 100644 --- a/test/common/grpc/grpc_client_integration_test_harness.h +++ b/test/common/grpc/grpc_client_integration_test_harness.h @@ -262,7 +262,7 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest { // Create a Grpc::AsyncClientImpl instance backed by enough fake/mock // infrastructure to initiate a loopback TCP connection to fake_upstream_. AsyncClientPtr createAsyncClientImpl() { - client_connection_ = std::make_unique( + client_connection_ = Network::ClientConnectionImpl::createClientConnection( dispatcher_, fake_upstream_->localAddress(), nullptr, std::move(async_client_transport_socket_), nullptr); ON_CALL(*mock_cluster_info_, connectTimeout()) diff --git a/test/common/network/connection_impl_test.cc b/test/common/network/connection_impl_test.cc index c724b2afc2609..68b52606acad9 100644 --- a/test/common/network/connection_impl_test.cc +++ b/test/common/network/connection_impl_test.cc @@ -78,10 +78,11 @@ INSTANTIATE_TEST_CASE_P(IpVersions, ConnectionImplDeathTest, TEST_P(ConnectionImplDeathTest, BadFd) { Event::SimulatedTimeSystem time_system; Event::DispatcherImpl dispatcher(time_system); - EXPECT_DEATH_LOG_TO_STDERR( - ConnectionImpl(dispatcher, std::make_unique(-1, nullptr, nullptr), - Network::Test::createRawBufferSocket(), false), - ".*assert failure: fd\\(\\) != -1.*"); + EXPECT_DEATH_LOG_TO_STDERR(ConnectionImpl::createServerConnection( + dispatcher, + std::make_unique(-1, nullptr, nullptr), + Network::Test::createRawBufferSocket()), + ".*assert failure: fd\\(\\) != -1.*"); } class ConnectionImplTest : public testing::TestWithParam { @@ -955,9 +956,9 @@ TEST_P(ConnectionImplTest, FlushWriteCloseTest) { // triggered. TEST_P(ConnectionImplTest, FlushWriteCloseTimeoutTest) { ConnectionMocks mocks = createConnectionMocks(); - auto server_connection = std::make_unique( + auto server_connection = ConnectionImpl::createServerConnection( *mocks.dispatcher, std::make_unique(0, nullptr, nullptr), - std::move(mocks.transport_socket), true); + std::move(mocks.transport_socket)); InSequence s1; @@ -1080,9 +1081,9 @@ TEST_P(ConnectionImplTest, FlushWriteAndDelayConfigDisabledTest) { std::function above_high) -> Buffer::Instance* { return new Buffer::WatermarkBuffer(below_low, above_high); })); - std::unique_ptr server_connection(new Network::ConnectionImpl( + auto server_connection = ConnectionImpl::createServerConnection( dispatcher, std::make_unique(0, nullptr, nullptr), - std::make_unique>(), true)); + std::make_unique>()); time_system_.setMonotonicTime(std::chrono::milliseconds(0)); @@ -1108,9 +1109,9 @@ TEST_P(ConnectionImplTest, FlushWriteAndDelayConfigDisabledTest) { // Test that tearing down the connection will disable the delayed close timer. TEST_P(ConnectionImplTest, DelayedCloseTimeoutDisableOnSocketClose) { ConnectionMocks mocks = createConnectionMocks(); - auto server_connection = std::make_unique( + auto server_connection = ConnectionImpl::createServerConnection( *mocks.dispatcher, std::make_unique(0, nullptr, nullptr), - std::move(mocks.transport_socket), true); + std::move(mocks.transport_socket)); InSequence s1; @@ -1132,9 +1133,9 @@ TEST_P(ConnectionImplTest, DelayedCloseTimeoutDisableOnSocketClose) { // Test that the delayed close timeout callback is resilient to connection teardown edge cases. TEST_P(ConnectionImplTest, DelayedCloseTimeoutNullStats) { ConnectionMocks mocks = createConnectionMocks(); - auto server_connection = std::make_unique( + auto server_connection = ConnectionImpl::createServerConnection( *mocks.dispatcher, std::make_unique(0, nullptr, nullptr), - std::move(mocks.transport_socket), true); + std::move(mocks.transport_socket)); InSequence s1; @@ -1180,9 +1181,9 @@ class MockTransportConnectionImplTest : public testing::Test { .WillOnce(Invoke([this](TransportSocketCallbacks& callbacks) { transport_socket_callbacks_ = &callbacks; })); - connection_ = std::make_unique( + connection_ = ConnectionImpl::createServerConnection( dispatcher_, std::make_unique(0, nullptr, nullptr), - TransportSocketPtr(transport_socket_), true); + TransportSocketPtr(transport_socket_)); connection_->addConnectionCallbacks(callbacks_); }