Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
2 changes: 2 additions & 0 deletions envoy/network/dns_resolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ 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); }
virtual void init() {}
Comment thread
yanjunxiang-google marked this conversation as resolved.
Outdated
virtual void cleanup() {}
};

} // namespace Network
Expand Down
7 changes: 5 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,11 @@ 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);
// Perform factory initialization.
Comment thread
yanjunxiang-google marked this conversation as resolved.
Outdated
factory.init();
Comment thread
yanjunxiang-google marked this conversation as resolved.
Outdated
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
11 changes: 7 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,11 @@ ProcessWide::~ProcessWide() {

ASSERT(init_data.count_ > 0);
if (--init_data.count_ == 0) {
ares_library_cleanup();
// Cleanup c-ares library if it is linked in.
Comment thread
yanjunxiang-google marked this conversation as resolved.
Outdated
if (auto* dns_factory = Config::Utility::getAndCheckFactoryByName<Network::DnsResolverFactory>(
std::string(Network::CaresDnsResolver), true)) {
dns_factory->cleanup();
}
}
}

Expand Down
17 changes: 17 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,23 @@ class CaresDnsResolverFactory : public DnsResolverFactory {
}
return std::make_shared<Network::DnsResolverImpl>(cares, dispatcher, resolvers);
}

void init() override {
// Initialize c-ares library in case first time.
if (!ares_library_initialized_) {
ares_library_initialized_ = true;
ares_library_init(ARES_LIB_INIT_ALL);
}
}
void cleanup() override {
// Cleanup c-ares library if initialized.
if (ares_library_initialized_) {
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:
mutable bool ares_library_initialized_{false};
Comment thread
yanjunxiang-google marked this conversation as resolved.
Outdated
};

// Register the CaresDnsResolverFactory
Expand Down