Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 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
16 changes: 15 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,20 @@ 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() {}
};

} // namespace Network
Expand Down
11 changes: 11 additions & 0 deletions source/common/config/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,17 @@ class Utility {
POOL_HISTOGRAM(scope))};
}



/**
* Get the hash map for this factory.
*/
template <class Factory>
static absl::flat_hash_map<std::string, Factory*>& getFactoryMap() {
Comment thread
yanjunxiang-google marked this conversation as resolved.
Outdated
return Registry::FactoryRegistry<Factory>::factories();
}


/**
* Get a Factory from the registry with a particular name (and templated type) with error checking
* to ensure the name and factory are valid.
Expand Down
6 changes: 4 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,10 @@ 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);
auto& factory =
Config::Utility::getAndCheckFactory<Network::DnsResolverFactory>(typed_dns_resolver_config);
factory.initialize();
return factory;
}

// Create the default DNS resolver factory. apple for MacOS or c-ares for all others.
Expand Down
2 changes: 1 addition & 1 deletion source/exe/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,8 @@ envoy_cc_library(
name = "process_wide_lib",
srcs = ["process_wide.cc"],
hdrs = ["process_wide.h"],
external_deps = ["ares"],
deps = [
"//envoy/network:dns_resolver_interface",
"//source/common/common:assert_lib",
"//source/common/event:libevent_lib",
"//source/common/http/http2:nghttp2_lib",
Expand Down
10 changes: 6 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,10 @@ ProcessWide::~ProcessWide() {

ASSERT(init_data.count_ > 0);
if (--init_data.count_ == 0) {
ares_library_cleanup();
for (const auto& dns_factory :
Config::Utility::getFactoryMap<Network::DnsResolverFactory>()) {
dns_factory.second->terminate();
}
}
}

Expand Down
21 changes: 21 additions & 0 deletions source/extensions/network/dns_resolver/cares/dns_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,27 @@ 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;
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;
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