Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion envoy/network/dns_resolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ constexpr absl::string_view DnsResolverCategory = "envoy.network.dns_resolver";

class DnsResolverFactory : public Config::TypedFactory {
public:
/*
/**
* @returns a DnsResolver object.
* @param dispatcher: the local dispatcher thread
* @param api: API interface to interact with system resources
Expand All @@ -26,6 +26,33 @@ class DnsResolverFactory : public Config::TypedFactory {
const envoy::config::core::v3::TypedExtensionConfig& typed_dns_resolver_config) const PURE;

std::string category() const override { return std::string(DnsResolverCategory); }

/**
* Initialize the related data for this type of DNS resolver.
* For some DNS resolvers, like c-ares, there are some specific data structure
* needs to be initialized before using it to resolve target.
*/
virtual void initialize() {}

/**
* Cleanup the related data for this type of DNS resolver.
* For some DNS resolvers, like c-ares, there are some specific data structure
* needs to be cleaned up before terminates Envoy.
*/
virtual void terminate() {}

/**
* Create the DNS resolver factory based on the typed config and initialize it.
* @returns the DNS Resolver factory.
* @param typed_dns_resolver_config: the typed DNS resolver config
*/
static Network::DnsResolverFactory&
createFactory(const envoy::config::core::v3::TypedExtensionConfig& typed_dns_resolver_config);

/**
* Call the terminate method on all the registered DNS resolver factories.
*/
static void terminateFactories();
};

} // namespace Network
Expand Down
17 changes: 15 additions & 2 deletions source/common/network/dns_resolver/dns_factory_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ void handleLegacyDnsResolverData(
Network::DnsResolverFactory& createDnsResolverFactoryFromTypedConfig(
const envoy::config::core::v3::TypedExtensionConfig& typed_dns_resolver_config) {
ENVOY_LOG_MISC(debug, "create DNS resolver type: {}", typed_dns_resolver_config.name());
return Config::Utility::getAndCheckFactory<Network::DnsResolverFactory>(
typed_dns_resolver_config);
return DnsResolverFactory::createFactory(typed_dns_resolver_config);
}

// Create the default DNS resolver factory. apple for MacOS or c-ares for all others.
Expand All @@ -92,5 +91,19 @@ Network::DnsResolverFactory& createDefaultDnsResolverFactory(
return createDnsResolverFactoryFromTypedConfig(typed_dns_resolver_config);
}

Network::DnsResolverFactory& DnsResolverFactory::createFactory(
const envoy::config::core::v3::TypedExtensionConfig& typed_dns_resolver_config) {
auto& factory =
Config::Utility::getAndCheckFactory<Network::DnsResolverFactory>(typed_dns_resolver_config);
factory.initialize();
return factory;
}

void DnsResolverFactory::terminateFactories() {
auto& factories = Registry::FactoryRegistry<Network::DnsResolverFactory>::factories();
std::for_each(factories.begin(), factories.end(),
[](auto& factory_it) { factory_it.second->terminate(); });
}

} // namespace Network
} // namespace Envoy
2 changes: 1 addition & 1 deletion source/exe/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,11 @@ envoy_cc_library(
name = "process_wide_lib",
srcs = ["process_wide.cc"],
hdrs = ["process_wide.h"],
external_deps = ["ares"],
deps = [
"//source/common/common:assert_lib",
"//source/common/event:libevent_lib",
"//source/common/http/http2:nghttp2_lib",
"//source/common/network/dns_resolver:dns_factory_util_lib",
"//source/server:proto_descriptors_lib",
],
)
Expand Down
7 changes: 3 additions & 4 deletions source/exe/process_wide.cc
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
#include "source/exe/process_wide.h"

#include "envoy/network/dns_resolver.h"

#include "source/common/common/assert.h"
#include "source/common/event/libevent.h"
#include "source/common/http/http2/nghttp2.h"
#include "source/server/proto_descriptors.h"

#include "ares.h"

namespace Envoy {
namespace {

Expand All @@ -30,7 +30,6 @@ ProcessWide::ProcessWide() {
if (init_data.count_++ == 0) {
// TODO(mattklein123): Audit the following as not all of these have to be re-initialized in the
// edge case where something does init/destroy/init/destroy.
ares_library_init(ARES_LIB_INIT_ALL);
Event::Libevent::Global::initialize();
Envoy::Server::validateProtoDescriptors();
Http::Http2::initializeNghttp2Logging();
Expand Down Expand Up @@ -58,7 +57,7 @@ ProcessWide::~ProcessWide() {

ASSERT(init_data.count_ > 0);
if (--init_data.count_ == 0) {
ares_library_cleanup();
Network::DnsResolverFactory::terminateFactories();
}
}

Expand Down
26 changes: 25 additions & 1 deletion source/extensions/network/dns_resolver/cares/dns_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,8 @@ DnsResolverImpl::AddrInfoPendingResolution::availableInterfaces() {
}

// c-ares DNS resolver factory
class CaresDnsResolverFactory : public DnsResolverFactory {
class CaresDnsResolverFactory : public DnsResolverFactory,
public Logger::Loggable<Logger::Id::dns> {
public:
std::string name() const override { return std::string(CaresDnsResolver); }

Expand Down Expand Up @@ -521,6 +522,29 @@ class CaresDnsResolverFactory : public DnsResolverFactory {
}
return std::make_shared<Network::DnsResolverImpl>(cares, dispatcher, resolvers);
}

void initialize() override {
// Initialize c-ares library in case first time.
absl::MutexLock lock(&mutex_);
if (!ares_library_initialized_) {
ares_library_initialized_ = true;
ENVOY_LOG(info, "c-ares library initialized.");
ares_library_init(ARES_LIB_INIT_ALL);
}
}
void terminate() override {
// Cleanup c-ares library if initialized.
absl::MutexLock lock(&mutex_);
if (ares_library_initialized_) {
ares_library_initialized_ = false;
ENVOY_LOG(info, "c-ares library cleaned up.");
ares_library_cleanup();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this cleanup to the ares factory destructor? That would feel a little cleaner.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't do it in the destructor since factories are statically initialized and will race with whatever static clean-up happens in c-ares.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Can you comment that the destructor is not called in deterministic order as the object is statically constructed.

}
}

private:
bool ares_library_initialized_ ABSL_GUARDED_BY(mutex_){false};
absl::Mutex mutex_;
};

// Register the CaresDnsResolverFactory
Expand Down