diff options
author | Jason Carey <jason.carey@mongodb.com> | 2020-01-14 19:02:36 +0000 |
---|---|---|
committer | A. Jesse Jiryu Davis <jesse@mongodb.com> | 2020-01-27 15:40:32 -0500 |
commit | be0e0cfab113a53517869c9fe4cd853b55260e77 (patch) | |
tree | e99afa18d5758d58c048aa34fd756d9dda3f808b | |
parent | fec4b2da8237e4b001cd0b9ce34ea28f62066341 (diff) | |
download | mongo-be0e0cfab113a53517869c9fe4cd853b55260e77.tar.gz |
SERVER-45261 create more state in NiTL ctor
Calls to NetworkInterfaceTL::startCommand() before
NetworkInterfaceTL::startup() wil cause crashes (due to an assumption
inside NetworkInterfaceTL::startCommand() that we have a connection
pool.
That's out of contract for a NetworkInterface
To fix that, lets create more state on the NiTL in it's ctor, and only
spin up the background thread in startup().
4 files changed, 39 insertions, 18 deletions
diff --git a/src/mongo/executor/network_interface_integration_fixture.cpp b/src/mongo/executor/network_interface_integration_fixture.cpp index ad2e31fdb03..62972eef6c3 100644 --- a/src/mongo/executor/network_interface_integration_fixture.cpp +++ b/src/mongo/executor/network_interface_integration_fixture.cpp @@ -47,7 +47,7 @@ namespace mongo { namespace executor { -void NetworkInterfaceIntegrationFixture::startNet( +void NetworkInterfaceIntegrationFixture::createNet( std::unique_ptr<NetworkConnectionHook> connectHook) { ConnectionPool::Options options; #ifdef _WIN32 @@ -59,8 +59,13 @@ void NetworkInterfaceIntegrationFixture::startNet( #endif _net = makeNetworkInterface( "NetworkInterfaceIntegrationFixture", std::move(connectHook), nullptr, std::move(options)); +} + +void NetworkInterfaceIntegrationFixture::startNet( + std::unique_ptr<NetworkConnectionHook> connectHook) { - _net->startup(); + createNet(std::move(connectHook)); + net().startup(); } void NetworkInterfaceIntegrationFixture::tearDown() { diff --git a/src/mongo/executor/network_interface_integration_fixture.h b/src/mongo/executor/network_interface_integration_fixture.h index 15c1a9f4361..eb164359d89 100644 --- a/src/mongo/executor/network_interface_integration_fixture.h +++ b/src/mongo/executor/network_interface_integration_fixture.h @@ -63,6 +63,7 @@ using StartCommandCB = std::function<void(const RemoteCommandResponse&)>; class NetworkInterfaceIntegrationFixture : public mongo::unittest::Test { public: + void createNet(std::unique_ptr<NetworkConnectionHook> connectHook = nullptr); void startNet(std::unique_ptr<NetworkConnectionHook> connectHook = nullptr); void tearDown() override; diff --git a/src/mongo/executor/network_interface_integration_test.cpp b/src/mongo/executor/network_interface_integration_test.cpp index 03cf7d2a1e9..8e593d0535b 100644 --- a/src/mongo/executor/network_interface_integration_test.cpp +++ b/src/mongo/executor/network_interface_integration_test.cpp @@ -73,6 +73,18 @@ TEST_F(NetworkInterfaceIntegrationFixture, Ping) { assertCommandOK("admin", BSON("ping" << 1)); } +TEST_F(NetworkInterfaceIntegrationFixture, PingWithoutStartup) { + createNet(); + + RemoteCommandRequest request{ + fixture().getServers()[0], "admin", BSON("ping" << 1), BSONObj(), nullptr, Minutes(5)}; + + auto fut = runCommand(makeCallbackHandle(), request); + ASSERT_FALSE(fut.isReady()); + net().startup(); + ASSERT(fut.get().isOK()); +} + // Hook that intentionally never finishes class HangingHook : public executor::NetworkConnectionHook { Status validateHost(const HostAndPort&, diff --git a/src/mongo/executor/network_interface_tl.cpp b/src/mongo/executor/network_interface_tl.cpp index e932130b005..01eceebbbd3 100644 --- a/src/mongo/executor/network_interface_tl.cpp +++ b/src/mongo/executor/network_interface_tl.cpp @@ -57,7 +57,25 @@ NetworkInterfaceTL::NetworkInterfaceTL(std::string instanceName, _connPoolOpts(std::move(connPoolOpts)), _onConnectHook(std::move(onConnectHook)), _metadataHook(std::move(metadataHook)), - _state(kDefault) {} + _state(kDefault) { + if (_svcCtx) { + _tl = _svcCtx->getTransportLayer(); + } + + // Even if you have a service context, it may not have a transport layer (mostly for unittests). + if (!_tl) { + warning() << "No TransportLayer configured during NetworkInterface startup"; + _ownedTransportLayer = + transport::TransportLayerManager::makeAndStartDefaultEgressTransportLayer(); + _tl = _ownedTransportLayer.get(); + } + + _reactor = _tl->getReactor(transport::TransportLayer::kNewReactor); + auto typeFactory = std::make_unique<connection_pool_tl::TLTypeFactory>( + _reactor, _tl, std::move(_onConnectHook), _connPoolOpts); + _pool = std::make_shared<ConnectionPool>( + std::move(typeFactory), std::string("NetworkInterfaceTL-") + _instanceName, _connPoolOpts); +} std::string NetworkInterfaceTL::getDiagnosticString() { return "DEPRECATED: getDiagnosticString is deprecated in NetworkInterfaceTL"; @@ -84,22 +102,7 @@ std::string NetworkInterfaceTL::getHostName() { void NetworkInterfaceTL::startup() { stdx::lock_guard<Latch> lk(_mutex); - if (_svcCtx) { - _tl = _svcCtx->getTransportLayer(); - } - - if (!_tl) { - warning() << "No TransportLayer configured during NetworkInterface startup"; - _ownedTransportLayer = - transport::TransportLayerManager::makeAndStartDefaultEgressTransportLayer(); - _tl = _ownedTransportLayer.get(); - } - _reactor = _tl->getReactor(transport::TransportLayer::kNewReactor); - auto typeFactory = std::make_unique<connection_pool_tl::TLTypeFactory>( - _reactor, _tl, std::move(_onConnectHook), _connPoolOpts); - _pool = std::make_shared<ConnectionPool>( - std::move(typeFactory), std::string("NetworkInterfaceTL-") + _instanceName, _connPoolOpts); _ioThread = stdx::thread([this] { setThreadName(_instanceName); _run(); |