diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc index 1a7e5d06268..d41c8238f1c 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc @@ -548,13 +548,13 @@ static grpc_ares_request* grpc_dns_lookup_ares_locked_impl( r, name, default_port); // Early out if the target is an ipv4 or ipv6 literal. if (resolve_as_ip_literal_locked(name, default_port, addrs)) { - GRPC_CLOSURE_SCHED(on_done, GRPC_ERROR_NONE); + grpc_ares_complete_request_locked(r); return r; } // Early out if the target is localhost and we're on Windows. if (grpc_ares_maybe_resolve_localhost_manually_locked(name, default_port, addrs)) { - GRPC_CLOSURE_SCHED(on_done, GRPC_ERROR_NONE); + grpc_ares_complete_request_locked(r); return r; } // Don't query for SRV and TXT records if the target is "localhost", so diff --git a/test/core/iomgr/resolve_address_test.cc b/test/core/iomgr/resolve_address_test.cc index 0ae0ec888b6..b041a15ff34 100644 --- a/test/core/iomgr/resolve_address_test.cc +++ b/test/core/iomgr/resolve_address_test.cc @@ -23,6 +23,8 @@ #include #include +#include + #include #include "src/core/lib/gpr/env.h" @@ -120,6 +122,35 @@ static void must_fail(void* argsp, grpc_error* err) { gpr_mu_unlock(args->mu); } +// This test assumes the environment has an ipv6 loopback +static void must_succeed_with_ipv6_first(void* argsp, grpc_error* err) { + args_struct* args = static_cast(argsp); + GPR_ASSERT(err == GRPC_ERROR_NONE); + GPR_ASSERT(args->addrs != nullptr); + GPR_ASSERT(args->addrs->naddrs > 0); + const struct sockaddr* first_address = + reinterpret_cast(args->addrs->addrs[0].addr); + GPR_ASSERT(first_address->sa_family == AF_INET6); + gpr_atm_rel_store(&args->done_atm, 1); + gpr_mu_lock(args->mu); + GRPC_LOG_IF_ERROR("pollset_kick", grpc_pollset_kick(args->pollset, nullptr)); + gpr_mu_unlock(args->mu); +} + +static void must_succeed_with_ipv4_first(void* argsp, grpc_error* err) { + args_struct* args = static_cast(argsp); + GPR_ASSERT(err == GRPC_ERROR_NONE); + GPR_ASSERT(args->addrs != nullptr); + GPR_ASSERT(args->addrs->naddrs > 0); + const struct sockaddr* first_address = + reinterpret_cast(args->addrs->addrs[0].addr); + GPR_ASSERT(first_address->sa_family == AF_INET); + gpr_atm_rel_store(&args->done_atm, 1); + gpr_mu_lock(args->mu); + GRPC_LOG_IF_ERROR("pollset_kick", grpc_pollset_kick(args->pollset, nullptr)); + gpr_mu_unlock(args->mu); +} + static void test_localhost(void) { grpc_core::ExecCtx exec_ctx; args_struct args; @@ -146,6 +177,33 @@ static void test_default_port(void) { args_finish(&args); } +static void test_localhost_result_has_ipv6_first(void) { + grpc_core::ExecCtx exec_ctx; + args_struct args; + args_init(&args); + grpc_resolve_address("localhost:1", nullptr, args.pollset_set, + GRPC_CLOSURE_CREATE(must_succeed_with_ipv6_first, &args, + grpc_schedule_on_exec_ctx), + &args.addrs); + grpc_core::ExecCtx::Get()->Flush(); + poll_pollset_until_request_done(&args); + args_finish(&args); +} + +static void test_localhost_result_has_ipv4_first_when_ipv6_isnt_available( + void) { + grpc_core::ExecCtx exec_ctx; + args_struct args; + args_init(&args); + grpc_resolve_address("localhost:1", nullptr, args.pollset_set, + GRPC_CLOSURE_CREATE(must_succeed_with_ipv4_first, &args, + grpc_schedule_on_exec_ctx), + &args.addrs); + grpc_core::ExecCtx::Get()->Flush(); + poll_pollset_until_request_done(&args); + args_finish(&args); +} + static void test_non_numeric_default_port(void) { grpc_core::ExecCtx exec_ctx; args_struct args; @@ -245,6 +303,34 @@ static void test_unparseable_hostports(void) { } } +typedef struct mock_ipv6_disabled_source_addr_factory { + address_sorting_source_addr_factory base; +} mock_ipv6_disabled_source_addr_factory; + +static bool mock_ipv6_disabled_source_addr_factory_get_source_addr( + address_sorting_source_addr_factory* factory, + const address_sorting_address* dest_addr, + address_sorting_address* source_addr) { + // Mock lack of IPv6. For IPv4, set the source addr to be the same + // as the destination; tests won't actually connect on the result anyways. + if (address_sorting_abstract_get_family(dest_addr) == + ADDRESS_SORTING_AF_INET6) { + return false; + } + memcpy(source_addr->addr, &dest_addr->addr, dest_addr->len); + source_addr->len = dest_addr->len; + return true; +} + +void mock_ipv6_disabled_source_addr_factory_destroy( + address_sorting_source_addr_factory* factory) {} + +const address_sorting_source_addr_factory_vtable + kMockIpv6DisabledSourceAddrFactoryVtable = { + mock_ipv6_disabled_source_addr_factory_get_source_addr, + mock_ipv6_disabled_source_addr_factory_destroy, +}; + int main(int argc, char** argv) { // First set the resolver type based off of --resolver const char* resolver_type = nullptr; @@ -289,11 +375,26 @@ int main(int argc, char** argv) { // these unit tests under c-ares risks flakiness. test_invalid_ip_addresses(); test_unparseable_hostports(); + } else { + test_localhost_result_has_ipv6_first(); } grpc_core::Executor::ShutdownAll(); } gpr_cmdline_destroy(cl); - grpc_shutdown(); + // The following test uses + // "address_sorting_override_source_addr_factory_for_testing", which works + // on a per-grpc-init basis, and so it's simplest to run this next test + // within a standalone grpc_init/grpc_shutdown pair. + if (gpr_stricmp(resolver_type, "ares") == 0) { + // Run a test case in which c-ares's address sorter + // thinks that IPv4 is available and IPv6 isn't. + grpc_init(); + mock_ipv6_disabled_source_addr_factory factory; + factory.base.vtable = &kMockIpv6DisabledSourceAddrFactoryVtable; + address_sorting_override_source_addr_factory_for_testing(&factory.base); + test_localhost_result_has_ipv4_first_when_ipv6_isnt_available(); + grpc_shutdown(); + } return 0; }