From 3b4bed273cd38964d097cb110f894c9963137af2 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Sun, 4 Jun 2017 01:41:15 -0700 Subject: Cancel the dns lookup in dns_ares_shutdown --- .../resolver/dns/c_ares/dns_resolver_ares.c | 14 +++++--- .../resolver/dns/c_ares/grpc_ares_ev_driver.h | 4 +++ .../dns/c_ares/grpc_ares_ev_driver_posix.c | 14 ++++++++ .../resolver/dns/c_ares/grpc_ares_wrapper.c | 37 ++++++++++++---------- .../resolver/dns/c_ares/grpc_ares_wrapper.h | 8 ++++- 5 files changed, 56 insertions(+), 21 deletions(-) (limited to 'src/core/ext/filters/client_channel/resolver/dns') diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.c b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.c index dcd16a3f1c..4080fb1770 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.c +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.c @@ -80,6 +80,8 @@ typedef struct { grpc_combiner *combiner; /** are we currently resolving? */ bool resolving; + /** the pending resolving request */ + grpc_ares_request *pending_request; /** which version of the result have we published? */ int published_version; /** which version of the result is current? */ @@ -124,6 +126,9 @@ static void dns_ares_shutdown_locked(grpc_exec_ctx *exec_ctx, if (r->have_retry_timer) { grpc_timer_cancel(exec_ctx, &r->retry_timer); } + if (r->pending_request != NULL) { + grpc_cancel_ares_request(exec_ctx, r->pending_request); + } if (r->next_completion != NULL) { *r->target_result = NULL; grpc_closure_sched( @@ -160,6 +165,7 @@ static void dns_ares_on_resolved_locked(grpc_exec_ctx *exec_ctx, void *arg, grpc_channel_args *result = NULL; GPR_ASSERT(r->resolving); r->resolving = false; + r->pending_request = NULL; if (r->lb_addresses != NULL) { grpc_arg new_arg = grpc_lb_addresses_create_channel_arg(r->lb_addresses); result = grpc_channel_args_copy_and_add(r->channel_args, &new_arg, 1); @@ -216,10 +222,10 @@ static void dns_ares_start_resolving_locked(grpc_exec_ctx *exec_ctx, GPR_ASSERT(!r->resolving); r->resolving = true; r->lb_addresses = NULL; - grpc_dns_lookup_ares(exec_ctx, r->dns_server, r->name_to_resolve, - r->default_port, r->interested_parties, - &r->dns_ares_on_resolved_locked, &r->lb_addresses, - true /* check_grpclb */); + r->pending_request = grpc_dns_lookup_ares( + exec_ctx, r->dns_server, r->name_to_resolve, r->default_port, + r->interested_parties, &r->dns_ares_on_resolved_locked, &r->lb_addresses, + true /* check_grpclb */); } static void dns_ares_maybe_finish_next_locked(grpc_exec_ctx *exec_ctx, diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver.h b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver.h index aa88940021..b45d7299e2 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver.h +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver.h @@ -62,5 +62,9 @@ grpc_error *grpc_ares_ev_driver_create(grpc_ares_ev_driver **ev_driver, of ARES_ECANCELLED. */ void grpc_ares_ev_driver_destroy(grpc_ares_ev_driver *ev_driver); +/* Shutdown all the grpc_fds used by \a ev_driver */ +void grpc_ares_ev_driver_shutdown(grpc_exec_ctx *exec_ctx, + grpc_ares_ev_driver *ev_driver); + #endif /* GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_RESOLVER_DNS_C_ARES_GRPC_ARES_EV_DRIVER_H \ */ diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver_posix.c b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver_posix.c index 1e4f3eb5ab..91183e30af 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver_posix.c +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver_posix.c @@ -155,6 +155,20 @@ void grpc_ares_ev_driver_destroy(grpc_ares_ev_driver *ev_driver) { grpc_ares_ev_driver_unref(ev_driver); } +void grpc_ares_ev_driver_shutdown(grpc_exec_ctx *exec_ctx, + grpc_ares_ev_driver *ev_driver) { + gpr_mu_lock(&ev_driver->mu); + ev_driver->shutting_down = true; + fd_node *fn = ev_driver->fds; + while (fn != NULL) { + grpc_fd_shutdown( + exec_ctx, fn->grpc_fd, + GRPC_ERROR_CREATE_FROM_STATIC_STRING("grpc_ares_ev_driver_shutdown")); + fn = fn->next; + } + gpr_mu_unlock(&ev_driver->mu); +} + // Search fd in the fd_node list head. This is an O(n) search, the max possible // value of n is ARES_GETSOCK_MAXNUM (16). n is typically 1 - 2 in our tests. static fd_node *pop_fd_node(fd_node **head, int fd) { diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.c b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.c index 8f856885ec..afbccfd840 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.c +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.c @@ -61,7 +61,7 @@ static gpr_once g_basic_init = GPR_ONCE_INIT; static gpr_mu g_init_mu; -typedef struct grpc_ares_request { +struct grpc_ares_request { /** indicates the DNS server to use, if specified */ struct ares_addr_port_node dns_server_addr; /** following members are set in grpc_resolve_address_ares_impl */ @@ -80,7 +80,7 @@ typedef struct grpc_ares_request { bool success; /** the errors explaining the request failure, set in on_done_cb */ grpc_error *error; -} grpc_ares_request; +}; typedef struct grpc_ares_hostbyname_request { /** following members are set in create_hostbyname_request */ @@ -121,12 +121,10 @@ static void grpc_ares_request_unref(grpc_exec_ctx *exec_ctx, 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. */ - gpr_log(GPR_DEBUG, "grpc_ares_request_unref NULl"); grpc_exec_ctx new_exec_ctx = GRPC_EXEC_CTX_INIT; grpc_closure_sched(&new_exec_ctx, r->on_done, r->error); grpc_exec_ctx_finish(&new_exec_ctx); } else { - gpr_log(GPR_DEBUG, "grpc_ares_request_unref exec_ctx"); grpc_closure_sched(exec_ctx, r->on_done, r->error); } gpr_mu_destroy(&r->mu); @@ -177,11 +175,11 @@ static void on_hostbyname_done_cb(void *arg, int status, int timeouts, gpr_realloc((*lb_addresses)->addresses, sizeof(grpc_lb_address) * (*lb_addresses)->num_addresses); for (i = prev_naddr; i < (*lb_addresses)->num_addresses; i++) { - memset(&(*lb_addresses)->addresses[i], 0, sizeof(grpc_lb_address)); switch (hostent->h_addrtype) { case AF_INET6: { size_t addr_len = sizeof(struct sockaddr_in6); struct sockaddr_in6 addr; + memset(&addr, 0, addr_len); memcpy(&addr.sin6_addr, hostent->h_addr_list[i - prev_naddr], sizeof(struct in6_addr)); addr.sin6_family = (sa_family_t)hostent->h_addrtype; @@ -202,6 +200,7 @@ static void on_hostbyname_done_cb(void *arg, int status, int timeouts, case AF_INET: { size_t addr_len = sizeof(struct sockaddr_in); struct sockaddr_in addr; + memset(&addr, 0, addr_len); memcpy(&addr.sin_addr, hostent->h_addr_list[i - prev_naddr], sizeof(struct in_addr)); addr.sin_family = (sa_family_t)hostent->h_addrtype; @@ -282,11 +281,10 @@ static void on_srv_query_done_cb(void *arg, int status, int timeouts, grpc_exec_ctx_finish(&exec_ctx); } -void grpc_dns_lookup_ares_impl(grpc_exec_ctx *exec_ctx, const char *dns_server, - const char *name, const char *default_port, - grpc_pollset_set *interested_parties, - grpc_closure *on_done, grpc_lb_addresses **addrs, - bool check_grpclb) { +grpc_ares_request *grpc_dns_lookup_ares_impl( + grpc_exec_ctx *exec_ctx, const char *dns_server, const char *name, + const char *default_port, grpc_pollset_set *interested_parties, + grpc_closure *on_done, grpc_lb_addresses **addrs, bool check_grpclb) { grpc_error *error = GRPC_ERROR_NONE; /* TODO(zyc): Enable tracing after #9603 is checked in */ /* if (grpc_dns_trace) { @@ -384,19 +382,26 @@ void grpc_dns_lookup_ares_impl(grpc_exec_ctx *exec_ctx, const char *dns_server, grpc_ares_request_unref(exec_ctx, r); gpr_free(host); gpr_free(port); - return; + return r; error_cleanup: grpc_closure_sched(exec_ctx, on_done, error); gpr_free(host); gpr_free(port); + return NULL; } -void (*grpc_dns_lookup_ares)(grpc_exec_ctx *exec_ctx, const char *dns_server, - const char *name, const char *default_port, - grpc_pollset_set *interested_parties, - grpc_closure *on_done, grpc_lb_addresses **addrs, - bool check_grpclb) = grpc_dns_lookup_ares_impl; +grpc_ares_request *(*grpc_dns_lookup_ares)( + grpc_exec_ctx *exec_ctx, const char *dns_server, const char *name, + const char *default_port, grpc_pollset_set *interested_parties, + grpc_closure *on_done, grpc_lb_addresses **addrs, + bool check_grpclb) = grpc_dns_lookup_ares_impl; + +void grpc_cancel_ares_request(grpc_exec_ctx *exec_ctx, grpc_ares_request *r) { + if (grpc_dns_lookup_ares == grpc_dns_lookup_ares_impl) { + grpc_ares_ev_driver_shutdown(exec_ctx, r->ev_driver); + } +} grpc_error *grpc_ares_init(void) { gpr_once_init(&g_basic_init, do_basic_init); diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.h b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.h index 9bcc115beb..4a8374a192 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.h +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.h @@ -40,6 +40,8 @@ #include "src/core/lib/iomgr/polling_entity.h" #include "src/core/lib/iomgr/resolve_address.h" +typedef struct grpc_ares_request grpc_ares_request; + /* Asynchronously resolve addr. Use \a 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 @@ -59,11 +61,15 @@ extern void (*grpc_resolve_address_ares)(grpc_exec_ctx *exec_ctx, function. \a on_done may be called directly in this function without being scheduled with \a exec_ctx, it must not try to acquire locks that are being held by the caller. */ -extern void (*grpc_dns_lookup_ares)( +extern grpc_ares_request *(*grpc_dns_lookup_ares)( grpc_exec_ctx *exec_ctx, const char *dns_server, const char *addr, const char *default_port, grpc_pollset_set *interested_parties, grpc_closure *on_done, grpc_lb_addresses **addresses, bool check_grpclb); +/* Cancel the pending grpc_ares_request \a request */ +void grpc_cancel_ares_request(grpc_exec_ctx *exec_ctx, + grpc_ares_request *request); + /* Initialize gRPC ares wrapper. Must be called at least once before grpc_resolve_address_ares(). */ grpc_error *grpc_ares_init(void); -- cgit v1.2.3