diff --git a/doc/environment_variables.md b/doc/environment_variables.md index d02801bc9bd..11ac890cd5a 100644 --- a/doc/environment_variables.md +++ b/doc/environment_variables.md @@ -65,3 +65,9 @@ some configuration as environment variables that can be set. - DEBUG - log all gRPC messages - INFO - log INFO and ERROR message - ERROR - log only errors + +* GRPC_DNS_RESOLVER + Declares which DNS resolver to use. Available DNS resolver include: + - ares - a DNS resolver based around the c-ares library + - native - a DNS resolver based around getaddrinfo(), creates a new thread to + perform name resolution diff --git a/src/core/ext/resolver/dns/c_ares/dns_resolver_ares.c b/src/core/ext/resolver/dns/c_ares/dns_resolver_ares.c index e1db898e7fd..64d4a5933ee 100644 --- a/src/core/ext/resolver/dns/c_ares/dns_resolver_ares.c +++ b/src/core/ext/resolver/dns/c_ares/dns_resolver_ares.c @@ -75,7 +75,7 @@ typedef struct { grpc_closure dns_ares_on_resolved_locked; /** Combiner guarding the rest of the state */ - grpc_combiner *lock; + grpc_combiner *combiner; /** are we currently resolving? */ bool resolving; /** which version of the result have we published? */ @@ -136,7 +136,7 @@ static void dns_ares_shutdown(grpc_exec_ctx *exec_ctx, grpc_resolver *resolver) { ares_dns_resolver *r = (ares_dns_resolver *)resolver; GRPC_RESOLVER_REF(&r->base, "dns-ares-shutdown"); - grpc_combiner_execute(exec_ctx, r->lock, &r->dns_ares_shutdown_locked, + grpc_combiner_execute(exec_ctx, r->combiner, &r->dns_ares_shutdown_locked, GRPC_ERROR_NONE, false); } @@ -155,7 +155,7 @@ static void dns_ares_channel_saw_error(grpc_exec_ctx *exec_ctx, grpc_resolver *resolver) { ares_dns_resolver *r = (ares_dns_resolver *)resolver; GRPC_RESOLVER_REF(&r->base, "ares-channel-saw-error"); - grpc_combiner_execute(exec_ctx, r->lock, + grpc_combiner_execute(exec_ctx, r->combiner, &r->dns_ares_channel_saw_error_locked, GRPC_ERROR_NONE, false); } @@ -175,7 +175,8 @@ static void dns_ares_on_retry_timer_locked(grpc_exec_ctx *exec_ctx, void *arg, static void dns_ares_on_retry_timer(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error) { ares_dns_resolver *r = arg; - grpc_combiner_execute(exec_ctx, r->lock, &r->dns_ares_on_retry_timer_locked, + grpc_combiner_execute(exec_ctx, r->combiner, + &r->dns_ares_on_retry_timer_locked, GRPC_ERROR_REF(error), false); } @@ -229,7 +230,7 @@ static void dns_ares_on_resolved_locked(grpc_exec_ctx *exec_ctx, void *arg, static void dns_ares_on_resolved(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error) { ares_dns_resolver *r = arg; - grpc_combiner_execute(exec_ctx, r->lock, &r->dns_ares_on_resolved_locked, + grpc_combiner_execute(exec_ctx, r->combiner, &r->dns_ares_on_resolved_locked, GRPC_ERROR_REF(error), false); } @@ -268,7 +269,7 @@ static void dns_ares_next(grpc_exec_ctx *exec_ctx, grpc_resolver *resolver, args->on_complete = on_complete; args->resolver = resolver; GRPC_RESOLVER_REF(resolver, "ares-next"); - grpc_combiner_execute(exec_ctx, r->lock, + grpc_combiner_execute(exec_ctx, r->combiner, grpc_closure_create(dns_ares_next_locked, args), GRPC_ERROR_NONE, false); } @@ -301,7 +302,7 @@ static void dns_ares_destroy(grpc_exec_ctx *exec_ctx, grpc_resolver *gr) { gpr_log(GPR_DEBUG, "dns_ares_destroy"); ares_dns_resolver *r = (ares_dns_resolver *)gr; grpc_ares_ev_driver_destroy(exec_ctx, r->ev_driver); - grpc_combiner_destroy(exec_ctx, r->lock); + grpc_combiner_destroy(exec_ctx, r->combiner); grpc_ares_cleanup(); if (r->resolved_result != NULL) { grpc_channel_args_destroy(r->resolved_result); @@ -345,7 +346,7 @@ static grpc_resolver *dns_ares_create(grpc_resolver_args *args, gpr_free(r); return NULL; } - r->lock = grpc_combiner_create(NULL); + r->combiner = grpc_combiner_create(NULL); r->name_to_resolve = proxy_name == NULL ? gpr_strdup(path) : proxy_name; r->default_port = gpr_strdup(default_port); grpc_arg server_name_arg; diff --git a/src/core/ext/resolver/dns/c_ares/grpc_ares_ev_driver_posix.c b/src/core/ext/resolver/dns/c_ares/grpc_ares_ev_driver_posix.c index 49b7485c947..8d8f5f05805 100644 --- a/src/core/ext/resolver/dns/c_ares/grpc_ares_ev_driver_posix.c +++ b/src/core/ext/resolver/dns/c_ares/grpc_ares_ev_driver_posix.c @@ -292,8 +292,8 @@ static void grpc_ares_notify_on_event_locked(grpc_exec_ctx *exec_ctx, gpr_mu_unlock(&fdn->mu); } } - // Any remaining fds in ev_driver->fds was not returned by ares_getsock() and - // is therefore no longer in use, so they can be shut donw and removed from + // Any remaining fds in ev_driver->fds were not returned by ares_getsock() and + // are therefore no longer in use, so they can be shut down and removed from // the list. while (ev_driver->fds != NULL) { fd_node *cur = ev_driver->fds; diff --git a/src/core/ext/resolver/dns/c_ares/grpc_ares_wrapper.c b/src/core/ext/resolver/dns/c_ares/grpc_ares_wrapper.c index cf43c941a01..9c83a8bf39d 100644 --- a/src/core/ext/resolver/dns/c_ares/grpc_ares_wrapper.c +++ b/src/core/ext/resolver/dns/c_ares/grpc_ares_wrapper.c @@ -74,8 +74,6 @@ typedef struct grpc_ares_request { grpc_resolved_addresses **addrs_out; /** the evernt driver used by this request */ grpc_ares_ev_driver *ev_driver; - /** the closure wraps request_resolving_address */ - grpc_closure request_closure; /** number of ongoing queries */ gpr_refcount pending_queries; @@ -89,19 +87,6 @@ typedef struct grpc_ares_request { static void do_basic_init(void) { gpr_mu_init(&g_init_mu); } -static void ares_request_unref(grpc_ares_request *r) { - if (gpr_unref(&r->pending_queries)) { - grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; - grpc_exec_ctx_sched(&exec_ctx, r->on_done, r->error, NULL); - grpc_exec_ctx_finish(&exec_ctx); - gpr_mu_destroy(&r->mu); - gpr_free(r->host); - gpr_free(r->port); - gpr_free(r->default_port); - gpr_free(r); - } -} - static uint16_t strhtons(const char *port) { if (strcmp(port, "http") == 0) { return htons(80); @@ -180,22 +165,23 @@ static void on_done_cb(void *arg, int status, int timeouts, } } gpr_mu_unlock(&r->mu); - ares_request_unref(r); -} - -static void request_resolving_address(grpc_exec_ctx *exec_ctx, void *arg, - grpc_error *error) { - grpc_ares_request *r = (grpc_ares_request *)arg; - grpc_ares_ev_driver *ev_driver = r->ev_driver; - ares_channel *channel = - (ares_channel *)grpc_ares_ev_driver_get_channel(ev_driver); - gpr_ref_init(&r->pending_queries, 1); - if (grpc_ipv6_loopback_available()) { - gpr_ref(&r->pending_queries); - ares_gethostbyname(*channel, r->host, AF_INET6, on_done_cb, r); + // If there are no pending queries, invoke on_done callback and destroy the + // request + if (gpr_unref(&r->pending_queries)) { + // A new exec_ctx is created here, as the c-ares interface does not provide + // one in this callback. It's safe to schedule on_done with the newly + // created exec_ctx, since the caller has been warned not to acquire locks + // in on_done. ares_dns_resolver is using combiner to protect resources + // needed by on_done. + grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; + grpc_exec_ctx_sched(&exec_ctx, r->on_done, r->error, NULL); + grpc_exec_ctx_finish(&exec_ctx); + gpr_mu_destroy(&r->mu); + gpr_free(r->host); + gpr_free(r->port); + gpr_free(r->default_port); + gpr_free(r); } - ares_gethostbyname(*channel, r->host, AF_INET, on_done_cb, r); - grpc_ares_ev_driver_start(exec_ctx, ev_driver); } void grpc_resolve_address_ares_impl(grpc_exec_ctx *exec_ctx, const char *name, @@ -240,8 +226,15 @@ void grpc_resolve_address_ares_impl(grpc_exec_ctx *exec_ctx, const char *name, r->host = host; r->success = false; r->error = GRPC_ERROR_NONE; - grpc_closure_init(&r->request_closure, request_resolving_address, r); - grpc_exec_ctx_sched(exec_ctx, &r->request_closure, GRPC_ERROR_NONE, NULL); + ares_channel *channel = + (ares_channel *)grpc_ares_ev_driver_get_channel(r->ev_driver); + gpr_ref_init(&r->pending_queries, 1); + if (grpc_ipv6_loopback_available()) { + gpr_ref(&r->pending_queries); + ares_gethostbyname(*channel, r->host, AF_INET6, on_done_cb, r); + } + ares_gethostbyname(*channel, r->host, AF_INET, on_done_cb, r); + grpc_ares_ev_driver_start(exec_ctx, ev_driver); return; error_cleanup: diff --git a/src/core/ext/resolver/dns/c_ares/grpc_ares_wrapper.h b/src/core/ext/resolver/dns/c_ares/grpc_ares_wrapper.h index 465b9af7bd3..ffeff86344b 100644 --- a/src/core/ext/resolver/dns/c_ares/grpc_ares_wrapper.h +++ b/src/core/ext/resolver/dns/c_ares/grpc_ares_wrapper.h @@ -40,9 +40,11 @@ #include "src/core/lib/iomgr/polling_entity.h" #include "src/core/lib/iomgr/resolve_address.h" -/* Asynchronously resolve addr. Use default_port if a port isn't designated - in addr, otherwise use the port in addr. grpc_ares_init() must be called - at least once before this function. */ +/* Asynchronously resolve addr. Use default_port if a port isn't designated in + addr, otherwise use the port in addr. grpc_ares_init() must be called at + least once before this function. \a on_done may be called directly in this + function without being scheduled with \a exec_ctx, it should not try to + acquire locks that are being held by the caller. */ extern void (*grpc_resolve_address_ares)(grpc_exec_ctx *exec_ctx, const char *addr, const char *default_port,