Cancel the dns lookup in dns_ares_shutdown

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,

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 \
           */

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) {

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);

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);

test/core/client_channel/resolvers/dns_resolver_connectivity_test.c

@@ -72,12 +72,10 @@ static void my_resolve_address(grpc_exec_ctx *exec_ctx, const char *addr,
   grpc_closure_sched(exec_ctx, on_done, error);
 }
 
-static void my_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 **lb_addrs,
-                               bool check_grpclb) {
+static grpc_ares_request *my_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 **lb_addrs, bool check_grpclb) {
   gpr_mu_lock(&g_mu);
   GPR_ASSERT(0 == strcmp("test", addr));
   grpc_error *error = GRPC_ERROR_NONE;
@@ -91,6 +89,7 @@ static void my_dns_lookup_ares(grpc_exec_ctx *exec_ctx, const char *dns_server,
     grpc_lb_addresses_set_address(*lb_addrs, 0, NULL, 0, NULL, NULL, NULL);
   }
   grpc_closure_sched(exec_ctx, on_done, error);
+  return NULL;
 }
 
 static grpc_resolver *create_resolver(grpc_exec_ctx *exec_ctx,

test/core/end2end/fuzzers/api_fuzzer.c

@@ -426,11 +426,10 @@ void my_resolve_address(grpc_exec_ctx *exec_ctx, const char *addr,
       gpr_now(GPR_CLOCK_MONOTONIC));
 }
 
-void my_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 **lb_addrs,
-                        bool check_grpclb) {
+grpc_ares_request *my_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 **lb_addrs, bool check_grpclb) {
   addr_req *r = gpr_malloc(sizeof(*r));
   r->addr = gpr_strdup(addr);
   r->on_done = on_done;
@@ -441,6 +440,7 @@ void my_dns_lookup_ares(grpc_exec_ctx *exec_ctx, const char *dns_server,
                                         gpr_time_from_seconds(1, GPR_TIMESPAN)),
       grpc_closure_create(finish_resolve, r, grpc_schedule_on_exec_ctx),
       gpr_now(GPR_CLOCK_MONOTONIC));
+  return NULL;
 }
 
 ////////////////////////////////////////////////////////////////////////////////