From 1f4b2a8e33e045464793665be887b54ec37b7a9d Mon Sep 17 00:00:00 2001
From: Yuchen Zeng <zyc@google.com>
Date: Mon, 5 Jun 2017 15:24:31 -0700
Subject: [PATCH] Fix the fd clean up process
---
.../dns/c_ares/grpc_ares_ev_driver_posix.c | 9 +++--
.../dns_resolver_connectivity_test.c | 2 +-
test/core/end2end/goaway_server_test.c | 39 +++++++++++++++++++
3 files changed, 46 insertions(+), 4 deletions(-)
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 91183e30af..f036630f02 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
@@ -114,9 +114,12 @@ static void fd_node_destroy(grpc_exec_ctx *exec_ctx, fd_node *fdn) {
GPR_ASSERT(!fdn->writable_registered);
gpr_mu_destroy(&fdn->mu);
grpc_pollset_set_del_fd(exec_ctx, fdn->ev_driver->pollset_set, fdn->grpc_fd);
- grpc_fd_shutdown(exec_ctx, fdn->grpc_fd,
- GRPC_ERROR_CREATE_FROM_STATIC_STRING("fd node destroyed"));
- grpc_fd_orphan(exec_ctx, fdn->grpc_fd, NULL, NULL, "c-ares query finished");
+ /* c-ares library has closed the fd inside grpc_fd. This fd may be picked up
+ immediately by another thread, and should not be closed by the following
+ grpc_fd_orphan. To prevent this fd from being closed by grpc_fd_orphan,
+ a fd pointer is provided. */
+ int fd;
+ grpc_fd_orphan(exec_ctx, fdn->grpc_fd, NULL, &fd, "c-ares query finished");
gpr_free(fdn);
}
diff --git a/test/core/client_channel/resolvers/dns_resolver_connectivity_test.c b/test/core/client_channel/resolvers/dns_resolver_connectivity_test.c
index df7a693287..f4ea5aa0c8 100644
--- a/test/core/client_channel/resolvers/dns_resolver_connectivity_test.c
+++ b/test/core/client_channel/resolvers/dns_resolver_connectivity_test.c
@@ -86,7 +86,7 @@ static grpc_ares_request *my_dns_lookup_ares(
} else {
gpr_mu_unlock(&g_mu);
*lb_addrs = grpc_lb_addresses_create(1, NULL);
- grpc_lb_addresses_set_address(*lb_addrs, 0, NULL, 0, NULL, NULL, NULL);
+ grpc_lb_addresses_set_address(*lb_addrs, 0, NULL, 0, false, NULL, NULL);
}
grpc_closure_sched(exec_ctx, on_done, error);
return NULL;
diff --git a/test/core/end2end/goaway_server_test.c b/test/core/end2end/goaway_server_test.c
index ababdb70a8..8ea78542b4 100644
--- a/test/core/end2end/goaway_server_test.c
+++ b/test/core/end2end/goaway_server_test.c
@@ -42,6 +42,8 @@
#include <grpc/support/log.h>
#include <grpc/support/string_util.h>
#include <string.h>
+#include "src/core/ext/filters/client_channel/lb_policy_factory.h"
+#include "src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.h"
#include "src/core/lib/iomgr/resolve_address.h"
#include "src/core/lib/iomgr/sockaddr.h"
#include "test/core/end2end/cq_verifier.h"
@@ -58,6 +60,11 @@ static void (*iomgr_resolve_address)(grpc_exec_ctx *exec_ctx, const char *addr,
grpc_closure *on_done,
grpc_resolved_addresses **addresses);
+static grpc_ares_request *(*iomgr_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);
+
static void set_resolve_port(int port) {
gpr_mu_lock(&g_mu);
g_resolve_port = port;
@@ -95,6 +102,36 @@ static void my_resolve_address(grpc_exec_ctx *exec_ctx, const char *addr,
grpc_closure_sched(exec_ctx, on_done, error);
}
+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) {
+ if (0 != strcmp(addr, "test")) {
+ return iomgr_dns_lookup_ares(exec_ctx, dns_server, addr, default_port,
+ interested_parties, on_done, lb_addrs,
+ check_grpclb);
+ }
+
+ grpc_error *error = GRPC_ERROR_NONE;
+ gpr_mu_lock(&g_mu);
+ if (g_resolve_port < 0) {
+ gpr_mu_unlock(&g_mu);
+ error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("Forced Failure");
+ } else {
+ *lb_addrs = grpc_lb_addresses_create(1, NULL);
+ struct sockaddr_in *sa = gpr_zalloc(sizeof(struct sockaddr_in));
+ sa->sin_family = AF_INET;
+ sa->sin_addr.s_addr = htonl(0x7f000001);
+ sa->sin_port = htons((uint16_t)g_resolve_port);
+ grpc_lb_addresses_set_address(*lb_addrs, 0, sa, sizeof(*sa), false, NULL,
+ NULL);
+ gpr_free(sa);
+ gpr_mu_unlock(&g_mu);
+ }
+ grpc_closure_sched(exec_ctx, on_done, error);
+ return NULL;
+}
+
int main(int argc, char **argv) {
grpc_completion_queue *cq;
cq_verifier *cqv;
@@ -106,7 +143,9 @@ int main(int argc, char **argv) {
gpr_mu_init(&g_mu);
grpc_init();
iomgr_resolve_address = grpc_resolve_address;
+ iomgr_dns_lookup_ares = grpc_dns_lookup_ares;
grpc_resolve_address = my_resolve_address;
+ grpc_dns_lookup_ares = my_dns_lookup_ares;
int was_cancelled1;
int was_cancelled2;
--
GitLab