Fix api_fuzzer failure, add proper cleanup

4 files changed, 115 insertions, 70 deletions

diff --git a/src/core/ext/resolver/dns/c_ares/grpc_ares_ev_driver.h b/src/core/ext/resolver/dns/c_ares/grpc_ares_ev_driver.h
index 29a2b3b54c..f94fa88af1 100644
--- a/src/core/ext/resolver/dns/c_ares/grpc_ares_ev_driver.h
+++ b/src/core/ext/resolver/dns/c_ares/grpc_ares_ev_driver.h
@@ -40,6 +40,9 @@ typedef struct grpc_ares_ev_driver grpc_ares_ev_driver;
 
 void grpc_ares_notify_on_event(grpc_exec_ctx *exec_ctx,
                                grpc_ares_ev_driver *ev_driver);
+void grpc_ares_gethostbyname(grpc_ares_ev_driver *ev_driver, const char *host,
+                             ares_host_callback on_done_cb, void *arg);
 
-grpc_ares_ev_driver *grpc_ares_ev_driver_create(ares_channel *channel,
-                                                grpc_pollset_set *pollset_set);
+grpc_error *grpc_ares_ev_driver_create(grpc_ares_ev_driver **ev_driver,
+                                       grpc_pollset_set *pollset_set);
+void grpc_ares_ev_driver_destroy(grpc_ares_ev_driver *ev_driver);
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 9b896fff0d..5c57f067fb 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
@@ -58,14 +58,35 @@ typedef struct fd_pair {
 
 struct grpc_ares_ev_driver {
   int id;
+  bool closing;
   ares_socket_t socks[ARES_GETSOCK_MAXNUM];
   int bitmask;
   grpc_closure driver_closure;
   grpc_pollset_set *pollset_set;
-  ares_channel *channel;
+  ares_channel channel;
   fd_pair *fds;
 };
 
+grpc_error *grpc_ares_ev_driver_create(grpc_ares_ev_driver **ev_driver,
+                                       grpc_pollset_set *pollset_set) {
+  int status;
+  *ev_driver = gpr_malloc(sizeof(grpc_ares_ev_driver));
+  status = ares_init(&(*ev_driver)->channel);
+  if (status != ARES_SUCCESS) {
+    gpr_free(*ev_driver);
+    return GRPC_ERROR_CREATE("Failed to init ares channel");
+  }
+  (*ev_driver)->pollset_set = pollset_set;
+  (*ev_driver)->fds = NULL;
+  (*ev_driver)->closing = false;
+  return GRPC_ERROR_NONE;
+}
+
+void grpc_ares_ev_driver_destroy(grpc_ares_ev_driver *ev_driver) {
+  // ev_driver->pollset_set = NULL;
+  ev_driver->closing = true;
+}
+
 static fd_pair *get_fd(fd_pair **head, int fd) {
   fd_pair dummy_head;
   fd_pair *node;
@@ -91,7 +112,7 @@ static void driver_cb(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error) {
     gpr_log(GPR_ERROR, "GRPC_ERROR_NONE");
     for (i = 0; i < ARES_GETSOCK_MAXNUM; i++) {
       ares_process_fd(
-          *d->channel,
+          d->channel,
           ARES_GETSOCK_READABLE(d->bitmask, i) ? d->socks[i] : ARES_SOCKET_BAD,
           ARES_GETSOCK_WRITABLE(d->bitmask, i) ? d->socks[i] : ARES_SOCKET_BAD);
     }
@@ -100,47 +121,56 @@ static void driver_cb(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error) {
   grpc_exec_ctx_flush(exec_ctx);
 }
 
+void grpc_ares_gethostbyname(grpc_ares_ev_driver *ev_driver, const char *host,
+                             ares_host_callback on_done_cb, void *arg) {
+  ares_gethostbyname(ev_driver->channel, host, AF_UNSPEC, on_done_cb, arg);
+}
+
 void grpc_ares_notify_on_event(grpc_exec_ctx *exec_ctx,
                                grpc_ares_ev_driver *ev_driver) {
   size_t i;
   fd_pair *new_list = NULL;
-  ev_driver->bitmask =
-      ares_getsock(*ev_driver->channel, ev_driver->socks, ARES_GETSOCK_MAXNUM);
-  grpc_closure_init(&ev_driver->driver_closure, driver_cb, ev_driver);
-  for (i = 0; i < ARES_GETSOCK_MAXNUM; i++) {
-    char *final_name;
-    gpr_asprintf(&final_name, "host1%" PRIuPTR, i);
-
-    if (ARES_GETSOCK_READABLE(ev_driver->bitmask, i) ||
-        ARES_GETSOCK_WRITABLE(ev_driver->bitmask, i)) {
-      gpr_log(GPR_ERROR, "%d", ev_driver->socks[i]);
-      fd_pair *fdp = get_fd(&ev_driver->fds, ev_driver->socks[i]);
-      if (!fdp) {
-        gpr_log(GPR_ERROR, "new fd");
-        fdp = gpr_malloc(sizeof(fd_pair));
-        fdp->grpc_fd = grpc_fd_create(ev_driver->socks[i], final_name);
-        fdp->fd = ev_driver->socks[i];
-        grpc_pollset_set_add_fd(exec_ctx, ev_driver->pollset_set, fdp->grpc_fd);
-        // new_fd_pair->grpc_fd = fd;
-        // new_fd_pair->next = ev_driver->fds;
-      }
-      fdp->next = new_list;
-      new_list = fdp;
-
-      if (ARES_GETSOCK_READABLE(ev_driver->bitmask, i)) {
-        gpr_log(GPR_ERROR, "READABLE");
-
-        grpc_fd_notify_on_read(exec_ctx, fdp->grpc_fd,
-                               &ev_driver->driver_closure);
-      }
-      if (ARES_GETSOCK_WRITABLE(ev_driver->bitmask, i)) {
-        gpr_log(GPR_ERROR, "writable");
-
-        grpc_fd_notify_on_write(exec_ctx, fdp->grpc_fd,
-                                &ev_driver->driver_closure);
+  gpr_log(GPR_ERROR, "\n\n notify_on_event");
+  if (!ev_driver->closing) {
+    ev_driver->bitmask =
+        ares_getsock(ev_driver->channel, ev_driver->socks, ARES_GETSOCK_MAXNUM);
+    grpc_closure_init(&ev_driver->driver_closure, driver_cb, ev_driver);
+    for (i = 0; i < ARES_GETSOCK_MAXNUM; i++) {
+      char *final_name;
+      gpr_asprintf(&final_name, "host1%" PRIuPTR, i);
+
+      if (ARES_GETSOCK_READABLE(ev_driver->bitmask, i) ||
+          ARES_GETSOCK_WRITABLE(ev_driver->bitmask, i)) {
+        gpr_log(GPR_ERROR, "%d", ev_driver->socks[i]);
+        fd_pair *fdp = get_fd(&ev_driver->fds, ev_driver->socks[i]);
+        if (!fdp) {
+          gpr_log(GPR_ERROR, "new fd");
+          fdp = gpr_malloc(sizeof(fd_pair));
+          fdp->grpc_fd = grpc_fd_create(ev_driver->socks[i], final_name);
+          fdp->fd = ev_driver->socks[i];
+          grpc_pollset_set_add_fd(exec_ctx, ev_driver->pollset_set,
+                                  fdp->grpc_fd);
+          // new_fd_pair->grpc_fd = fd;
+          // new_fd_pair->next = ev_driver->fds;
+        }
+        fdp->next = new_list;
+        new_list = fdp;
+
+        if (ARES_GETSOCK_READABLE(ev_driver->bitmask, i)) {
+          gpr_log(GPR_ERROR, "READABLE");
+
+          grpc_fd_notify_on_read(exec_ctx, fdp->grpc_fd,
+                                 &ev_driver->driver_closure);
+        }
+        if (ARES_GETSOCK_WRITABLE(ev_driver->bitmask, i)) {
+          gpr_log(GPR_ERROR, "writable");
+
+          grpc_fd_notify_on_write(exec_ctx, fdp->grpc_fd,
+                                  &ev_driver->driver_closure);
+        }
       }
+      gpr_free(final_name);
     }
-    gpr_free(final_name);
   }
 
   while (ev_driver->fds != NULL) {
@@ -159,17 +189,12 @@ void grpc_ares_notify_on_event(grpc_exec_ctx *exec_ctx,
   }
 
   ev_driver->fds = new_list;
+  if (ev_driver->closing) {
+    ares_destroy(ev_driver->channel);
+    gpr_free(ev_driver);
+  }
 
   gpr_log(GPR_ERROR, "eof notify_on_event");
 }
 
-grpc_ares_ev_driver *grpc_ares_ev_driver_create(ares_channel *channel,
-                                                grpc_pollset_set *pollset_set) {
-  grpc_ares_ev_driver *ev_driver = gpr_malloc(sizeof(grpc_ares_ev_driver));
-  ev_driver->channel = channel;
-  ev_driver->pollset_set = pollset_set;
-  ev_driver->fds = NULL;
-  return ev_driver;
-}
-
 #endif
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 0602c208ac..396a44b58c 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
@@ -77,7 +77,6 @@ struct grpc_ares_request {
   grpc_resolved_addresses **addrs_out;
   grpc_closure request_closure;
   void *arg;
-  ares_channel channel;
   grpc_ares_ev_driver *ev_driver;
 };
 
@@ -85,6 +84,17 @@ static void do_basic_init(void) {
   gpr_mu_init(&g_init_mu);
 }
 
+static void destroy_request(grpc_ares_request *request) {
+  grpc_ares_ev_driver_destroy(request->ev_driver);
+
+  // ares_cancel(request->channel);
+  // ares_destroy(request->channel);
+  gpr_free(request->name);
+  gpr_free(request->host);
+  gpr_free(request->port);
+  gpr_free(request->default_port);
+}
+
 static void on_done_cb(void *arg, int status, int timeouts,
                        struct hostent *hostent) {
   gpr_log(GPR_ERROR, "status: %d", status);
@@ -147,20 +157,24 @@ static void on_done_cb(void *arg, int status, int timeouts,
             GRPC_ERROR_STR_SYSCALL, "getaddrinfo"),
         GRPC_ERROR_STR_TARGET_ADDRESS, r->name);
   }
+
   grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT;
   grpc_exec_ctx_sched(&exec_ctx, r->on_done, err, NULL);
   grpc_exec_ctx_flush(&exec_ctx);
   grpc_exec_ctx_finish(&exec_ctx);
+
+  destroy_request(r);
+  gpr_free(r);
 }
 
-static void resolve_address_impl(grpc_exec_ctx *exec_ctx, void *arg,
-                                 grpc_error *error) {
+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;
   gpr_log(GPR_ERROR, "before ares_gethostbyname %s", r->host);
-  ares_gethostbyname(r->channel, r->host, AF_UNSPEC, on_done_cb, r);
+  grpc_ares_gethostbyname(r->ev_driver, r->host, on_done_cb, r);
   gpr_log(GPR_ERROR, "before ares_getsock");
-  grpc_ares_notify_on_event(exec_ctx, r->ev_driver);
+  grpc_ares_notify_on_event(exec_ctx, ev_driver);
   gpr_log(GPR_ERROR, "eof resolve_address_impl");
 }
 
@@ -200,16 +214,13 @@ static int try_fake_resolve(const char *name, const char *port,
   return 0;
 }
 
-grpc_ares_request *grpc_resolve_address_ares(grpc_exec_ctx *exec_ctx,
-                                             const char *name,
-                                             const char *default_port,
-                                             grpc_pollset_set *pollset_set,
-                                             grpc_closure *on_done,
-                                             grpc_resolved_addresses **addrs) {
+grpc_ares_request *grpc_resolve_address_ares_impl(
+    grpc_exec_ctx *exec_ctx, const char *name, const char *default_port,
+    grpc_pollset_set *pollset_set, grpc_closure *on_done,
+    grpc_resolved_addresses **addrs) {
   char *host;
   char *port;
   grpc_error *err;
-  int status;
 
   if ((err = grpc_customized_resolve_address(name, default_port, addrs)) !=
       GRPC_ERROR_CANCELLED) {
@@ -222,19 +233,20 @@ grpc_ares_request *grpc_resolve_address_ares(grpc_exec_ctx *exec_ctx,
   r->default_port = gpr_strdup(default_port);
   r->on_done = on_done;
   r->addrs_out = addrs;
+  err = grpc_ares_ev_driver_create(&r->ev_driver, pollset_set);
 
-  status = ares_init(&r->channel);
-  if (status != ARES_SUCCESS) {
-    grpc_exec_ctx_sched(exec_ctx, on_done,
-                        GRPC_ERROR_CREATE("Failed to init ares"), NULL);
-    return r;
+  if (err != GRPC_ERROR_NONE) {
+    grpc_exec_ctx_sched(exec_ctx, on_done, err, NULL);
+    return NULL;
   }
 
-  r->ev_driver = grpc_ares_ev_driver_create(&r->channel, pollset_set);
 
   if (name[0] == 'u' && name[1] == 'n' && name[2] == 'i' && name[3] == 'x' &&
       name[4] == ':' && name[5] != 0) {
-    grpc_resolve_unix_domain_address(name + 5, addrs);
+    grpc_exec_ctx_sched(exec_ctx, on_done,
+                        grpc_resolve_unix_domain_address(name + 5, addrs),
+                        NULL);
+    return r;
   }
 
   /* parse name, splitting it into host and port parts */
@@ -255,7 +267,7 @@ grpc_ares_request *grpc_resolve_address_ares(grpc_exec_ctx *exec_ctx,
   } else {
     r->port = gpr_strdup(port);
     r->host = gpr_strdup(host);
-    grpc_closure_init(&r->request_closure, resolve_address_impl, r);
+    grpc_closure_init(&r->request_closure, request_resolving_address, r);
     grpc_exec_ctx_sched(exec_ctx, &r->request_closure, GRPC_ERROR_NONE, NULL);
   }
 
@@ -264,6 +276,11 @@ grpc_ares_request *grpc_resolve_address_ares(grpc_exec_ctx *exec_ctx,
   return r;
 }
 
+grpc_ares_request *(*grpc_resolve_address_ares)(
+    grpc_exec_ctx *exec_ctx, const char *name, const char *default_port,
+    grpc_pollset_set *pollset_set, grpc_closure *on_done,
+    grpc_resolved_addresses **addrs) = grpc_resolve_address_ares_impl;
+
 grpc_error *grpc_ares_init(void) {
   gpr_once_init(&g_basic_init, do_basic_init);
   gpr_mu_lock(&g_init_mu);
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 f4468df5a0..b6e7a169bb 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
@@ -42,7 +42,7 @@
 
 typedef struct grpc_ares_request grpc_ares_request;
 
-grpc_ares_request *grpc_resolve_address_ares(
+extern grpc_ares_request *(*grpc_resolve_address_ares)(
     grpc_exec_ctx *exec_ctx, const char *addr, const char *default_port,
     grpc_pollset_set *pollset_set, grpc_closure *on_done,
     grpc_resolved_addresses **addresses);