From 1a8d2b6760ca3767c9cbb14f12cc244fc45bc200 Mon Sep 17 00:00:00 2001 From: apolcyn Date: Mon, 20 Dec 2021 14:01:06 -0800 Subject: [PATCH] API to cancel grpc_resolve_address (#27883) Add an API to cancel grpc_resolve_address --- BUILD | 5 + CMakeLists.txt | 132 ++-- build_autogenerated.yaml | 52 +- gRPC-C++.podspec | 8 + gRPC-Core.podspec | 8 + grpc.gemspec | 4 + package.xml | 4 + .../resolver/dns/c_ares/dns_resolver_ares.cc | 202 ++++-- .../resolver/dns/c_ares/grpc_ares_wrapper.cc | 75 +-- .../resolver/dns/c_ares/grpc_ares_wrapper.h | 11 - .../resolver/dns/native/dns_resolver.cc | 103 +-- .../transport/chttp2/server/chttp2_server.cc | 34 +- src/core/lib/http/httpcli.cc | 35 +- src/core/lib/iomgr/event_engine/iomgr.cc | 5 +- src/core/lib/iomgr/event_engine/resolver.cc | 114 ++-- src/core/lib/iomgr/event_engine/resolver.h | 56 ++ src/core/lib/iomgr/iomgr_custom.cc | 5 +- src/core/lib/iomgr/iomgr_posix.cc | 4 +- src/core/lib/iomgr/iomgr_posix_cfstream.cc | 4 +- src/core/lib/iomgr/iomgr_windows.cc | 4 +- src/core/lib/iomgr/resolve_address.cc | 29 +- src/core/lib/iomgr/resolve_address.h | 82 ++- src/core/lib/iomgr/resolve_address_custom.cc | 240 ++++--- src/core/lib/iomgr/resolve_address_custom.h | 120 +++- src/core/lib/iomgr/resolve_address_impl.h | 59 ++ src/core/lib/iomgr/resolve_address_posix.cc | 148 ++-- src/core/lib/iomgr/resolve_address_posix.h | 47 ++ src/core/lib/iomgr/resolve_address_windows.cc | 167 +++-- src/core/lib/iomgr/resolve_address_windows.h | 47 ++ src/core/lib/iomgr/unix_sockets_posix.cc | 38 +- src/core/lib/iomgr/unix_sockets_posix.h | 8 +- src/core/lib/iomgr/unix_sockets_posix_noop.cc | 16 +- .../dns_resolver_connectivity_test.cc | 75 ++- .../resolvers/dns_resolver_cooldown_test.cc | 88 +-- test/core/end2end/dualstack_socket_test.cc | 28 +- .../end2end/fixtures/http_proxy_fixture.cc | 16 +- test/core/end2end/fuzzers/api_fuzzer.cc | 104 ++- test/core/end2end/goaway_server_test.cc | 101 +-- test/core/iomgr/BUILD | 20 +- test/core/iomgr/resolve_address_posix_test.cc | 30 +- test/core/iomgr/resolve_address_test.cc | 631 +++++++++--------- test/core/surface/server_test.cc | 11 +- .../transport/chttp2/settings_timeout_test.cc | 14 +- test/core/util/resolve_localhost_ip46.cc | 18 +- tools/doxygen/Doxyfile.c++.internal | 4 + tools/doxygen/Doxyfile.core.internal | 4 + tools/run_tests/generated/tests.json | 100 ++- 47 files changed, 1851 insertions(+), 1259 deletions(-) create mode 100644 src/core/lib/iomgr/event_engine/resolver.h create mode 100644 src/core/lib/iomgr/resolve_address_impl.h create mode 100644 src/core/lib/iomgr/resolve_address_posix.h create mode 100644 src/core/lib/iomgr/resolve_address_windows.h diff --git a/BUILD b/BUILD index 164dfcfd468..9e687fb05de 100644 --- a/BUILD +++ b/BUILD @@ -1950,7 +1950,10 @@ grpc_cc_library( "src/core/lib/iomgr/port.h", "src/core/lib/iomgr/python_util.h", "src/core/lib/iomgr/resolve_address.h", + "src/core/lib/iomgr/resolve_address_impl.h", "src/core/lib/iomgr/resolve_address_custom.h", + "src/core/lib/iomgr/resolve_address_posix.h", + "src/core/lib/iomgr/resolve_address_windows.h", "src/core/lib/iomgr/sockaddr.h", "src/core/lib/iomgr/sockaddr_posix.h", "src/core/lib/iomgr/sockaddr_windows.h", @@ -2031,6 +2034,7 @@ grpc_cc_library( "src/core/lib/iomgr/event_engine/pollset.h", "src/core/lib/iomgr/event_engine/promise.h", "src/core/lib/iomgr/event_engine/resolved_address_internal.h", + "src/core/lib/iomgr/event_engine/resolver.h", ], external_deps = [ "absl/container:flat_hash_map", @@ -3142,6 +3146,7 @@ grpc_cc_library( ], external_deps = [ "absl/strings", + "absl/functional:bind_front", ], language = "c++", deps = [ diff --git a/CMakeLists.txt b/CMakeLists.txt index 166638421df..5f275b33c5b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -709,11 +709,9 @@ if(gRPC_BUILD_TESTS) if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) add_dependencies(buildtests_c resolve_address_using_ares_resolver_posix_test) endif() - add_dependencies(buildtests_c resolve_address_using_ares_resolver_test) if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) add_dependencies(buildtests_c resolve_address_using_native_resolver_posix_test) endif() - add_dependencies(buildtests_c resolve_address_using_native_resolver_test) add_dependencies(buildtests_c secure_channel_create_test) add_dependencies(buildtests_c secure_endpoint_test) add_dependencies(buildtests_c security_connector_test) @@ -932,6 +930,8 @@ if(gRPC_BUILD_TESTS) if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) add_dependencies(buildtests_cxx remove_stream_from_stalled_lists_test) endif() + add_dependencies(buildtests_cxx resolve_address_using_ares_resolver_test) + add_dependencies(buildtests_cxx resolve_address_using_native_resolver_test) add_dependencies(buildtests_cxx resource_quota_test) add_dependencies(buildtests_cxx retry_throttle_test) add_dependencies(buildtests_cxx rls_end2end_test) @@ -6484,33 +6484,6 @@ if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) endif() -endif() -if(gRPC_BUILD_TESTS) - -add_executable(resolve_address_using_ares_resolver_test - test/core/iomgr/resolve_address_test.cc -) - -target_include_directories(resolve_address_using_ares_resolver_test - PRIVATE - ${CMAKE_CURRENT_SOURCE_DIR} - ${CMAKE_CURRENT_SOURCE_DIR}/include - ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR} - ${_gRPC_RE2_INCLUDE_DIR} - ${_gRPC_SSL_INCLUDE_DIR} - ${_gRPC_UPB_GENERATED_DIR} - ${_gRPC_UPB_GRPC_GENERATED_DIR} - ${_gRPC_UPB_INCLUDE_DIR} - ${_gRPC_XXHASH_INCLUDE_DIR} - ${_gRPC_ZLIB_INCLUDE_DIR} -) - -target_link_libraries(resolve_address_using_ares_resolver_test - ${_gRPC_ALLTARGETS_LIBRARIES} - grpc_test_util -) - - endif() if(gRPC_BUILD_TESTS) if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) @@ -6543,33 +6516,6 @@ endif() endif() if(gRPC_BUILD_TESTS) -add_executable(resolve_address_using_native_resolver_test - test/core/iomgr/resolve_address_test.cc -) - -target_include_directories(resolve_address_using_native_resolver_test - PRIVATE - ${CMAKE_CURRENT_SOURCE_DIR} - ${CMAKE_CURRENT_SOURCE_DIR}/include - ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR} - ${_gRPC_RE2_INCLUDE_DIR} - ${_gRPC_SSL_INCLUDE_DIR} - ${_gRPC_UPB_GENERATED_DIR} - ${_gRPC_UPB_GRPC_GENERATED_DIR} - ${_gRPC_UPB_INCLUDE_DIR} - ${_gRPC_XXHASH_INCLUDE_DIR} - ${_gRPC_ZLIB_INCLUDE_DIR} -) - -target_link_libraries(resolve_address_using_native_resolver_test - ${_gRPC_ALLTARGETS_LIBRARIES} - grpc_test_util -) - - -endif() -if(gRPC_BUILD_TESTS) - add_executable(secure_channel_create_test test/core/surface/secure_channel_create_test.cc ) @@ -14195,6 +14141,80 @@ endif() endif() if(gRPC_BUILD_TESTS) +add_executable(resolve_address_using_ares_resolver_test + test/core/iomgr/resolve_address_test.cc + test/core/util/fake_udp_and_tcp_server.cc + third_party/googletest/googletest/src/gtest-all.cc + third_party/googletest/googlemock/src/gmock-all.cc +) + +target_include_directories(resolve_address_using_ares_resolver_test + PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR} + ${CMAKE_CURRENT_SOURCE_DIR}/include + ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR} + ${_gRPC_RE2_INCLUDE_DIR} + ${_gRPC_SSL_INCLUDE_DIR} + ${_gRPC_UPB_GENERATED_DIR} + ${_gRPC_UPB_GRPC_GENERATED_DIR} + ${_gRPC_UPB_INCLUDE_DIR} + ${_gRPC_XXHASH_INCLUDE_DIR} + ${_gRPC_ZLIB_INCLUDE_DIR} + third_party/googletest/googletest/include + third_party/googletest/googletest + third_party/googletest/googlemock/include + third_party/googletest/googlemock + ${_gRPC_PROTO_GENS_DIR} +) + +target_link_libraries(resolve_address_using_ares_resolver_test + ${_gRPC_PROTOBUF_LIBRARIES} + ${_gRPC_ALLTARGETS_LIBRARIES} + grpc_test_util + grpc++_test_config +) + + +endif() +if(gRPC_BUILD_TESTS) + +add_executable(resolve_address_using_native_resolver_test + test/core/iomgr/resolve_address_test.cc + test/core/util/fake_udp_and_tcp_server.cc + third_party/googletest/googletest/src/gtest-all.cc + third_party/googletest/googlemock/src/gmock-all.cc +) + +target_include_directories(resolve_address_using_native_resolver_test + PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR} + ${CMAKE_CURRENT_SOURCE_DIR}/include + ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR} + ${_gRPC_RE2_INCLUDE_DIR} + ${_gRPC_SSL_INCLUDE_DIR} + ${_gRPC_UPB_GENERATED_DIR} + ${_gRPC_UPB_GRPC_GENERATED_DIR} + ${_gRPC_UPB_INCLUDE_DIR} + ${_gRPC_XXHASH_INCLUDE_DIR} + ${_gRPC_ZLIB_INCLUDE_DIR} + third_party/googletest/googletest/include + third_party/googletest/googletest + third_party/googletest/googlemock/include + third_party/googletest/googlemock + ${_gRPC_PROTO_GENS_DIR} +) + +target_link_libraries(resolve_address_using_native_resolver_test + ${_gRPC_PROTOBUF_LIBRARIES} + ${_gRPC_ALLTARGETS_LIBRARIES} + grpc_test_util + grpc++_test_config +) + + +endif() +if(gRPC_BUILD_TESTS) + add_executable(resource_quota_test src/core/lib/debug/trace.cc src/core/lib/event_engine/memory_allocator.cc diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index ee35213f99a..6e660f87924 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -791,6 +791,7 @@ libs: - src/core/lib/iomgr/event_engine/pollset.h - src/core/lib/iomgr/event_engine/promise.h - src/core/lib/iomgr/event_engine/resolved_address_internal.h + - src/core/lib/iomgr/event_engine/resolver.h - src/core/lib/iomgr/exec_ctx.h - src/core/lib/iomgr/executor.h - src/core/lib/iomgr/executor/mpmcqueue.h @@ -817,6 +818,9 @@ libs: - src/core/lib/iomgr/python_util.h - src/core/lib/iomgr/resolve_address.h - src/core/lib/iomgr/resolve_address_custom.h + - src/core/lib/iomgr/resolve_address_impl.h + - src/core/lib/iomgr/resolve_address_posix.h + - src/core/lib/iomgr/resolve_address_windows.h - src/core/lib/iomgr/sockaddr.h - src/core/lib/iomgr/sockaddr_posix.h - src/core/lib/iomgr/sockaddr_windows.h @@ -1888,6 +1892,7 @@ libs: - src/core/lib/iomgr/event_engine/pollset.h - src/core/lib/iomgr/event_engine/promise.h - src/core/lib/iomgr/event_engine/resolved_address_internal.h + - src/core/lib/iomgr/event_engine/resolver.h - src/core/lib/iomgr/exec_ctx.h - src/core/lib/iomgr/executor.h - src/core/lib/iomgr/executor/mpmcqueue.h @@ -1914,6 +1919,9 @@ libs: - src/core/lib/iomgr/python_util.h - src/core/lib/iomgr/resolve_address.h - src/core/lib/iomgr/resolve_address_custom.h + - src/core/lib/iomgr/resolve_address_impl.h + - src/core/lib/iomgr/resolve_address_posix.h + - src/core/lib/iomgr/resolve_address_windows.h - src/core/lib/iomgr/sockaddr.h - src/core/lib/iomgr/sockaddr_posix.h - src/core/lib/iomgr/sockaddr_windows.h @@ -3942,16 +3950,6 @@ targets: - linux - posix - mac -- name: resolve_address_using_ares_resolver_test - build: test - language: c - headers: [] - src: - - test/core/iomgr/resolve_address_test.cc - deps: - - grpc_test_util - args: - - --resolver=ares - name: resolve_address_using_native_resolver_posix_test build: test language: c @@ -3966,16 +3964,6 @@ targets: - linux - posix - mac -- name: resolve_address_using_native_resolver_test - build: test - language: c - headers: [] - src: - - test/core/iomgr/resolve_address_test.cc - deps: - - grpc_test_util - args: - - --resolver=native - name: secure_channel_create_test build: test language: c @@ -7375,6 +7363,30 @@ targets: - linux - posix - mac +- name: resolve_address_using_ares_resolver_test + gtest: true + build: test + language: c++ + headers: + - test/core/util/fake_udp_and_tcp_server.h + src: + - test/core/iomgr/resolve_address_test.cc + - test/core/util/fake_udp_and_tcp_server.cc + deps: + - grpc_test_util + - grpc++_test_config +- name: resolve_address_using_native_resolver_test + gtest: true + build: test + language: c++ + headers: + - test/core/util/fake_udp_and_tcp_server.h + src: + - test/core/iomgr/resolve_address_test.cc + - test/core/util/fake_udp_and_tcp_server.cc + deps: + - grpc_test_util + - grpc++_test_config - name: resource_quota_test gtest: true build: test diff --git a/gRPC-C++.podspec b/gRPC-C++.podspec index ec59810eb5e..c2d92b5c9bc 100644 --- a/gRPC-C++.podspec +++ b/gRPC-C++.podspec @@ -657,6 +657,7 @@ Pod::Spec.new do |s| 'src/core/lib/iomgr/event_engine/pollset.h', 'src/core/lib/iomgr/event_engine/promise.h', 'src/core/lib/iomgr/event_engine/resolved_address_internal.h', + 'src/core/lib/iomgr/event_engine/resolver.h', 'src/core/lib/iomgr/exec_ctx.h', 'src/core/lib/iomgr/executor.h', 'src/core/lib/iomgr/executor/mpmcqueue.h', @@ -683,6 +684,9 @@ Pod::Spec.new do |s| 'src/core/lib/iomgr/python_util.h', 'src/core/lib/iomgr/resolve_address.h', 'src/core/lib/iomgr/resolve_address_custom.h', + 'src/core/lib/iomgr/resolve_address_impl.h', + 'src/core/lib/iomgr/resolve_address_posix.h', + 'src/core/lib/iomgr/resolve_address_windows.h', 'src/core/lib/iomgr/sockaddr.h', 'src/core/lib/iomgr/sockaddr_posix.h', 'src/core/lib/iomgr/sockaddr_windows.h', @@ -1376,6 +1380,7 @@ Pod::Spec.new do |s| 'src/core/lib/iomgr/event_engine/pollset.h', 'src/core/lib/iomgr/event_engine/promise.h', 'src/core/lib/iomgr/event_engine/resolved_address_internal.h', + 'src/core/lib/iomgr/event_engine/resolver.h', 'src/core/lib/iomgr/exec_ctx.h', 'src/core/lib/iomgr/executor.h', 'src/core/lib/iomgr/executor/mpmcqueue.h', @@ -1402,6 +1407,9 @@ Pod::Spec.new do |s| 'src/core/lib/iomgr/python_util.h', 'src/core/lib/iomgr/resolve_address.h', 'src/core/lib/iomgr/resolve_address_custom.h', + 'src/core/lib/iomgr/resolve_address_impl.h', + 'src/core/lib/iomgr/resolve_address_posix.h', + 'src/core/lib/iomgr/resolve_address_windows.h', 'src/core/lib/iomgr/sockaddr.h', 'src/core/lib/iomgr/sockaddr_posix.h', 'src/core/lib/iomgr/sockaddr_windows.h', diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index ba35ccdd857..fe7db29be46 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -1027,6 +1027,7 @@ Pod::Spec.new do |s| 'src/core/lib/iomgr/event_engine/resolved_address_internal.cc', 'src/core/lib/iomgr/event_engine/resolved_address_internal.h', 'src/core/lib/iomgr/event_engine/resolver.cc', + 'src/core/lib/iomgr/event_engine/resolver.h', 'src/core/lib/iomgr/event_engine/tcp.cc', 'src/core/lib/iomgr/event_engine/timer.cc', 'src/core/lib/iomgr/exec_ctx.cc', @@ -1086,8 +1087,11 @@ Pod::Spec.new do |s| 'src/core/lib/iomgr/resolve_address.h', 'src/core/lib/iomgr/resolve_address_custom.cc', 'src/core/lib/iomgr/resolve_address_custom.h', + 'src/core/lib/iomgr/resolve_address_impl.h', 'src/core/lib/iomgr/resolve_address_posix.cc', + 'src/core/lib/iomgr/resolve_address_posix.h', 'src/core/lib/iomgr/resolve_address_windows.cc', + 'src/core/lib/iomgr/resolve_address_windows.h', 'src/core/lib/iomgr/sockaddr.h', 'src/core/lib/iomgr/sockaddr_posix.h', 'src/core/lib/iomgr/sockaddr_windows.h', @@ -1909,6 +1913,7 @@ Pod::Spec.new do |s| 'src/core/lib/iomgr/event_engine/pollset.h', 'src/core/lib/iomgr/event_engine/promise.h', 'src/core/lib/iomgr/event_engine/resolved_address_internal.h', + 'src/core/lib/iomgr/event_engine/resolver.h', 'src/core/lib/iomgr/exec_ctx.h', 'src/core/lib/iomgr/executor.h', 'src/core/lib/iomgr/executor/mpmcqueue.h', @@ -1935,6 +1940,9 @@ Pod::Spec.new do |s| 'src/core/lib/iomgr/python_util.h', 'src/core/lib/iomgr/resolve_address.h', 'src/core/lib/iomgr/resolve_address_custom.h', + 'src/core/lib/iomgr/resolve_address_impl.h', + 'src/core/lib/iomgr/resolve_address_posix.h', + 'src/core/lib/iomgr/resolve_address_windows.h', 'src/core/lib/iomgr/sockaddr.h', 'src/core/lib/iomgr/sockaddr_posix.h', 'src/core/lib/iomgr/sockaddr_windows.h', diff --git a/grpc.gemspec b/grpc.gemspec index 381bcfe20b5..f6cc5adfde0 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -946,6 +946,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/iomgr/event_engine/resolved_address_internal.cc ) s.files += %w( src/core/lib/iomgr/event_engine/resolved_address_internal.h ) s.files += %w( src/core/lib/iomgr/event_engine/resolver.cc ) + s.files += %w( src/core/lib/iomgr/event_engine/resolver.h ) s.files += %w( src/core/lib/iomgr/event_engine/tcp.cc ) s.files += %w( src/core/lib/iomgr/event_engine/timer.cc ) s.files += %w( src/core/lib/iomgr/exec_ctx.cc ) @@ -1005,8 +1006,11 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/iomgr/resolve_address.h ) s.files += %w( src/core/lib/iomgr/resolve_address_custom.cc ) s.files += %w( src/core/lib/iomgr/resolve_address_custom.h ) + s.files += %w( src/core/lib/iomgr/resolve_address_impl.h ) s.files += %w( src/core/lib/iomgr/resolve_address_posix.cc ) + s.files += %w( src/core/lib/iomgr/resolve_address_posix.h ) s.files += %w( src/core/lib/iomgr/resolve_address_windows.cc ) + s.files += %w( src/core/lib/iomgr/resolve_address_windows.h ) s.files += %w( src/core/lib/iomgr/sockaddr.h ) s.files += %w( src/core/lib/iomgr/sockaddr_posix.h ) s.files += %w( src/core/lib/iomgr/sockaddr_windows.h ) diff --git a/package.xml b/package.xml index 9db3cae6638..d75e1fbf8f8 100644 --- a/package.xml +++ b/package.xml @@ -926,6 +926,7 @@ + @@ -985,8 +986,11 @@ + + + diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc index 0c114678a5e..3881917d048 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc @@ -59,9 +59,9 @@ namespace grpc_core { namespace { -class AresDnsResolver : public Resolver { +class AresClientChannelDNSResolver : public Resolver { public: - explicit AresDnsResolver(ResolverArgs args); + explicit AresClientChannelDNSResolver(ResolverArgs args); void StartLocked() override; @@ -72,7 +72,7 @@ class AresDnsResolver : public Resolver { void ShutdownLocked() override; private: - ~AresDnsResolver() override; + ~AresClientChannelDNSResolver() override; void MaybeStartResolvingLocked(); void StartResolvingLocked(); @@ -126,7 +126,7 @@ class AresDnsResolver : public Resolver { bool shutdown_initiated_ = false; }; -AresDnsResolver::AresDnsResolver(ResolverArgs args) +AresClientChannelDNSResolver::AresClientChannelDNSResolver(ResolverArgs args) : dns_server_(args.uri.authority()), name_to_resolve_(absl::StripPrefix(args.uri.path(), "/")), channel_args_(grpc_channel_args_copy(args.args)), @@ -156,31 +156,33 @@ AresDnsResolver::AresDnsResolver(ResolverArgs args) GRPC_CLOSURE_INIT(&on_resolved_, OnResolved, this, grpc_schedule_on_exec_ctx); } -AresDnsResolver::~AresDnsResolver() { - GRPC_CARES_TRACE_LOG("resolver:%p destroying AresDnsResolver", this); +AresClientChannelDNSResolver::~AresClientChannelDNSResolver() { + GRPC_CARES_TRACE_LOG("resolver:%p destroying AresClientChannelDNSResolver", + this); grpc_channel_args_destroy(channel_args_); } -void AresDnsResolver::StartLocked() { - GRPC_CARES_TRACE_LOG("resolver:%p AresDnsResolver::StartLocked() is called.", - this); +void AresClientChannelDNSResolver::StartLocked() { + GRPC_CARES_TRACE_LOG( + "resolver:%p AresClientChannelDNSResolver::StartLocked() is called.", + this); MaybeStartResolvingLocked(); } -void AresDnsResolver::RequestReresolutionLocked() { +void AresClientChannelDNSResolver::RequestReresolutionLocked() { if (!resolving_) { MaybeStartResolvingLocked(); } } -void AresDnsResolver::ResetBackoffLocked() { +void AresClientChannelDNSResolver::ResetBackoffLocked() { if (have_next_resolution_timer_) { grpc_timer_cancel(&next_resolution_timer_); } backoff_.Reset(); } -void AresDnsResolver::ShutdownLocked() { +void AresClientChannelDNSResolver::ShutdownLocked() { shutdown_initiated_ = true; if (have_next_resolution_timer_) { grpc_timer_cancel(&next_resolution_timer_); @@ -190,14 +192,17 @@ void AresDnsResolver::ShutdownLocked() { } } -void AresDnsResolver::OnNextResolution(void* arg, grpc_error_handle error) { - AresDnsResolver* r = static_cast(arg); +void AresClientChannelDNSResolver::OnNextResolution(void* arg, + grpc_error_handle error) { + AresClientChannelDNSResolver* r = + static_cast(arg); (void)GRPC_ERROR_REF(error); // ref owned by lambda r->work_serializer_->Run([r, error]() { r->OnNextResolutionLocked(error); }, DEBUG_LOCATION); } -void AresDnsResolver::OnNextResolutionLocked(grpc_error_handle error) { +void AresClientChannelDNSResolver::OnNextResolutionLocked( + grpc_error_handle error) { GRPC_CARES_TRACE_LOG( "resolver:%p re-resolution timer fired. error: %s. shutdown_initiated_: " "%d", @@ -302,14 +307,16 @@ std::string ChooseServiceConfig(char* service_config_choice_json, return service_config->Dump(); } -void AresDnsResolver::OnResolved(void* arg, grpc_error_handle error) { - AresDnsResolver* r = static_cast(arg); +void AresClientChannelDNSResolver::OnResolved(void* arg, + grpc_error_handle error) { + AresClientChannelDNSResolver* r = + static_cast(arg); (void)GRPC_ERROR_REF(error); // ref owned by lambda r->work_serializer_->Run([r, error]() { r->OnResolvedLocked(error); }, DEBUG_LOCATION); } -void AresDnsResolver::OnResolvedLocked(grpc_error_handle error) { +void AresClientChannelDNSResolver::OnResolvedLocked(grpc_error_handle error) { GPR_ASSERT(resolving_); resolving_ = false; delete pending_request_; @@ -402,7 +409,7 @@ void AresDnsResolver::OnResolvedLocked(grpc_error_handle error) { GRPC_ERROR_UNREF(error); } -void AresDnsResolver::MaybeStartResolvingLocked() { +void AresClientChannelDNSResolver::MaybeStartResolvingLocked() { // If there is an existing timer, the time it fires is the earliest time we // can start the next resolution. if (have_next_resolution_timer_) return; @@ -436,7 +443,7 @@ void AresDnsResolver::MaybeStartResolvingLocked() { StartResolvingLocked(); } -void AresDnsResolver::StartResolvingLocked() { +void AresClientChannelDNSResolver::StartResolvingLocked() { // TODO(roth): We currently deal with this ref manually. Once the // new closure API is done, find a way to track this ref with the timer // callback as part of the type system. @@ -458,8 +465,7 @@ void AresDnsResolver::StartResolvingLocked() { // // Factory // - -class AresDnsResolverFactory : public ResolverFactory { +class AresClientChannelDNSResolverFactory : public ResolverFactory { public: bool IsValidUri(const URI& uri) const override { if (absl::StripPrefix(uri.path(), "/").empty()) { @@ -470,30 +476,133 @@ class AresDnsResolverFactory : public ResolverFactory { } OrphanablePtr CreateResolver(ResolverArgs args) const override { - return MakeOrphanable(std::move(args)); + return MakeOrphanable(std::move(args)); } const char* scheme() const override { return "dns"; } }; -} // namespace +class AresDNSResolver : public DNSResolver { + public: + class AresRequest : public DNSResolver::Request { + public: + AresRequest( + absl::string_view name, absl::string_view default_port, + grpc_pollset_set* interested_parties, + std::function>)> + on_resolve_address_done) + : name_(std::string(name)), + default_port_(std::string(default_port)), + interested_parties_(interested_parties), + on_resolve_address_done_(std::move(on_resolve_address_done)) { + GRPC_CARES_TRACE_LOG("AresRequest:%p ctor", this); + GRPC_CLOSURE_INIT(&on_dns_lookup_done_, OnDnsLookupDone, this, + grpc_schedule_on_exec_ctx); + } -} // namespace grpc_core + ~AresRequest() override { + GRPC_CARES_TRACE_LOG("AresRequest:%p dtor ares_request_:%p", this, + ares_request_.get()); + } -extern grpc_address_resolver_vtable* grpc_resolve_address_impl; -static grpc_address_resolver_vtable* default_resolver; + void Start() override { + absl::MutexLock lock(&mu_); + Ref().release(); // ref held by resolution + ares_request_ = std::unique_ptr(grpc_dns_lookup_ares( + "" /* dns_server */, name_.c_str(), default_port_.c_str(), + interested_parties_, &on_dns_lookup_done_, &addresses_, + nullptr /* balancer_addresses */, nullptr /* service_config_json */, + GRPC_DNS_ARES_DEFAULT_QUERY_TIMEOUT_MS)); + GRPC_CARES_TRACE_LOG("AresRequest:%p Start ares_request_:%p", this, + ares_request_.get()); + } -static grpc_error_handle blocking_resolve_address_ares( - const char* name, const char* default_port, - grpc_resolved_addresses** addresses) { - return default_resolver->blocking_resolve_address(name, default_port, - addresses); -} + void Orphan() override { + { + absl::MutexLock lock(&mu_); + GRPC_CARES_TRACE_LOG("AresRequest:%p Orphan ares_request_:%p", this, + ares_request_.get()); + grpc_cancel_ares_request(ares_request_.get()); + } + Unref(); + } + + private: + static void OnDnsLookupDone(void* arg, grpc_error_handle error) { + AresRequest* r = static_cast(arg); + std::vector resolved_addresses; + { + absl::MutexLock lock(&r->mu_); + GRPC_CARES_TRACE_LOG("AresRequest:%p OnDnsLookupDone error:%s", r, + grpc_error_std_string(error).c_str()); + if (r->addresses_ != nullptr) { + resolved_addresses.reserve(r->addresses_->size()); + for (const auto& server_address : *r->addresses_) { + resolved_addresses.push_back(server_address.address()); + } + } + } + if (error == GRPC_ERROR_NONE) { + // it's safe to run this inline since we've already been scheduled + // on the ExecCtx + r->on_resolve_address_done_(std::move(resolved_addresses)); + } else { + r->on_resolve_address_done_(grpc_error_to_absl_status(error)); + } + r->Unref(); + } + + // mutex to synchronize access to this object (but not to the ares_request + // object itself). + absl::Mutex mu_; + // the name to resolve + const std::string name_; + // the default port to use if name doesn't have one + const std::string default_port_; + // parties interested in our I/O + grpc_pollset_set* const interested_parties_; + // user-provided completion callback + const std::function>)> + on_resolve_address_done_; + // currently resolving addresses + std::unique_ptr addresses_ ABSL_GUARDED_BY(mu_); + // closure to call when the resolve_address_ares request completes + // a closure wrapping on_resolve_address_done, which should be invoked + // when the grpc_dns_lookup_ares operation is done. + grpc_closure on_dns_lookup_done_ ABSL_GUARDED_BY(mu_); + // underlying ares_request that the query is performed on + std::unique_ptr ares_request_ ABSL_GUARDED_BY(mu_); + }; + + // gets the singleton instance, possibly creating it first + static AresDNSResolver* GetOrCreate() { + static AresDNSResolver* instance = new AresDNSResolver(); + return instance; + } + + OrphanablePtr ResolveName( + absl::string_view name, absl::string_view default_port, + grpc_pollset_set* interested_parties, + std::function>)> + on_done) override { + return MakeOrphanable(name, default_port, interested_parties, + std::move(on_done)); + } + + absl::StatusOr> ResolveNameBlocking( + absl::string_view name, absl::string_view default_port) override { + // TODO(apolcyn): change this to wrap the async version of the c-ares + // API with a promise, and remove the reference to the previous resolver. + return default_resolver_->ResolveNameBlocking(name, default_port); + } -static grpc_address_resolver_vtable ares_resolver = { - grpc_resolve_address_ares, blocking_resolve_address_ares}; + private: + // the previous default DNS resolver, used to delegate blocking DNS calls to + DNSResolver* default_resolver_ = GetDNSResolver(); +}; -static bool should_use_ares(const char* resolver_env) { +bool ShouldUseAres(const char* resolver_env) { // TODO(lidiz): Remove the "g_custom_iomgr_enabled" flag once c-ares support // custom IO managers (e.g. gevent). return !g_custom_iomgr_enabled && @@ -501,13 +610,17 @@ static bool should_use_ares(const char* resolver_env) { gpr_stricmp(resolver_env, "ares") == 0); } -static bool g_use_ares_dns_resolver; +bool g_use_ares_dns_resolver; + +} // namespace + +} // namespace grpc_core void grpc_resolver_dns_ares_init() { grpc_core::UniquePtr resolver = GPR_GLOBAL_CONFIG_GET(grpc_dns_resolver); - if (should_use_ares(resolver.get())) { - g_use_ares_dns_resolver = true; + if (grpc_core::ShouldUseAres(resolver.get())) { + grpc_core::g_use_ares_dns_resolver = true; gpr_log(GPR_DEBUG, "Using ares dns resolver"); address_sorting_init(); grpc_error_handle error = grpc_ares_init(); @@ -515,19 +628,16 @@ void grpc_resolver_dns_ares_init() { GRPC_LOG_IF_ERROR("grpc_ares_init() failed", error); return; } - if (default_resolver == nullptr) { - default_resolver = grpc_resolve_address_impl; - } - grpc_set_resolver_impl(&ares_resolver); + grpc_core::SetDNSResolver(grpc_core::AresDNSResolver::GetOrCreate()); grpc_core::ResolverRegistry::Builder::RegisterResolverFactory( - absl::make_unique()); + absl::make_unique()); } else { - g_use_ares_dns_resolver = false; + grpc_core::g_use_ares_dns_resolver = false; } } void grpc_resolver_dns_ares_shutdown() { - if (g_use_ares_dns_resolver) { + if (grpc_core::g_use_ares_dns_resolver) { address_sorting_shutdown(); grpc_ares_cleanup(); } 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 9387c764fe8..200a73dbdfc 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 @@ -822,7 +822,7 @@ void grpc_dns_lookup_ares_continue_after_check_localhost_and_ip_literals_locked( GRPC_ERROR_STR_TARGET_ADDRESS, name); goto error_cleanup; } else if (port.empty()) { - if (default_port == nullptr) { + if (default_port == nullptr || strlen(default_port) == 0) { error = grpc_error_set_str( GRPC_ERROR_CREATE_FROM_STATIC_STRING("no port in name"), GRPC_ERROR_STR_TARGET_ADDRESS, name); @@ -913,7 +913,7 @@ static bool inner_resolve_as_ip_literal_locked( return false; } if (port->empty()) { - if (default_port == nullptr) { + if (default_port == nullptr || strlen(default_port) == 0) { gpr_log(GPR_ERROR, "No port or default port for %s while attempting to resolve as " "ip literal.", @@ -976,7 +976,7 @@ static bool inner_maybe_resolve_localhost_manually_locked( return false; } if (port->empty()) { - if (default_port == nullptr) { + if (default_port == nullptr || strlen(default_port) == 0) { gpr_log(GPR_ERROR, "No port or default port for %s during manual localhost " "resolution check.", @@ -1082,6 +1082,8 @@ grpc_ares_request* (*grpc_dns_lookup_ares)( static void grpc_cancel_ares_request_impl(grpc_ares_request* r) { GPR_ASSERT(r != nullptr); grpc_core::MutexLock lock(&r->mu); + GRPC_CARES_TRACE_LOG("request:%p grpc_cancel_ares_request ev_driver:%p", r, + r->ev_driver); if (r->ev_driver != nullptr) { grpc_ares_ev_driver_shutdown_locked(r->ev_driver); } @@ -1109,71 +1111,4 @@ grpc_error_handle grpc_ares_init(void) { return GRPC_ERROR_NONE; } void grpc_ares_cleanup(void) {} #endif // GPR_WINDOWS -/* - * grpc_resolve_address_ares related structs and functions - */ - -typedef struct grpc_resolve_address_ares_request { - // synchronizers access to this object (but not to the ares_request itself) - absl::Mutex mu; - // the pointer to receive the resolved addresses - grpc_resolved_addresses** addrs_out ABSL_GUARDED_BY(mu); - // currently resolving addresses - std::unique_ptr addresses ABSL_GUARDED_BY(mu); - // closure to call when the resolve_address_ares request completes - grpc_closure* on_resolve_address_done ABSL_GUARDED_BY(mu); - // a closure wrapping on_resolve_address_done, which should be invoked when - // the grpc_dns_lookup_ares operation is done. - grpc_closure on_dns_lookup_done ABSL_GUARDED_BY(mu); - // underlying ares_request that the query is performed on - std::unique_ptr ares_request ABSL_GUARDED_BY(mu); -} grpc_resolve_address_ares_request; - -static void on_dns_lookup_done(void* arg, grpc_error_handle error) { - auto r = std::unique_ptr( - static_cast(arg)); - absl::MutexLock lock(&r->mu); - grpc_resolved_addresses** resolved_addresses = r->addrs_out; - if (r->addresses == nullptr || r->addresses->empty()) { - *resolved_addresses = nullptr; - } else { - *resolved_addresses = static_cast( - gpr_zalloc(sizeof(grpc_resolved_addresses))); - (*resolved_addresses)->naddrs = r->addresses->size(); - (*resolved_addresses)->addrs = - static_cast(gpr_zalloc( - sizeof(grpc_resolved_address) * (*resolved_addresses)->naddrs)); - for (size_t i = 0; i < (*resolved_addresses)->naddrs; ++i) { - memcpy(&(*resolved_addresses)->addrs[i], &(*r->addresses)[i].address(), - sizeof(grpc_resolved_address)); - } - } - grpc_core::ExecCtx::Run(DEBUG_LOCATION, r->on_resolve_address_done, - GRPC_ERROR_REF(error)); -} - -static void grpc_resolve_address_ares_impl(const char* name, - const char* default_port, - grpc_pollset_set* interested_parties, - grpc_closure* on_done, - grpc_resolved_addresses** addrs) { - grpc_resolve_address_ares_request* r = - new grpc_resolve_address_ares_request(); - absl::MutexLock lock(&r->mu); - r->addrs_out = addrs; - r->on_resolve_address_done = on_done; - GRPC_CLOSURE_INIT(&r->on_dns_lookup_done, on_dns_lookup_done, r, - grpc_schedule_on_exec_ctx); - r->ares_request = std::unique_ptr(grpc_dns_lookup_ares( - nullptr /* dns_server */, name, default_port, interested_parties, - &r->on_dns_lookup_done, &r->addresses, nullptr /* balancer_addresses */, - nullptr /* service_config_json */, - GRPC_DNS_ARES_DEFAULT_QUERY_TIMEOUT_MS)); -} - -void (*grpc_resolve_address_ares)( - const char* name, const char* default_port, - grpc_pollset_set* interested_parties, grpc_closure* on_done, - grpc_resolved_addresses** addrs) = grpc_resolve_address_ares_impl; - #endif /* GRPC_ARES == 1 */ 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 247408741a9..95b0a938e6d 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 @@ -69,17 +69,6 @@ struct grpc_ares_request { grpc_error_handle error ABSL_GUARDED_BY(mu) = GRPC_ERROR_NONE; }; -/* Asynchronously resolve \a name. Use \a default_port if a port isn't - designated in \a name, otherwise use the port in \a name. grpc_ares_init() - must be called at least once before this function. \a on_done may be - called directly in this function without being scheduled with \a exec_ctx, - so it must not try to acquire locks that are being held by the caller. */ -extern void (*grpc_resolve_address_ares)(const char* name, - const char* default_port, - grpc_pollset_set* interested_parties, - grpc_closure* on_done, - grpc_resolved_addresses** addresses); - /* Asynchronously resolve \a name. It will try to resolve grpclb SRV records in addition to the normal address records. For normal address records, it uses \a default_port if a port isn't designated in \a name, otherwise it uses the diff --git a/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc b/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc index c694ebec245..30e7f7b626a 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc @@ -21,6 +21,7 @@ #include #include +#include "absl/functional/bind_front.h" #include "absl/strings/str_cat.h" #include @@ -47,9 +48,9 @@ namespace grpc_core { namespace { -class NativeDnsResolver : public Resolver { +class NativeClientChannelDNSResolver : public Resolver { public: - explicit NativeDnsResolver(ResolverArgs args); + explicit NativeClientChannelDNSResolver(ResolverArgs args); void StartLocked() override; @@ -60,15 +61,17 @@ class NativeDnsResolver : public Resolver { void ShutdownLocked() override; private: - ~NativeDnsResolver() override; + ~NativeClientChannelDNSResolver() override; void MaybeStartResolvingLocked(); void StartResolvingLocked(); static void OnNextResolution(void* arg, grpc_error_handle error); void OnNextResolutionLocked(grpc_error_handle error); - static void OnResolved(void* arg, grpc_error_handle error); - void OnResolvedLocked(grpc_error_handle error); + void OnResolved( + absl::StatusOr> addresses_or); + void OnResolvedLocked( + absl::StatusOr> addresses_or); /// name to resolve std::string name_to_resolve_; @@ -82,7 +85,6 @@ class NativeDnsResolver : public Resolver { bool shutdown_ = false; /// are we currently resolving? bool resolving_ = false; - grpc_closure on_resolved_; /// next resolution timer bool have_next_resolution_timer_ = false; grpc_timer next_resolution_timer_; @@ -93,11 +95,12 @@ class NativeDnsResolver : public Resolver { grpc_millis last_resolution_timestamp_ = -1; /// retry backoff state BackOff backoff_; - /// currently resolving addresses - grpc_resolved_addresses* addresses_ = nullptr; + /// tracks pending resolutions + OrphanablePtr dns_request_; }; -NativeDnsResolver::NativeDnsResolver(ResolverArgs args) +NativeClientChannelDNSResolver::NativeClientChannelDNSResolver( + ResolverArgs args) : name_to_resolve_(absl::StripPrefix(args.uri.path(), "/")), channel_args_(grpc_channel_args_copy(args.args)), work_serializer_(std::move(args.work_serializer)), @@ -118,41 +121,47 @@ NativeDnsResolver::NativeDnsResolver(ResolverArgs args) } } -NativeDnsResolver::~NativeDnsResolver() { +NativeClientChannelDNSResolver::~NativeClientChannelDNSResolver() { grpc_channel_args_destroy(channel_args_); grpc_pollset_set_destroy(interested_parties_); } -void NativeDnsResolver::StartLocked() { MaybeStartResolvingLocked(); } +void NativeClientChannelDNSResolver::StartLocked() { + MaybeStartResolvingLocked(); +} -void NativeDnsResolver::RequestReresolutionLocked() { +void NativeClientChannelDNSResolver::RequestReresolutionLocked() { if (!resolving_) { MaybeStartResolvingLocked(); } } -void NativeDnsResolver::ResetBackoffLocked() { +void NativeClientChannelDNSResolver::ResetBackoffLocked() { if (have_next_resolution_timer_) { grpc_timer_cancel(&next_resolution_timer_); } backoff_.Reset(); } -void NativeDnsResolver::ShutdownLocked() { +void NativeClientChannelDNSResolver::ShutdownLocked() { shutdown_ = true; if (have_next_resolution_timer_) { grpc_timer_cancel(&next_resolution_timer_); } + dns_request_.reset(); } -void NativeDnsResolver::OnNextResolution(void* arg, grpc_error_handle error) { - NativeDnsResolver* r = static_cast(arg); +void NativeClientChannelDNSResolver::OnNextResolution(void* arg, + grpc_error_handle error) { + NativeClientChannelDNSResolver* r = + static_cast(arg); (void)GRPC_ERROR_REF(error); // ref owned by lambda r->work_serializer_->Run([r, error]() { r->OnNextResolutionLocked(error); }, DEBUG_LOCATION); } -void NativeDnsResolver::OnNextResolutionLocked(grpc_error_handle error) { +void NativeClientChannelDNSResolver::OnNextResolutionLocked( + grpc_error_handle error) { have_next_resolution_timer_ = false; if (error == GRPC_ERROR_NONE && !resolving_) { StartResolvingLocked(); @@ -161,28 +170,29 @@ void NativeDnsResolver::OnNextResolutionLocked(grpc_error_handle error) { GRPC_ERROR_UNREF(error); } -void NativeDnsResolver::OnResolved(void* arg, grpc_error_handle error) { - NativeDnsResolver* r = static_cast(arg); - (void)GRPC_ERROR_REF(error); // owned by lambda - r->work_serializer_->Run([r, error]() { r->OnResolvedLocked(error); }, - DEBUG_LOCATION); +void NativeClientChannelDNSResolver::OnResolved( + absl::StatusOr> addresses_or) { + work_serializer_->Run( + [this, addresses_or]() mutable { + OnResolvedLocked(std::move(addresses_or)); + }, + DEBUG_LOCATION); } -void NativeDnsResolver::OnResolvedLocked(grpc_error_handle error) { +void NativeClientChannelDNSResolver::OnResolvedLocked( + absl::StatusOr> addresses_or) { GPR_ASSERT(resolving_); resolving_ = false; + dns_request_.reset(); if (shutdown_) { Unref(DEBUG_LOCATION, "dns-resolving"); - GRPC_ERROR_UNREF(error); return; } - if (addresses_ != nullptr) { + if (addresses_or.ok()) { ServerAddressList addresses; - for (size_t i = 0; i < addresses_->naddrs; ++i) { - addresses.emplace_back(&addresses_->addrs[i].addr, - addresses_->addrs[i].len, nullptr /* args */); + for (auto& addr : *addresses_or) { + addresses.emplace_back(addr, nullptr /* args */); } - grpc_resolved_addresses_destroy(addresses_); Result result; result.addresses = std::move(addresses); result.args = grpc_channel_args_copy(channel_args_); @@ -191,11 +201,10 @@ void NativeDnsResolver::OnResolvedLocked(grpc_error_handle error) { // next request gets triggered. backoff_.Reset(); } else { + std::string error_message = addresses_or.status().ToString(); gpr_log(GPR_INFO, "dns resolution failed (will retry): %s", - grpc_error_std_string(error).c_str()); + error_message.c_str()); // Return transient error. - std::string error_message; - grpc_error_get_str(error, GRPC_ERROR_STR_DESCRIPTION, &error_message); Result result; result.addresses = absl::UnavailableError(absl::StrCat( "DNS resolution failed for ", name_to_resolve_, ": ", error_message)); @@ -219,15 +228,15 @@ void NativeDnsResolver::OnResolvedLocked(grpc_error_handle error) { } else { gpr_log(GPR_DEBUG, "retrying immediately"); } - GRPC_CLOSURE_INIT(&on_next_resolution_, NativeDnsResolver::OnNextResolution, - this, grpc_schedule_on_exec_ctx); + GRPC_CLOSURE_INIT(&on_next_resolution_, + NativeClientChannelDNSResolver::OnNextResolution, this, + grpc_schedule_on_exec_ctx); grpc_timer_init(&next_resolution_timer_, next_try, &on_next_resolution_); } Unref(DEBUG_LOCATION, "dns-resolving"); - GRPC_ERROR_UNREF(error); } -void NativeDnsResolver::MaybeStartResolvingLocked() { +void NativeClientChannelDNSResolver::MaybeStartResolvingLocked() { // If there is an existing timer, the time it fires is the earliest time we // can start the next resolution. if (have_next_resolution_timer_) return; @@ -253,7 +262,7 @@ void NativeDnsResolver::MaybeStartResolvingLocked() { // callback as part of the type system. Ref(DEBUG_LOCATION, "next_resolution_timer_cooldown").release(); GRPC_CLOSURE_INIT(&on_next_resolution_, - NativeDnsResolver::OnNextResolution, this, + NativeClientChannelDNSResolver::OnNextResolution, this, grpc_schedule_on_exec_ctx); grpc_timer_init(&next_resolution_timer_, ExecCtx::Get()->Now() + ms_until_next_resolution, @@ -264,7 +273,7 @@ void NativeDnsResolver::MaybeStartResolvingLocked() { StartResolvingLocked(); } -void NativeDnsResolver::StartResolvingLocked() { +void NativeClientChannelDNSResolver::StartResolvingLocked() { gpr_log(GPR_DEBUG, "Start resolving."); // TODO(roth): We currently deal with this ref manually. Once the // new closure API is done, find a way to track this ref with the timer @@ -272,11 +281,10 @@ void NativeDnsResolver::StartResolvingLocked() { Ref(DEBUG_LOCATION, "dns-resolving").release(); GPR_ASSERT(!resolving_); resolving_ = true; - addresses_ = nullptr; - GRPC_CLOSURE_INIT(&on_resolved_, NativeDnsResolver::OnResolved, this, - grpc_schedule_on_exec_ctx); - grpc_resolve_address(name_to_resolve_.c_str(), kDefaultSecurePort, - interested_parties_, &on_resolved_, &addresses_); + dns_request_ = GetDNSResolver()->ResolveName( + name_to_resolve_, kDefaultSecurePort, interested_parties_, + absl::bind_front(&NativeClientChannelDNSResolver::OnResolved, this)); + dns_request_->Start(); last_resolution_timestamp_ = ExecCtx::Get()->Now(); } @@ -284,7 +292,7 @@ void NativeDnsResolver::StartResolvingLocked() { // Factory // -class NativeDnsResolverFactory : public ResolverFactory { +class NativeClientChannelDNSResolverFactory : public ResolverFactory { public: bool IsValidUri(const URI& uri) const override { if (GPR_UNLIKELY(!uri.authority().empty())) { @@ -300,7 +308,7 @@ class NativeDnsResolverFactory : public ResolverFactory { OrphanablePtr CreateResolver(ResolverArgs args) const override { if (!IsValidUri(args.uri)) return nullptr; - return MakeOrphanable(std::move(args)); + return MakeOrphanable(std::move(args)); } const char* scheme() const override { return "dns"; } @@ -316,7 +324,7 @@ void grpc_resolver_dns_native_init() { if (gpr_stricmp(resolver.get(), "native") == 0) { gpr_log(GPR_DEBUG, "Using native dns resolver"); grpc_core::ResolverRegistry::Builder::RegisterResolverFactory( - absl::make_unique()); + absl::make_unique()); } else { grpc_core::ResolverRegistry::Builder::InitRegistry(); grpc_core::ResolverFactory* existing_factory = @@ -324,7 +332,8 @@ void grpc_resolver_dns_native_init() { if (existing_factory == nullptr) { gpr_log(GPR_DEBUG, "Using native dns resolver"); grpc_core::ResolverRegistry::Builder::RegisterResolverFactory( - absl::make_unique()); + absl::make_unique< + grpc_core::NativeClientChannelDNSResolverFactory>()); } } } diff --git a/src/core/ext/transport/chttp2/server/chttp2_server.cc b/src/core/ext/transport/chttp2/server/chttp2_server.cc index 1badf18ec2b..4016d4f7c94 100644 --- a/src/core/ext/transport/chttp2/server/chttp2_server.cc +++ b/src/core/ext/transport/chttp2/server/chttp2_server.cc @@ -55,6 +55,7 @@ #include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/surface/api_trace.h" #include "src/core/lib/surface/server.h" +#include "src/core/lib/transport/error_utils.h" #include "src/core/lib/uri/uri_parser.h" namespace grpc_core { @@ -882,7 +883,7 @@ grpc_error_handle Chttp2ServerAddPort(Server* server, const char* addr, args_modifier); } *port_num = -1; - grpc_resolved_addresses* resolved = nullptr; + absl::StatusOr> resolved_or; std::vector error_list; std::string parsed_addr = URI::PercentDecode(addr); absl::string_view parsed_addr_unprefixed{parsed_addr}; @@ -890,26 +891,26 @@ grpc_error_handle Chttp2ServerAddPort(Server* server, const char* addr, grpc_error_handle error = [&]() { grpc_error_handle error = GRPC_ERROR_NONE; if (absl::ConsumePrefix(&parsed_addr_unprefixed, kUnixUriPrefix)) { - error = - grpc_resolve_unix_domain_address(parsed_addr_unprefixed, &resolved); + resolved_or = grpc_resolve_unix_domain_address(parsed_addr_unprefixed); } else if (absl::ConsumePrefix(&parsed_addr_unprefixed, kUnixAbstractUriPrefix)) { - error = grpc_resolve_unix_abstract_domain_address(parsed_addr_unprefixed, - &resolved); + resolved_or = + grpc_resolve_unix_abstract_domain_address(parsed_addr_unprefixed); } else { - error = grpc_blocking_resolve_address(parsed_addr.c_str(), "https", - &resolved); + resolved_or = GetDNSResolver()->ResolveNameBlocking(parsed_addr, "https"); + } + if (!resolved_or.ok()) { + return absl_status_to_grpc_error(resolved_or.status()); } - if (error != GRPC_ERROR_NONE) return error; // Create a listener for each resolved address. - for (size_t i = 0; i < resolved->naddrs; i++) { + for (auto& addr : *resolved_or) { // If address has a wildcard port (0), use the same port as a previous // listener. - if (*port_num != -1 && grpc_sockaddr_get_port(&resolved->addrs[i]) == 0) { - grpc_sockaddr_set_port(&resolved->addrs[i], *port_num); + if (*port_num != -1 && grpc_sockaddr_get_port(&addr) == 0) { + grpc_sockaddr_set_port(&addr, *port_num); } int port_temp = -1; - error = Chttp2ServerListener::Create(server, &resolved->addrs[i], + error = Chttp2ServerListener::Create(server, &addr, grpc_channel_args_copy(args), args_modifier, &port_temp); if (error != GRPC_ERROR_NONE) { @@ -922,17 +923,17 @@ grpc_error_handle Chttp2ServerAddPort(Server* server, const char* addr, } } } - if (error_list.size() == resolved->naddrs) { + if (error_list.size() == resolved_or->size()) { std::string msg = absl::StrFormat("No address added out of total %" PRIuPTR " resolved", - resolved->naddrs); + resolved_or->size()); return GRPC_ERROR_CREATE_REFERENCING_FROM_COPIED_STRING( msg.c_str(), error_list.data(), error_list.size()); } else if (!error_list.empty()) { std::string msg = absl::StrFormat( "Only %" PRIuPTR " addresses added out of total %" PRIuPTR " resolved", - resolved->naddrs - error_list.size(), resolved->naddrs); + resolved_or->size() - error_list.size(), resolved_or->size()); error = GRPC_ERROR_CREATE_REFERENCING_FROM_COPIED_STRING( msg.c_str(), error_list.data(), error_list.size()); gpr_log(GPR_INFO, "WARNING: %s", grpc_error_std_string(error).c_str()); @@ -945,9 +946,6 @@ grpc_error_handle Chttp2ServerAddPort(Server* server, const char* addr, GRPC_ERROR_UNREF(error); } grpc_channel_args_destroy(args); - if (resolved != nullptr) { - grpc_resolved_addresses_destroy(resolved); - } if (error != GRPC_ERROR_NONE) *port_num = 0; return error; } diff --git a/src/core/lib/http/httpcli.cc b/src/core/lib/http/httpcli.cc index 071f8bfad4a..99660818772 100644 --- a/src/core/lib/http/httpcli.cc +++ b/src/core/lib/http/httpcli.cc @@ -24,6 +24,7 @@ #include +#include "absl/functional/bind_front.h" #include "absl/strings/str_format.h" #include "absl/strings/string_view.h" @@ -44,6 +45,7 @@ #include "src/core/lib/iomgr/tcp_client.h" #include "src/core/lib/resource_quota/api.h" #include "src/core/lib/slice/slice_internal.h" +#include "src/core/lib/transport/error_utils.h" namespace grpc_core { namespace { @@ -75,17 +77,14 @@ class InternalRequest { GRPC_CLOSURE_INIT(&done_write_, DoneWrite, this, grpc_schedule_on_exec_ctx); GPR_ASSERT(pollent); grpc_polling_entity_add_to_pollset_set(pollent_, context->pollset_set); - grpc_resolve_address( + dns_request_ = GetDNSResolver()->ResolveName( host_.c_str(), handshaker_->default_port, context_->pollset_set, - GRPC_CLOSURE_CREATE(OnResolved, this, grpc_schedule_on_exec_ctx), - &addresses_); + absl::bind_front(&InternalRequest::OnResolved, this)); + dns_request_->Start(); } ~InternalRequest() { grpc_http_parser_destroy(&parser_); - if (addresses_ != nullptr) { - grpc_resolved_addresses_destroy(addresses_); - } if (ep_ != nullptr) { grpc_endpoint_destroy(ep_); } @@ -108,7 +107,7 @@ class InternalRequest { overall_error_ = GRPC_ERROR_CREATE_FROM_STATIC_STRING("Failed HTTP/1 client request"); } - grpc_resolved_address* addr = &addresses_->addrs[next_address_ - 1]; + const grpc_resolved_address* addr = &addresses_[next_address_ - 1]; std::string addr_text = grpc_sockaddr_to_uri(addr); overall_error_ = grpc_error_add_child( overall_error_, @@ -193,16 +192,15 @@ class InternalRequest { } void NextAddress(grpc_error_handle error) { - grpc_resolved_address* addr; if (error != GRPC_ERROR_NONE) { AppendError(error); } - if (next_address_ == addresses_->naddrs) { + if (next_address_ == addresses_.size()) { Finish(GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING( "Failed HTTP requests to all targets", &overall_error_, 1)); return; } - addr = &addresses_->addrs[next_address_++]; + const grpc_resolved_address* addr = &addresses_[next_address_++]; GRPC_CLOSURE_INIT(&connected_, OnConnected, this, grpc_schedule_on_exec_ctx); grpc_arg rq_arg = grpc_channel_arg_pointer_create( @@ -217,19 +215,21 @@ class InternalRequest { grpc_channel_args_destroy(args); } - static void OnResolved(void* arg, grpc_error_handle error) { - InternalRequest* req = static_cast(arg); - if (error != GRPC_ERROR_NONE) { - req->Finish(GRPC_ERROR_REF(error)); + void OnResolved( + absl::StatusOr> addresses_or) { + dns_request_.reset(); + if (!addresses_or.ok()) { + Finish(absl_status_to_grpc_error(addresses_or.status())); return; } - req->next_address_ = 0; - req->NextAddress(GRPC_ERROR_NONE); + addresses_ = std::move(*addresses_or); + next_address_ = 0; + NextAddress(GRPC_ERROR_NONE); } grpc_slice request_text_; grpc_http_parser parser_; - grpc_resolved_addresses* addresses_ = nullptr; + std::vector addresses_; size_t next_address_ = 0; grpc_endpoint* ep_ = nullptr; ResourceQuotaRefPtr resource_quota_; @@ -248,6 +248,7 @@ class InternalRequest { grpc_closure done_write_; grpc_closure connected_; grpc_error_handle overall_error_ = GRPC_ERROR_NONE; + OrphanablePtr dns_request_; }; } // namespace diff --git a/src/core/lib/iomgr/event_engine/iomgr.cc b/src/core/lib/iomgr/event_engine/iomgr.cc index c392100855b..2fc7f046d9a 100644 --- a/src/core/lib/iomgr/event_engine/iomgr.cc +++ b/src/core/lib/iomgr/event_engine/iomgr.cc @@ -18,6 +18,7 @@ #include "src/core/lib/debug/trace.h" #include "src/core/lib/iomgr/closure.h" +#include "src/core/lib/iomgr/event_engine/resolver.h" #include "src/core/lib/iomgr/iomgr_internal.h" #include "src/core/lib/iomgr/tcp_client.h" #include "src/core/lib/iomgr/tcp_server.h" @@ -29,7 +30,6 @@ extern grpc_tcp_server_vtable grpc_event_engine_tcp_server_vtable; extern grpc_timer_vtable grpc_event_engine_timer_vtable; extern grpc_pollset_vtable grpc_event_engine_pollset_vtable; extern grpc_pollset_set_vtable grpc_event_engine_pollset_set_vtable; -extern grpc_address_resolver_vtable grpc_event_engine_resolver_vtable; // Disabled by default. grpc_polling_trace must be defined in all iomgr // implementations due to its usage in lockfree_event. @@ -75,7 +75,8 @@ void grpc_set_default_iomgr_platform() { grpc_set_timer_impl(&grpc_event_engine_timer_vtable); grpc_set_pollset_vtable(&grpc_event_engine_pollset_vtable); grpc_set_pollset_set_vtable(&grpc_event_engine_pollset_set_vtable); - grpc_set_resolver_impl(&grpc_event_engine_resolver_vtable); + grpc_core::SetDNSResolver( + grpc_core::experimental::EventEngineDNSResolver::GetOrCreate()); grpc_set_iomgr_platform_vtable(&vtable); } diff --git a/src/core/lib/iomgr/event_engine/resolver.cc b/src/core/lib/iomgr/event_engine/resolver.cc index 4cea6ce7c8a..fc3fc46e283 100644 --- a/src/core/lib/iomgr/event_engine/resolver.cc +++ b/src/core/lib/iomgr/event_engine/resolver.cc @@ -24,11 +24,15 @@ #include "src/core/lib/iomgr/error.h" #include "src/core/lib/iomgr/event_engine/promise.h" #include "src/core/lib/iomgr/event_engine/resolved_address_internal.h" +#include "src/core/lib/iomgr/event_engine/resolver.h" #include "src/core/lib/iomgr/resolve_address.h" +#include "src/core/lib/iomgr/resolve_address_impl.h" #include "src/core/lib/iomgr/work_serializer.h" #include "src/core/lib/surface/init.h" #include "src/core/lib/transport/error_utils.h" +namespace grpc_core { +namespace experimental { namespace { using ::grpc_event_engine::experimental::CreateGRPCResolvedAddress; using ::grpc_event_engine::experimental::EventEngine; @@ -39,77 +43,91 @@ using ::grpc_event_engine::experimental::Promise; /// /// This provides a place to store the ownership of the DNSResolver object until /// the request is complete. -class DnsRequest { +class EventEngineDNSRequest : DNSRequest { public: - DnsRequest(std::unique_ptr dns_resolver, - absl::string_view address, absl::string_view default_port, - grpc_closure* on_done, grpc_resolved_addresses** addresses) + EventEngineDNSRequest(std::unique_ptr dns_resolver, + absl::string_view name, absl::string_view default_port, + grpc_closure* on_done, + std::vector* addresses) : dns_resolver_(std::move(dns_resolver)), - cb_(on_done), - addresses_(addresses) { + name_(std::string(name)), + default_port_(std::string(default_port)), + on_done_(std::move(on_done)) {} + + void Start() override { + if (dns_resolver_ == nullptr) { + new DNSCallbackExecCtxScheduler( + std::move(on_done_), + absl::UnknownError("Failed to get DNS Resolver.")); + return; + } + Ref().release(); // ref held by pending resolution dns_resolver_->LookupHostname( - absl::bind_front(&DnsRequest::OnLookupComplete, this), address, - default_port, absl::InfiniteFuture()); + absl::bind_front(&EventEngineDNSRequest::OnLookupComplete, this), name_, + default_port_, absl::InfiniteFuture()); } + // TOOD(hork): implement cancellation; currently it's a no-op + void Orphan() override { Unref(); } + private: void OnLookupComplete( absl::StatusOr> addresses) { - grpc_core::ExecCtx exec_ctx; + ExecCtx exec_ctx; // Convert addresses to iomgr form. - *addresses_ = static_cast( - gpr_malloc(sizeof(grpc_resolved_addresses))); - (*addresses_)->naddrs = addresses->size(); - (*addresses_)->addrs = static_cast( - gpr_malloc(sizeof(grpc_resolved_address) * addresses->size())); + std::vector result; + results.reserve(addresses->size()); for (size_t i = 0; i < addresses->size(); ++i) { - (*addresses_)->addrs[i] = CreateGRPCResolvedAddress((*addresses)[i]); + results.push_back(CreateGRPCResolvedAddress(addresses[i])); + } + if (addresses.ok()) { + on_done_(std::move(result)); + } else { + on_done_(addresses.status()); } - grpc_closure* cb = cb_; - delete this; - grpc_core::Closure::Run(DEBUG_LOCATION, cb, - absl_status_to_grpc_error(addresses.status())); + Unref(); } std::unique_ptr dns_resolver_; - grpc_closure* cb_; - grpc_resolved_addresses** addresses_; + const std::string name_; + const std::string default_port_; + std::function>)> + on_done_; }; -void resolve_address(const char* addr, const char* default_port, - grpc_pollset_set* /* interested_parties */, - grpc_closure* on_done, - grpc_resolved_addresses** addresses) { - std::unique_ptr dns_resolver = - GetDefaultEventEngine()->GetDNSResolver(); - if (dns_resolver == nullptr) { - grpc_core::ExecCtx::Run( - DEBUG_LOCATION, on_done, - GRPC_ERROR_CREATE_FROM_STATIC_STRING("Failed to get DNS Resolver.")); - return; - } - new DnsRequest(std::move(dns_resolver), addr, default_port, on_done, - addresses); +} // namespace + +DNSResolver* EventEngineDNSResolver::GetOrCreate() { + static EventEngineDNSResolver* instance = new EventEngineDNSResolver(); + return instance; } -void blocking_handle_async_resolve_done(void* arg, grpc_error_handle error) { - static_cast*>(arg)->Set(std::move(error)); +OrphanablePtr EventEngineDNSResolver::ResolveName( + absl::string_view name, absl::string_view default_port, + grpc_pollset_set* /* interested_parties */, + std::function>)> + on_done) { + std::unique_ptr dns_resolver = + GetDefaultEventEngine()->GetDNSResolver(); + return MakeOrphanable( + std::move(dns_resolver), name, default_port, std::move(on_done)); } -grpc_error_handle blocking_resolve_address( - const char* name, const char* default_port, - grpc_resolved_addresses** addresses) { +absl::StatusOr> +EventEngineDNSResolver::ResolveNameBlocking(absl::string_view name, + absl::string_view default_port) { grpc_closure on_done; - Promise evt; - GRPC_CLOSURE_INIT(&on_done, blocking_handle_async_resolve_done, &evt, - grpc_schedule_on_exec_ctx); - resolve_address(name, default_port, nullptr, &on_done, addresses); + Promise>> evt; + auto r = ResolveName( + name, default_port, + [&evt](void(absl::StatusOr> result) { + evt.Set(std::move(result)); + })); + r->Start(); return evt.Get(); } -} // namespace - -grpc_address_resolver_vtable grpc_event_engine_resolver_vtable{ - resolve_address, blocking_resolve_address}; +} // namespace experimental +} // namespace grpc_core #endif // GRPC_USE_EVENT_ENGINE diff --git a/src/core/lib/iomgr/event_engine/resolver.h b/src/core/lib/iomgr/event_engine/resolver.h new file mode 100644 index 00000000000..09c98ae7b0d --- /dev/null +++ b/src/core/lib/iomgr/event_engine/resolver.h @@ -0,0 +1,56 @@ +// +// Copyright 2015 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#ifndef GRPC_CORE_LIB_IOMGR_EVENT_ENGINE_RESOLVER_H +#define GRPC_CORE_LIB_IOMGR_EVENT_ENGINE_RESOLVER_H + +#include + +#include +#include + +#include + +#include +#include +#include +#include + +#include "src/core/lib/iomgr/port.h" +#include "src/core/lib/iomgr/resolve_address.h" + +namespace grpc_core { +namespace experimental { + +class EventEngineDNSResolver : public DNSResolver { + public: + // Gets the singleton instance, creating it first if it doesn't exist + static EventEngineDNSResolver* GetOrCreate(); + + OrphanablePtr ResolveName( + absl::string_view name, absl::string_view default_port, + grpc_pollset_set* interested_parties, + std::function>)> + on_done) override; + + absl::StatusOr> ResolveNameBlocking( + absl::string_view name, absl::string_view default_port) override; +}; + +} // namespace experimental +} // namespace grpc_core + +#endif // GRPC_CORE_LIB_IOMGR_EVENT_ENGINE_RESOLVER_H diff --git a/src/core/lib/iomgr/iomgr_custom.cc b/src/core/lib/iomgr/iomgr_custom.cc index 70f4e1b7b13..f9acd075269 100644 --- a/src/core/lib/iomgr/iomgr_custom.cc +++ b/src/core/lib/iomgr/iomgr_custom.cc @@ -68,7 +68,10 @@ void grpc_custom_iomgr_init(grpc_socket_vtable* socket, grpc_custom_timer_init(timer); grpc_custom_pollset_init(poller); grpc_custom_pollset_set_init(); - grpc_custom_resolver_init(resolver); + grpc_core::CustomDNSResolver::Create(resolver); + grpc_core::CustomDNSResolver* custom_dns_resolver = + grpc_core::CustomDNSResolver::Get(); + grpc_core::SetDNSResolver(custom_dns_resolver); grpc_set_iomgr_platform_vtable(&vtable); } diff --git a/src/core/lib/iomgr/iomgr_posix.cc b/src/core/lib/iomgr/iomgr_posix.cc index 2450ef0beee..437d0327507 100644 --- a/src/core/lib/iomgr/iomgr_posix.cc +++ b/src/core/lib/iomgr/iomgr_posix.cc @@ -26,6 +26,7 @@ #include "src/core/lib/iomgr/ev_posix.h" #include "src/core/lib/iomgr/iomgr_internal.h" #include "src/core/lib/iomgr/resolve_address.h" +#include "src/core/lib/iomgr/resolve_address_posix.h" #include "src/core/lib/iomgr/tcp_client.h" #include "src/core/lib/iomgr/tcp_posix.h" #include "src/core/lib/iomgr/tcp_server.h" @@ -36,7 +37,6 @@ extern grpc_tcp_client_vtable grpc_posix_tcp_client_vtable; extern grpc_timer_vtable grpc_generic_timer_vtable; extern grpc_pollset_vtable grpc_posix_pollset_vtable; extern grpc_pollset_set_vtable grpc_posix_pollset_set_vtable; -extern grpc_address_resolver_vtable grpc_posix_resolver_vtable; static void iomgr_platform_init(void) { grpc_wakeup_fd_global_init(); @@ -79,7 +79,7 @@ void grpc_set_default_iomgr_platform() { grpc_set_timer_impl(&grpc_generic_timer_vtable); grpc_set_pollset_vtable(&grpc_posix_pollset_vtable); grpc_set_pollset_set_vtable(&grpc_posix_pollset_set_vtable); - grpc_set_resolver_impl(&grpc_posix_resolver_vtable); + grpc_core::SetDNSResolver(grpc_core::NativeDNSResolver::GetOrCreate()); grpc_set_iomgr_platform_vtable(&vtable); } diff --git a/src/core/lib/iomgr/iomgr_posix_cfstream.cc b/src/core/lib/iomgr/iomgr_posix_cfstream.cc index 27af38e4b0c..74cd19cc25a 100644 --- a/src/core/lib/iomgr/iomgr_posix_cfstream.cc +++ b/src/core/lib/iomgr/iomgr_posix_cfstream.cc @@ -41,6 +41,7 @@ #include "src/core/lib/iomgr/ev_posix.h" #include "src/core/lib/iomgr/iomgr_internal.h" #include "src/core/lib/iomgr/resolve_address.h" +#include "src/core/lib/iomgr/resolve_address_posix.h" #include "src/core/lib/iomgr/tcp_client.h" #include "src/core/lib/iomgr/tcp_posix.h" #include "src/core/lib/iomgr/tcp_server.h" @@ -55,7 +56,6 @@ extern grpc_tcp_client_vtable grpc_cfstream_client_vtable; extern grpc_timer_vtable grpc_generic_timer_vtable; extern grpc_pollset_vtable grpc_posix_pollset_vtable; extern grpc_pollset_set_vtable grpc_posix_pollset_set_vtable; -extern grpc_address_resolver_vtable grpc_posix_resolver_vtable; static void apple_iomgr_platform_init(void) { grpc_pollset_global_init(); } @@ -178,7 +178,7 @@ void grpc_set_default_iomgr_platform() { grpc_set_iomgr_platform_vtable(&apple_vtable); } grpc_set_timer_impl(&grpc_generic_timer_vtable); - grpc_set_resolver_impl(&grpc_posix_resolver_vtable); + grpc_core::SetDNSResolver(grpc_core::NativeDNSResolver::GetOrCreate()); } bool grpc_iomgr_run_in_background() { diff --git a/src/core/lib/iomgr/iomgr_windows.cc b/src/core/lib/iomgr/iomgr_windows.cc index 93fdaf85879..87509d8a039 100644 --- a/src/core/lib/iomgr/iomgr_windows.cc +++ b/src/core/lib/iomgr/iomgr_windows.cc @@ -28,6 +28,7 @@ #include "src/core/lib/iomgr/iomgr.h" #include "src/core/lib/iomgr/pollset_windows.h" #include "src/core/lib/iomgr/resolve_address.h" +#include "src/core/lib/iomgr/resolve_address_windows.h" #include "src/core/lib/iomgr/sockaddr_windows.h" #include "src/core/lib/iomgr/socket_windows.h" #include "src/core/lib/iomgr/tcp_client.h" @@ -39,7 +40,6 @@ extern grpc_tcp_client_vtable grpc_windows_tcp_client_vtable; extern grpc_timer_vtable grpc_generic_timer_vtable; extern grpc_pollset_vtable grpc_windows_pollset_vtable; extern grpc_pollset_set_vtable grpc_windows_pollset_set_vtable; -extern grpc_address_resolver_vtable grpc_windows_resolver_vtable; /* Windows' io manager is going to be fully designed using IO completion ports. All of what we're doing here is basically make sure that @@ -96,7 +96,7 @@ void grpc_set_default_iomgr_platform() { grpc_set_timer_impl(&grpc_generic_timer_vtable); grpc_set_pollset_vtable(&grpc_windows_pollset_vtable); grpc_set_pollset_set_vtable(&grpc_windows_pollset_set_vtable); - grpc_set_resolver_impl(&grpc_windows_resolver_vtable); + grpc_core::SetDNSResolver(grpc_core::NativeDNSResolver::GetOrCreate()); grpc_set_iomgr_platform_vtable(&vtable); } diff --git a/src/core/lib/iomgr/resolve_address.cc b/src/core/lib/iomgr/resolve_address.cc index a2e159a6703..3dfe1bf5482 100644 --- a/src/core/lib/iomgr/resolve_address.cc +++ b/src/core/lib/iomgr/resolve_address.cc @@ -24,32 +24,13 @@ namespace grpc_core { const char* kDefaultSecurePort = "https"; -} // namespace grpc_core - -grpc_address_resolver_vtable* grpc_resolve_address_impl; -void grpc_set_resolver_impl(grpc_address_resolver_vtable* vtable) { - grpc_resolve_address_impl = vtable; +namespace { +DNSResolver* g_dns_resolver; } -void grpc_resolve_address(const char* addr, const char* default_port, - grpc_pollset_set* interested_parties, - grpc_closure* on_done, - grpc_resolved_addresses** addresses) { - grpc_resolve_address_impl->resolve_address( - addr, default_port, interested_parties, on_done, addresses); -} +void SetDNSResolver(DNSResolver* resolver) { g_dns_resolver = resolver; } -void grpc_resolved_addresses_destroy(grpc_resolved_addresses* addresses) { - if (addresses != nullptr) { - gpr_free(addresses->addrs); - } - gpr_free(addresses); -} +DNSResolver* GetDNSResolver() { return g_dns_resolver; } -grpc_error_handle grpc_blocking_resolve_address( - const char* name, const char* default_port, - grpc_resolved_addresses** addresses) { - return grpc_resolve_address_impl->blocking_resolve_address(name, default_port, - addresses); -} +} // namespace grpc_core diff --git a/src/core/lib/iomgr/resolve_address.h b/src/core/lib/iomgr/resolve_address.h index b9e25b0f77e..6fbf572c9c4 100644 --- a/src/core/lib/iomgr/resolve_address.h +++ b/src/core/lib/iomgr/resolve_address.h @@ -23,6 +23,9 @@ #include +#include "absl/status/statusor.h" + +#include "src/core/lib/gprpp/orphanable.h" #include "src/core/lib/iomgr/port.h" #ifdef GRPC_WINSOCK_SOCKET @@ -41,43 +44,56 @@ struct grpc_resolved_address { char addr[GRPC_MAX_SOCKADDR_SIZE]; socklen_t len; }; -struct grpc_resolved_addresses { - size_t naddrs; - grpc_resolved_address* addrs; -}; namespace grpc_core { extern const char* kDefaultSecurePort; constexpr int kDefaultSecurePortInt = 443; -} // namespace grpc_core -typedef struct grpc_address_resolver_vtable { - void (*resolve_address)(const char* addr, const char* default_port, - grpc_pollset_set* interested_parties, - grpc_closure* on_done, - grpc_resolved_addresses** addresses); - grpc_error_handle (*blocking_resolve_address)( - const char* name, const char* default_port, - grpc_resolved_addresses** addresses); -} grpc_address_resolver_vtable; - -void grpc_set_resolver_impl(grpc_address_resolver_vtable* vtable); - -/* Asynchronously resolve addr. Use default_port if a port isn't designated - in addr, otherwise use the port in addr. */ -/* TODO(apolcyn): add a timeout here */ -void grpc_resolve_address(const char* addr, const char* default_port, - grpc_pollset_set* interested_parties, - grpc_closure* on_done, - grpc_resolved_addresses** addresses); - -/* Destroy resolved addresses */ -void grpc_resolved_addresses_destroy(grpc_resolved_addresses* addresses); - -/* Resolve addr in a blocking fashion. On success, - result must be freed with grpc_resolved_addresses_destroy. */ -grpc_error_handle grpc_blocking_resolve_address( - const char* name, const char* default_port, - grpc_resolved_addresses** addresses); +// A singleton class used for async and blocking DNS resolution +class DNSResolver { + public: + // Tracks a single asynchronous DNS resolution attempt. The DNS + // resolution should be arranged to be cancelled as soon as possible + // when Orphan is called. + class Request : public InternallyRefCounted { + public: + // Begins async DNS resolution + virtual void Start() = 0; + }; + + virtual ~DNSResolver() {} + + // Asynchronously resolve name. Use \a default_port if a port isn't designated + // in \a name, otherwise use the port in \a name. On completion, \a on_done is + // invoked with the result. + // + // Note for implementations: calls may acquire locks in \a on_done which + // were previously held while calling Request::Start(). Therefore, + // implementations must not invoke \a on_done inline from the call to + // Request::Start(). The DNSCallbackExecCtxScheduler utility may help address + // this. + virtual OrphanablePtr ResolveName( + absl::string_view name, absl::string_view default_port, + grpc_pollset_set* interested_parties, + std::function>)> + on_done) GRPC_MUST_USE_RESULT = 0; + + // Resolve name in a blocking fashion. Use \a default_port if a port isn't + // designated in \a name, otherwise use the port in \a name. + virtual absl::StatusOr> + ResolveNameBlocking(absl::string_view name, + absl::string_view default_port) = 0; +}; + +// Override the active DNS resolver which should be used for all DNS +// resolution in gRPC. Note this should only be used during library +// initialization or within tests. +void SetDNSResolver(DNSResolver* resolver); + +// Get the singleton DNS resolver instance which should be used for all +// DNS resolution in gRPC. +DNSResolver* GetDNSResolver(); + +} // namespace grpc_core #endif /* GRPC_CORE_LIB_IOMGR_RESOLVE_ADDRESS_H */ diff --git a/src/core/lib/iomgr/resolve_address_custom.cc b/src/core/lib/iomgr/resolve_address_custom.cc index 6d8c599cef0..dedd853d7a5 100644 --- a/src/core/lib/iomgr/resolve_address_custom.cc +++ b/src/core/lib/iomgr/resolve_address_custom.cc @@ -1,20 +1,18 @@ -/* - * - * Copyright 2018 gRPC authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ +// +// Copyright 2018 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// #include @@ -22,6 +20,7 @@ #include +#include #include #include "absl/strings/str_format.h" @@ -35,135 +34,158 @@ #include "src/core/lib/gprpp/host_port.h" #include "src/core/lib/iomgr/iomgr_custom.h" #include "src/core/lib/iomgr/port.h" +#include "src/core/lib/iomgr/resolve_address_impl.h" +#include "src/core/lib/transport/error_utils.h" -struct grpc_custom_resolver { - grpc_closure* on_done = nullptr; - grpc_resolved_addresses** addresses = nullptr; - std::string host; - std::string port; -}; - -static grpc_custom_resolver_vtable* resolve_address_vtable = nullptr; - -static int retry_named_port_failure(grpc_custom_resolver* r, - grpc_resolved_addresses** res) { - // This loop is copied from resolve_address_posix.c - const char* svc[][2] = {{"http", "80"}, {"https", "443"}}; - for (size_t i = 0; i < GPR_ARRAY_SIZE(svc); i++) { - if (r->port == svc[i][0]) { - r->port = svc[i][1]; - if (res) { - grpc_error_handle error = resolve_address_vtable->resolve( - r->host.c_str(), r->port.c_str(), res); - if (error != GRPC_ERROR_NONE) { - GRPC_ERROR_UNREF(error); - return 0; - } - } else { - resolve_address_vtable->resolve_async(r, r->host.c_str(), - r->port.c_str()); - } - return 1; - } +void grpc_resolved_addresses_destroy(grpc_resolved_addresses* addresses) { + if (addresses != nullptr) { + gpr_free(addresses->addrs); } - return 0; + gpr_free(addresses); } -void grpc_custom_resolve_callback(grpc_custom_resolver* r, +void grpc_custom_resolve_callback(grpc_custom_resolver* resolver, grpc_resolved_addresses* result, grpc_error_handle error) { GRPC_CUSTOM_IOMGR_ASSERT_SAME_THREAD(); grpc_core::ApplicationCallbackExecCtx callback_exec_ctx; grpc_core::ExecCtx exec_ctx; - if (error == GRPC_ERROR_NONE) { - *r->addresses = result; - } else if (retry_named_port_failure(r, nullptr)) { - return; - } - if (r->on_done) { - grpc_core::ExecCtx::Run(DEBUG_LOCATION, r->on_done, error); + grpc_core::CustomDNSResolver::Request* request = + grpc_core::CustomDNSResolver::Request::FromC(resolver); + if (error != GRPC_ERROR_NONE) { + request->ResolveCallback(grpc_error_to_absl_status(error)); + } else { + std::vector addresses; + for (size_t i = 0; i < result->naddrs; i++) { + addresses.push_back(result->addrs[i]); + } + request->ResolveCallback(std::move(addresses)); + grpc_resolved_addresses_destroy(result); } - delete r; + GRPC_ERROR_UNREF(error); } -static grpc_error_handle try_split_host_port(const char* name, - const char* default_port, - std::string* host, - std::string* port) { - /* parse name, splitting it into host and port parts */ - grpc_core::SplitHostPort(name, host, port); +namespace grpc_core { + +namespace { + +absl::Status TrySplitHostPort(absl::string_view name, + absl::string_view default_port, std::string* host, + std::string* port) { + // parse name, splitting it into host and port parts + SplitHostPort(name, host, port); if (host->empty()) { - return GRPC_ERROR_CREATE_FROM_CPP_STRING( + return absl::UnknownError( absl::StrFormat("unparseable host:port: '%s'", name)); } if (port->empty()) { // TODO(murgatroid99): add tests for this case if (default_port == nullptr) { - return GRPC_ERROR_CREATE_FROM_CPP_STRING( - absl::StrFormat("no port in name '%s'", name)); + return absl::UnknownError(absl::StrFormat("no port in name '%s'", name)); + } + *port = std::string(default_port); + } + return absl::OkStatus(); +} + +absl::StatusOr NamedPortToNumeric(absl::string_view named_port) { + if (named_port == "http") { + return "80"; + } else if (named_port == "https") { + return "443"; + } else { + return absl::UnknownError(absl::StrCat("unknown named port: ", named_port)); + } +} + +} // namespace + +void CustomDNSResolver::Request::ResolveCallback( + absl::StatusOr> result) { + if (!result.ok()) { + auto numeric_port_or = NamedPortToNumeric(port_); + if (numeric_port_or.ok()) { + port_ = *numeric_port_or; + resolve_address_vtable_->resolve_async(c_ptr(), host_.c_str(), + port_.c_str()); + // keep holding ref for active resolution + return; } - *port = default_port; } - return GRPC_ERROR_NONE; + // since we can't guarantee that we're not being called inline from + // Start(), run the callback on the ExecCtx. + new DNSCallbackExecCtxScheduler(std::move(on_done_), std::move(result)); + Unref(); } -static grpc_error_handle blocking_resolve_address_impl( - const char* name, const char* default_port, - grpc_resolved_addresses** addresses) { +namespace { +CustomDNSResolver* g_custom_dns_resolver; +} // namespace + +// Creates the global custom resolver with the specified vtable. +void CustomDNSResolver::Create(grpc_custom_resolver_vtable* vtable) { + if (g_custom_dns_resolver != nullptr) return; + g_custom_dns_resolver = new CustomDNSResolver(vtable); +} + +// Gets the singleton instance. +CustomDNSResolver* CustomDNSResolver::Get() { return g_custom_dns_resolver; } + +absl::StatusOr> +CustomDNSResolver::ResolveNameBlocking(absl::string_view name, + absl::string_view default_port) { GRPC_CUSTOM_IOMGR_ASSERT_SAME_THREAD(); - grpc_custom_resolver resolver; - grpc_error_handle err = - try_split_host_port(name, default_port, &resolver.host, &resolver.port); - if (err != GRPC_ERROR_NONE) { - return err; + std::string host; + std::string port; + absl::Status parse_status = + TrySplitHostPort(name, default_port, &host, &port); + if (!parse_status.ok()) { + return parse_status; } - /* Call getaddrinfo */ + // Call getaddrinfo + ExecCtx* curr = ExecCtx::Get(); + ExecCtx::Set(nullptr); grpc_resolved_addresses* addrs; - grpc_core::ExecCtx* curr = grpc_core::ExecCtx::Get(); - grpc_core::ExecCtx::Set(nullptr); - err = resolve_address_vtable->resolve(resolver.host.c_str(), - resolver.port.c_str(), &addrs); + grpc_error_handle err = + resolve_address_vtable_->resolve(host.c_str(), port.c_str(), &addrs); if (err != GRPC_ERROR_NONE) { - if (retry_named_port_failure(&resolver, &addrs)) { + auto numeric_port_or = NamedPortToNumeric(port); + if (numeric_port_or.ok()) { + port = *numeric_port_or; GRPC_ERROR_UNREF(err); - err = GRPC_ERROR_NONE; + err = + resolve_address_vtable_->resolve(host.c_str(), port.c_str(), &addrs); } } - grpc_core::ExecCtx::Set(curr); + ExecCtx::Set(curr); if (err == GRPC_ERROR_NONE) { - *addresses = addrs; + GPR_ASSERT(addrs != nullptr); + std::vector result; + for (size_t i = 0; i < addrs->naddrs; i++) { + result.push_back(addrs->addrs[i]); + } + grpc_resolved_addresses_destroy(addrs); + return result; } - return err; + auto error_result = grpc_error_to_absl_status(err); + GRPC_ERROR_UNREF(err); + return error_result; } -static void resolve_address_impl(const char* name, const char* default_port, - grpc_pollset_set* /*interested_parties*/, - grpc_closure* on_done, - grpc_resolved_addresses** addrs) { +void CustomDNSResolver::Request::Start() { GRPC_CUSTOM_IOMGR_ASSERT_SAME_THREAD(); - std::string host; - std::string port; - grpc_error_handle err = try_split_host_port(name, default_port, &host, &port); - if (err != GRPC_ERROR_NONE) { - grpc_core::ExecCtx::Run(DEBUG_LOCATION, on_done, err); + absl::Status parse_status = + TrySplitHostPort(name_, default_port_, &host_, &port_); + if (!parse_status.ok()) { + new DNSCallbackExecCtxScheduler(std::move(on_done_), + std::move(parse_status)); return; } - grpc_custom_resolver* r = new grpc_custom_resolver(); - r->on_done = on_done; - r->addresses = addrs; - r->host = std::move(host); - r->port = std::move(port); - - /* Call getaddrinfo */ - resolve_address_vtable->resolve_async(r, r->host.c_str(), r->port.c_str()); + // Call getaddrinfo + Ref().release(); // ref held by resolution + resolve_address_vtable_->resolve_async(c_ptr(), host_.c_str(), port_.c_str()); } -static grpc_address_resolver_vtable custom_resolver_vtable = { - resolve_address_impl, blocking_resolve_address_impl}; - -void grpc_custom_resolver_init(grpc_custom_resolver_vtable* impl) { - resolve_address_vtable = impl; - grpc_set_resolver_impl(&custom_resolver_vtable); -} +} // namespace grpc_core diff --git a/src/core/lib/iomgr/resolve_address_custom.h b/src/core/lib/iomgr/resolve_address_custom.h index 4063dcf155a..54586265fe3 100644 --- a/src/core/lib/iomgr/resolve_address_custom.h +++ b/src/core/lib/iomgr/resolve_address_custom.h @@ -1,30 +1,43 @@ -/* - * - * Copyright 2018 gRPC authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ +// +// Copyright 2018 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// #ifndef GRPC_CORE_LIB_IOMGR_RESOLVE_ADDRESS_CUSTOM_H #define GRPC_CORE_LIB_IOMGR_RESOLVE_ADDRESS_CUSTOM_H #include +#include + +#include "src/core/lib/gprpp/cpp_impl_of.h" #include "src/core/lib/iomgr/port.h" #include "src/core/lib/iomgr/resolve_address.h" #include "src/core/lib/iomgr/sockaddr.h" +// User-configured custom DNS resolution APIs + +// TODO(apolcyn): This type could be deleted as a part of converting +// this grpc_custom_resolver API to C++. +struct grpc_resolved_addresses { + size_t naddrs; + grpc_resolved_address* addrs; +}; + +// Destroy resolved addresses +void grpc_resolved_addresses_destroy(grpc_resolved_addresses* addresses); + typedef struct grpc_custom_resolver grpc_custom_resolver; typedef struct grpc_custom_resolver_vtable { @@ -34,11 +47,80 @@ typedef struct grpc_custom_resolver_vtable { const char* port); } grpc_custom_resolver_vtable; +// TODO(apolcyn): as a part of converting this API to C++, +// callers of \a grpc_custom_resolve_callback could instead just invoke +// CustomDNSResolver::Request::ResolveCallback directly. void grpc_custom_resolve_callback(grpc_custom_resolver* resolver, grpc_resolved_addresses* result, grpc_error_handle error); -/* Internal APIs */ -void grpc_custom_resolver_init(grpc_custom_resolver_vtable* impl); +// Internal APIs + +namespace grpc_core { + +class CustomDNSResolver : public DNSResolver { + public: + class Request : public DNSResolver::Request, + public CppImplOf { + public: + Request( + absl::string_view name, absl::string_view default_port, + std::function>)> + on_done, + const grpc_custom_resolver_vtable* resolve_address_vtable) + : name_(name), + default_port_(default_port), + on_done_(std::move(on_done)), + resolve_address_vtable_(resolve_address_vtable) {} + + // Starts the resolution + void Start() override; + + // This is a no-op for the native resolver. Note + // that no I/O polling is required for the resolution to finish. + void Orphan() override { Unref(); } + + // Continues async resolution with the results passed first in to + // grpc_custom_resolve_callback. + void ResolveCallback( + absl::StatusOr> result); + + private: + const std::string name_; + const std::string default_port_; + std::string host_; + std::string port_; + std::function>)> + on_done_; + // user-defined DNS methods + const grpc_custom_resolver_vtable* resolve_address_vtable_; + }; + + // Creates the global custom resolver with the specified vtable. + static void Create(grpc_custom_resolver_vtable* vtable); + + // Gets the singleton instance. + static CustomDNSResolver* Get(); + + explicit CustomDNSResolver(const grpc_custom_resolver_vtable* vtable) + : resolve_address_vtable_(vtable) {} + + OrphanablePtr ResolveName( + absl::string_view name, absl::string_view default_port, + grpc_pollset_set* /* interested_parties */, + std::function>)> + on_done) override { + return MakeOrphanable(name, default_port, std::move(on_done), + resolve_address_vtable_); + } + + absl::StatusOr> ResolveNameBlocking( + absl::string_view name, absl::string_view default_port) override; + + private: + // user-defined DNS methods + const grpc_custom_resolver_vtable* resolve_address_vtable_; +}; +} // namespace grpc_core #endif /* GRPC_CORE_LIB_IOMGR_RESOLVE_ADDRESS_CUSTOM_H */ diff --git a/src/core/lib/iomgr/resolve_address_impl.h b/src/core/lib/iomgr/resolve_address_impl.h new file mode 100644 index 00000000000..0f912182758 --- /dev/null +++ b/src/core/lib/iomgr/resolve_address_impl.h @@ -0,0 +1,59 @@ +// +// Copyright 2015 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#ifndef GRPC_CORE_LIB_IOMGR_RESOLVE_ADDRESS_IMPL_H +#define GRPC_CORE_LIB_IOMGR_RESOLVE_ADDRESS_IMPL_H + +#include + +#include + +#include "src/core/lib/iomgr/port.h" +#include "src/core/lib/iomgr/resolve_address.h" + +namespace grpc_core { + +// A fire and forget class used by DNSResolver::Request implementations to +// schedule DNS resolution callbacks on the ExecCtx, which is frequently +// necessary to avoid lock inversion related problems. +class DNSCallbackExecCtxScheduler { + public: + DNSCallbackExecCtxScheduler( + std::function>)> + on_done, + absl::StatusOr> param) + : on_done_(std::move(on_done)), param_(std::move(param)) { + GRPC_CLOSURE_INIT(&closure_, RunCallback, this, grpc_schedule_on_exec_ctx); + ExecCtx::Run(DEBUG_LOCATION, &closure_, GRPC_ERROR_NONE); + } + + private: + static void RunCallback(void* arg, grpc_error_handle /* error */) { + DNSCallbackExecCtxScheduler* self = + static_cast(arg); + self->on_done_(std::move(self->param_)); + delete self; + } + + const std::function>)> + on_done_; + absl::StatusOr> param_; + grpc_closure closure_; +}; + +} // namespace grpc_core + +#endif /* GRPC_CORE_LIB_IOMGR_RESOLVE_ADDRESS_IMPL_H */ diff --git a/src/core/lib/iomgr/resolve_address_posix.cc b/src/core/lib/iomgr/resolve_address_posix.cc index 1427cbda965..41e54ce58ab 100644 --- a/src/core/lib/iomgr/resolve_address_posix.cc +++ b/src/core/lib/iomgr/resolve_address_posix.cc @@ -37,30 +37,89 @@ #include "src/core/lib/iomgr/executor.h" #include "src/core/lib/iomgr/iomgr_internal.h" #include "src/core/lib/iomgr/resolve_address.h" +#include "src/core/lib/iomgr/resolve_address_posix.h" #include "src/core/lib/iomgr/sockaddr.h" #include "src/core/lib/iomgr/unix_sockets_posix.h" +#include "src/core/lib/transport/error_utils.h" + +namespace grpc_core { +namespace { + +class NativeDNSRequest : public DNSResolver::Request { + public: + NativeDNSRequest( + absl::string_view name, absl::string_view default_port, + std::function>)> + on_done) + : name_(name), default_port_(default_port), on_done_(std::move(on_done)) { + GRPC_CLOSURE_INIT(&request_closure_, DoRequestThread, this, nullptr); + } + + // Starts the resolution + void Start() override { + Ref().release(); // ref held by callback + Executor::Run(&request_closure_, GRPC_ERROR_NONE, ExecutorType::RESOLVER); + } + + // This is a no-op for the native resolver. Note + // that no I/O polling is required for the resolution to finish. + void Orphan() override { Unref(); } + + private: + // Callback to be passed to grpc Executor to asynch-ify + // ResolveNameBlocking + static void DoRequestThread(void* rp, grpc_error_handle /*error*/) { + NativeDNSRequest* r = static_cast(rp); + auto result = + GetDNSResolver()->ResolveNameBlocking(r->name_, r->default_port_); + // running inline is safe since we've already been scheduled on the executor + r->on_done_(std::move(result)); + r->Unref(); + } + + const std::string name_; + const std::string default_port_; + const std::function>)> + on_done_; + grpc_closure request_closure_; +}; -static grpc_error_handle posix_blocking_resolve_address( - const char* name, const char* default_port, - grpc_resolved_addresses** addresses) { - grpc_core::ExecCtx exec_ctx; +} // namespace + +NativeDNSResolver* NativeDNSResolver::GetOrCreate() { + static NativeDNSResolver* instance = new NativeDNSResolver(); + return instance; +} + +OrphanablePtr NativeDNSResolver::ResolveName( + absl::string_view name, absl::string_view default_port, + grpc_pollset_set* /* interested_parties */, + std::function>)> + on_done) { + return MakeOrphanable(name, default_port, + std::move(on_done)); +} + +absl::StatusOr> +NativeDNSResolver::ResolveNameBlocking(absl::string_view name, + absl::string_view default_port) { + ExecCtx exec_ctx; struct addrinfo hints; struct addrinfo *result = nullptr, *resp; int s; size_t i; grpc_error_handle err; - + std::vector addresses; std::string host; std::string port; - /* parse name, splitting it into host and port parts */ - grpc_core::SplitHostPort(name, &host, &port); + // parse name, splitting it into host and port parts + SplitHostPort(name, &host, &port); if (host.empty()) { err = grpc_error_set_str( GRPC_ERROR_CREATE_FROM_STATIC_STRING("unparseable host:port"), GRPC_ERROR_STR_TARGET_ADDRESS, name); goto done; } - if (port.empty()) { if (default_port == nullptr) { err = grpc_error_set_str( @@ -68,21 +127,18 @@ static grpc_error_handle posix_blocking_resolve_address( GRPC_ERROR_STR_TARGET_ADDRESS, name); goto done; } - port = default_port; + port = std::string(default_port); } - - /* Call getaddrinfo */ + // Call getaddrinfo memset(&hints, 0, sizeof(hints)); hints.ai_family = AF_UNSPEC; /* ipv4 or ipv6 */ hints.ai_socktype = SOCK_STREAM; /* stream socket */ hints.ai_flags = AI_PASSIVE; /* for wildcard IP address */ - GRPC_SCHEDULING_START_BLOCKING_REGION; s = getaddrinfo(host.c_str(), port.c_str(), &hints, &result); GRPC_SCHEDULING_END_BLOCKING_REGION; - if (s != 0) { - /* Retry if well-known service name is recognized */ + // Retry if well-known service name is recognized const char* svc[][2] = {{"http", "80"}, {"https", "443"}}; for (i = 0; i < GPR_ARRAY_SIZE(svc); i++) { if (port == svc[i][0]) { @@ -93,7 +149,6 @@ static grpc_error_handle posix_blocking_resolve_address( } } } - if (s != 0) { err = grpc_error_set_str( grpc_error_set_str( @@ -106,65 +161,26 @@ static grpc_error_handle posix_blocking_resolve_address( GRPC_ERROR_STR_TARGET_ADDRESS, name); goto done; } - - /* Success path: set addrs non-NULL, fill it in */ - *addresses = static_cast( - gpr_malloc(sizeof(grpc_resolved_addresses))); - (*addresses)->naddrs = 0; - for (resp = result; resp != nullptr; resp = resp->ai_next) { - (*addresses)->naddrs++; - } - (*addresses)->addrs = static_cast( - gpr_malloc(sizeof(grpc_resolved_address) * (*addresses)->naddrs)); - i = 0; + // Success path: fill in addrs for (resp = result; resp != nullptr; resp = resp->ai_next) { - memcpy(&(*addresses)->addrs[i].addr, resp->ai_addr, resp->ai_addrlen); - (*addresses)->addrs[i].len = resp->ai_addrlen; - i++; + grpc_resolved_address addr; + memcpy(&addr.addr, resp->ai_addr, resp->ai_addrlen); + addr.len = resp->ai_addrlen; + addresses.push_back(addr); } err = GRPC_ERROR_NONE; - done: if (result) { freeaddrinfo(result); } - return err; -} - -struct request { - char* name; - char* default_port; - grpc_closure* on_done; - grpc_resolved_addresses** addrs_out; - grpc_closure request_closure; - void* arg; -}; -/* Callback to be passed to grpc Executor to asynch-ify - * grpc_blocking_resolve_address */ -static void do_request_thread(void* rp, grpc_error_handle /*error*/) { - request* r = static_cast(rp); - grpc_core::ExecCtx::Run( - DEBUG_LOCATION, r->on_done, - grpc_blocking_resolve_address(r->name, r->default_port, r->addrs_out)); - gpr_free(r->name); - gpr_free(r->default_port); - gpr_free(r); + if (err == GRPC_ERROR_NONE) { + return addresses; + } + auto error_result = grpc_error_to_absl_status(err); + GRPC_ERROR_UNREF(err); + return error_result; } -static void posix_resolve_address(const char* name, const char* default_port, - grpc_pollset_set* /*interested_parties*/, - grpc_closure* on_done, - grpc_resolved_addresses** addrs) { - request* r = static_cast(gpr_malloc(sizeof(request))); - GRPC_CLOSURE_INIT(&r->request_closure, do_request_thread, r, nullptr); - r->name = gpr_strdup(name); - r->default_port = gpr_strdup(default_port); - r->on_done = on_done; - r->addrs_out = addrs; - grpc_core::Executor::Run(&r->request_closure, GRPC_ERROR_NONE, - grpc_core::ExecutorType::RESOLVER); -} +} // namespace grpc_core -grpc_address_resolver_vtable grpc_posix_resolver_vtable = { - posix_resolve_address, posix_blocking_resolve_address}; #endif diff --git a/src/core/lib/iomgr/resolve_address_posix.h b/src/core/lib/iomgr/resolve_address_posix.h new file mode 100644 index 00000000000..6362d02d43c --- /dev/null +++ b/src/core/lib/iomgr/resolve_address_posix.h @@ -0,0 +1,47 @@ +// +// Copyright 2015 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#ifndef GRPC_CORE_LIB_IOMGR_RESOLVE_ADDRESS_POSIX_H +#define GRPC_CORE_LIB_IOMGR_RESOLVE_ADDRESS_POSIX_H + +#include + +#include + +#include "src/core/lib/iomgr/port.h" +#include "src/core/lib/iomgr/resolve_address.h" + +namespace grpc_core { + +// A DNS resolver which uses the native platform's getaddrinfo API. +class NativeDNSResolver : public DNSResolver { + public: + // Gets the singleton instance, creating it first if it doesn't exist + static NativeDNSResolver* GetOrCreate(); + + OrphanablePtr ResolveName( + absl::string_view name, absl::string_view default_port, + grpc_pollset_set* interested_parties, + std::function>)> + on_done) override; + + absl::StatusOr> ResolveNameBlocking( + absl::string_view name, absl::string_view default_port) override; +}; + +} // namespace grpc_core + +#endif // GRPC_CORE_LIB_IOMGR_RESOLVE_ADDRESS_POSIX_H diff --git a/src/core/lib/iomgr/resolve_address_windows.cc b/src/core/lib/iomgr/resolve_address_windows.cc index 8cea3c1ef97..90592312cfd 100644 --- a/src/core/lib/iomgr/resolve_address_windows.cc +++ b/src/core/lib/iomgr/resolve_address_windows.cc @@ -1,20 +1,18 @@ -/* - * - * Copyright 2015 gRPC authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ +// +// Copyright 2015 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// #include @@ -43,29 +41,83 @@ #include "src/core/lib/iomgr/executor.h" #include "src/core/lib/iomgr/iomgr_internal.h" #include "src/core/lib/iomgr/resolve_address.h" +#include "src/core/lib/iomgr/resolve_address_windows.h" #include "src/core/lib/iomgr/sockaddr.h" +#include "src/core/lib/transport/error_utils.h" + +namespace grpc_core { +namespace { + +class NativeDNSRequest : public DNSResolver::Request { + public: + NativeDNSRequest( + absl::string_view name, absl::string_view default_port, + std::function>)> + on_done) + : name_(name), default_port_(default_port), on_done_(std::move(on_done)) { + GRPC_CLOSURE_INIT(&request_closure_, DoRequestThread, this, nullptr); + } + + // Starts the resolution + void Start() override { + Ref().release(); // ref held by callback + Executor::Run(&request_closure_, GRPC_ERROR_NONE, ExecutorType::RESOLVER); + } + + // This is a no-op for the native resolver. Note + // that no I/O polling is required for the resolution to finish. + void Orphan() override { Unref(); } + + private: + // Callback to be passed to grpc Executor to asynch-ify + // ResolveNameBlocking + static void DoRequestThread(void* rp, grpc_error_handle /*error*/) { + NativeDNSRequest* r = static_cast(rp); + auto result = + GetDNSResolver()->ResolveNameBlocking(r->name_, r->default_port_); + // running inline is safe since we've already been scheduled on the executor + r->on_done_(std::move(result)); + r->Unref(); + } -struct request { - char* name; - char* default_port; - grpc_closure request_closure; - grpc_closure* on_done; - grpc_resolved_addresses** addresses; + const std::string name_; + const std::string default_port_; + const std::function>)> + on_done_; + grpc_closure request_closure_; }; -static grpc_error_handle windows_blocking_resolve_address( - const char* name, const char* default_port, - grpc_resolved_addresses** addresses) { - grpc_core::ExecCtx exec_ctx; + +} // namespace + +NativeDNSResolver* NativeDNSResolver::GetOrCreate() { + static NativeDNSResolver* instance = new NativeDNSResolver(); + return instance; +} + +OrphanablePtr NativeDNSResolver::ResolveName( + absl::string_view name, absl::string_view default_port, + grpc_pollset_set* /* interested_parties */, + std::function>)> + on_done) { + return MakeOrphanable(name, default_port, + std::move(on_done)); +} + +absl::StatusOr> +NativeDNSResolver::ResolveNameBlocking(absl::string_view name, + absl::string_view default_port) { + ExecCtx exec_ctx; struct addrinfo hints; struct addrinfo *result = NULL, *resp; int s; size_t i; grpc_error_handle error = GRPC_ERROR_NONE; + std::vector addresses; - /* parse name, splitting it into host and port parts */ + // parse name, splitting it into host and port parts std::string host; std::string port; - grpc_core::SplitHostPort(name, &host, &port); + SplitHostPort(name, &host, &port); if (host.empty()) { error = GRPC_ERROR_CREATE_FROM_CPP_STRING( absl::StrFormat("unparseable host:port: '%s'", name)); @@ -77,10 +129,10 @@ static grpc_error_handle windows_blocking_resolve_address( absl::StrFormat("no port in name '%s'", name)); goto done; } - port = default_port; + port = std::string(default_port); } - /* Call getaddrinfo */ + // Call getaddrinfo memset(&hints, 0, sizeof(hints)); hints.ai_family = AF_UNSPEC; /* ipv4 or ipv6 */ hints.ai_socktype = SOCK_STREAM; /* stream socket */ @@ -94,59 +146,26 @@ static grpc_error_handle windows_blocking_resolve_address( goto done; } - /* Success path: set addrs non-NULL, fill it in */ - (*addresses) = - (grpc_resolved_addresses*)gpr_malloc(sizeof(grpc_resolved_addresses)); - (*addresses)->naddrs = 0; - for (resp = result; resp != NULL; resp = resp->ai_next) { - (*addresses)->naddrs++; - } - (*addresses)->addrs = (grpc_resolved_address*)gpr_malloc( - sizeof(grpc_resolved_address) * (*addresses)->naddrs); - i = 0; + // Success path: set addrs non-NULL, fill it in for (resp = result; resp != NULL; resp = resp->ai_next) { - memcpy(&(*addresses)->addrs[i].addr, resp->ai_addr, resp->ai_addrlen); - (*addresses)->addrs[i].len = resp->ai_addrlen; - i++; + grpc_resolved_address addr; + memcpy(&addr.addr, resp->ai_addr, resp->ai_addrlen); + addr.len = resp->ai_addrlen; + addresses.push_back(addr); } done: if (result) { freeaddrinfo(result); } - return error; -} - -/* Callback to be passed to grpc_executor to asynch-ify - * grpc_blocking_resolve_address */ -static void do_request_thread(void* rp, grpc_error_handle error) { - request* r = (request*)rp; if (error == GRPC_ERROR_NONE) { - error = - grpc_blocking_resolve_address(r->name, r->default_port, r->addresses); - } else { - GRPC_ERROR_REF(error); + return addresses; } - grpc_core::ExecCtx::Run(DEBUG_LOCATION, r->on_done, error); - gpr_free(r->name); - gpr_free(r->default_port); - gpr_free(r); + auto error_result = grpc_error_to_absl_status(error); + GRPC_ERROR_UNREF(error); + return error_result; } -static void windows_resolve_address(const char* name, const char* default_port, - grpc_pollset_set* interested_parties, - grpc_closure* on_done, - grpc_resolved_addresses** addresses) { - request* r = (request*)gpr_malloc(sizeof(request)); - GRPC_CLOSURE_INIT(&r->request_closure, do_request_thread, r, nullptr); - r->name = gpr_strdup(name); - r->default_port = gpr_strdup(default_port); - r->on_done = on_done; - r->addresses = addresses; - grpc_core::Executor::Run(&r->request_closure, GRPC_ERROR_NONE, - grpc_core::ExecutorType::RESOLVER); -} +} // namespace grpc_core -grpc_address_resolver_vtable grpc_windows_resolver_vtable = { - windows_resolve_address, windows_blocking_resolve_address}; #endif diff --git a/src/core/lib/iomgr/resolve_address_windows.h b/src/core/lib/iomgr/resolve_address_windows.h new file mode 100644 index 00000000000..403c4ff0d13 --- /dev/null +++ b/src/core/lib/iomgr/resolve_address_windows.h @@ -0,0 +1,47 @@ +// +// Copyright 2015 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#ifndef GRPC_CORE_LIB_IOMGR_RESOLVE_ADDRESS_WINDOWS_H +#define GRPC_CORE_LIB_IOMGR_RESOLVE_ADDRESS_WINDOWS_H + +#include + +#include + +#include "src/core/lib/iomgr/port.h" +#include "src/core/lib/iomgr/resolve_address.h" + +namespace grpc_core { + +// A DNS resolver which uses the native platform's getaddrinfo API. +class NativeDNSResolver : public DNSResolver { + public: + // Gets the singleton instance, creating it first if it doesn't exist + static NativeDNSResolver* GetOrCreate(); + + OrphanablePtr ResolveName( + absl::string_view name, absl::string_view default_port, + grpc_pollset_set* interested_parties, + std::function>)> + on_done) override; + + absl::StatusOr> ResolveNameBlocking( + absl::string_view name, absl::string_view default_port) override; +}; + +} // namespace grpc_core + +#endif // GRPC_CORE_LIB_IOMGR_RESOLVE_ADDRESS_WINDOWS_H diff --git a/src/core/lib/iomgr/unix_sockets_posix.cc b/src/core/lib/iomgr/unix_sockets_posix.cc index dada84a86bd..bf7e5e9788b 100644 --- a/src/core/lib/iomgr/unix_sockets_posix.cc +++ b/src/core/lib/iomgr/unix_sockets_posix.cc @@ -35,29 +35,35 @@ #include "src/core/lib/gpr/useful.h" #include "src/core/lib/iomgr/sockaddr.h" #include "src/core/lib/iomgr/unix_sockets_posix.h" +#include "src/core/lib/transport/error_utils.h" void grpc_create_socketpair_if_unix(int sv[2]) { GPR_ASSERT(socketpair(AF_UNIX, SOCK_STREAM, 0, sv) == 0); } -grpc_error_handle grpc_resolve_unix_domain_address( - absl::string_view name, grpc_resolved_addresses** addresses) { - *addresses = static_cast( - gpr_malloc(sizeof(grpc_resolved_addresses))); - (*addresses)->naddrs = 1; - (*addresses)->addrs = static_cast( - gpr_malloc(sizeof(grpc_resolved_address))); - return grpc_core::UnixSockaddrPopulate(name, (*addresses)->addrs); +absl::StatusOr> +grpc_resolve_unix_domain_address(absl::string_view name) { + grpc_resolved_address addr; + grpc_error_handle error = grpc_core::UnixSockaddrPopulate(name, &addr); + if (error == GRPC_ERROR_NONE) { + return std::vector({addr}); + } + auto result = grpc_error_to_absl_status(error); + GRPC_ERROR_UNREF(error); + return result; } -grpc_error_handle grpc_resolve_unix_abstract_domain_address( - const absl::string_view name, grpc_resolved_addresses** addresses) { - *addresses = static_cast( - gpr_malloc(sizeof(grpc_resolved_addresses))); - (*addresses)->naddrs = 1; - (*addresses)->addrs = static_cast( - gpr_malloc(sizeof(grpc_resolved_address))); - return grpc_core::UnixAbstractSockaddrPopulate(name, (*addresses)->addrs); +absl::StatusOr> +grpc_resolve_unix_abstract_domain_address(const absl::string_view name) { + grpc_resolved_address addr; + grpc_error_handle error = + grpc_core::UnixAbstractSockaddrPopulate(name, &addr); + if (error == GRPC_ERROR_NONE) { + return std::vector({addr}); + } + auto result = grpc_error_to_absl_status(error); + GRPC_ERROR_UNREF(error); + return result; } int grpc_is_unix_socket(const grpc_resolved_address* resolved_addr) { diff --git a/src/core/lib/iomgr/unix_sockets_posix.h b/src/core/lib/iomgr/unix_sockets_posix.h index 823dd832742..3af73ff76c4 100644 --- a/src/core/lib/iomgr/unix_sockets_posix.h +++ b/src/core/lib/iomgr/unix_sockets_posix.h @@ -32,11 +32,11 @@ void grpc_create_socketpair_if_unix(int sv[2]); -grpc_error_handle grpc_resolve_unix_domain_address( - absl::string_view name, grpc_resolved_addresses** addresses); +absl::StatusOr> +grpc_resolve_unix_domain_address(absl::string_view name); -grpc_error_handle grpc_resolve_unix_abstract_domain_address( - absl::string_view name, grpc_resolved_addresses** addresses); +absl::StatusOr> +grpc_resolve_unix_abstract_domain_address(absl::string_view name); int grpc_is_unix_socket(const grpc_resolved_address* resolved_addr); diff --git a/src/core/lib/iomgr/unix_sockets_posix_noop.cc b/src/core/lib/iomgr/unix_sockets_posix_noop.cc index 15592e713f9..f4dd2041287 100644 --- a/src/core/lib/iomgr/unix_sockets_posix_noop.cc +++ b/src/core/lib/iomgr/unix_sockets_posix_noop.cc @@ -33,18 +33,14 @@ void grpc_create_socketpair_if_unix(int /* sv */[2]) { GPR_ASSERT(0); } -grpc_error_handle grpc_resolve_unix_domain_address( - absl::string_view /* name */, grpc_resolved_addresses** addresses) { - *addresses = NULL; - return GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "Unix domain sockets are not supported on Windows"); +absl::StatusOr> +grpc_resolve_unix_domain_address(absl::string_view /* name */) { + return absl::UnknownError("Unix domain sockets are not supported on Windows"); } -grpc_error_handle grpc_resolve_unix_abstract_domain_address( - absl::string_view, grpc_resolved_addresses** addresses) { - *addresses = NULL; - return GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "Unix domain sockets are not supported on Windows"); +absl::StatusOr> +grpc_resolve_unix_abstract_domain_address(absl::string_view /* name */) { + return absl::UnknownError("Unix domain sockets are not supported on Windows"); } int grpc_is_unix_socket(const grpc_resolved_address* /* addr */) { diff --git a/test/core/client_channel/resolvers/dns_resolver_connectivity_test.cc b/test/core/client_channel/resolvers/dns_resolver_connectivity_test.cc index 873b34f0317..13e0d436c70 100644 --- a/test/core/client_channel/resolvers/dns_resolver_connectivity_test.cc +++ b/test/core/client_channel/resolvers/dns_resolver_connectivity_test.cc @@ -27,6 +27,7 @@ #include "src/core/ext/filters/client_channel/server_address.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/iomgr/resolve_address.h" +#include "src/core/lib/iomgr/resolve_address_impl.h" #include "src/core/lib/iomgr/timer.h" #include "src/core/lib/iomgr/work_serializer.h" #include "test/core/util/test_config.h" @@ -35,30 +36,58 @@ static gpr_mu g_mu; static bool g_fail_resolution = true; static std::shared_ptr* g_work_serializer; -static void my_resolve_address(const char* addr, const char* /*default_port*/, - grpc_pollset_set* /*interested_parties*/, - grpc_closure* on_done, - grpc_resolved_addresses** addrs) { - gpr_mu_lock(&g_mu); - GPR_ASSERT(0 == strcmp("test", addr)); - grpc_error_handle error = GRPC_ERROR_NONE; - if (g_fail_resolution) { - g_fail_resolution = false; - gpr_mu_unlock(&g_mu); - error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("Forced Failure"); - } else { - gpr_mu_unlock(&g_mu); - *addrs = static_cast(gpr_malloc(sizeof(**addrs))); - (*addrs)->naddrs = 1; - (*addrs)->addrs = static_cast( - gpr_malloc(sizeof(*(*addrs)->addrs))); - (*addrs)->addrs[0].len = 123; +namespace { + +class TestDNSResolver : public grpc_core::DNSResolver { + public: + class TestDNSRequest : public grpc_core::DNSResolver::Request { + public: + explicit TestDNSRequest( + std::function>)> + on_done) + : on_done_(std::move(on_done)) {} + + void Start() override { + gpr_mu_lock(&g_mu); + if (g_fail_resolution) { + g_fail_resolution = false; + gpr_mu_unlock(&g_mu); + new grpc_core::DNSCallbackExecCtxScheduler( + std::move(on_done_), absl::UnknownError("Forced Failure")); + } else { + gpr_mu_unlock(&g_mu); + std::vector addrs; + grpc_resolved_address phony_resolved_address; + memset(&phony_resolved_address, 0, sizeof(phony_resolved_address)); + addrs.push_back(phony_resolved_address); + new grpc_core::DNSCallbackExecCtxScheduler(std::move(on_done_), addrs); + } + } + + void Orphan() override { Unref(); } + + private: + std::function>)> + on_done_; + }; + + grpc_core::OrphanablePtr ResolveName( + absl::string_view name, absl::string_view /* default_port */, + grpc_pollset_set* /* interested_parties */, + std::function>)> + on_done) override { + GPR_ASSERT("test" == name); + return grpc_core::MakeOrphanable(std::move(on_done)); } - grpc_core::ExecCtx::Run(DEBUG_LOCATION, on_done, error); -} -static grpc_address_resolver_vtable test_resolver = {my_resolve_address, - nullptr}; + absl::StatusOr> ResolveNameBlocking( + absl::string_view /* name */, + absl::string_view /* default_port */) override { + GPR_ASSERT(0); + } +}; + +} // namespace static grpc_ares_request* my_dns_lookup_ares( const char* /*dns_server*/, const char* addr, const char* /*default_port*/, @@ -153,7 +182,7 @@ int main(int argc, char** argv) { gpr_mu_init(&g_mu); auto work_serializer = std::make_shared(); g_work_serializer = &work_serializer; - grpc_set_resolver_impl(&test_resolver); + grpc_core::SetDNSResolver(new TestDNSResolver()); grpc_dns_lookup_ares = my_dns_lookup_ares; grpc_cancel_ares_request = my_cancel_ares_request; diff --git a/test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc b/test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc index 35c6b8a88ea..7c49b4ec02d 100644 --- a/test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc +++ b/test/core/client_channel/resolvers/dns_resolver_cooldown_test.cc @@ -17,6 +17,7 @@ */ #include +#include #include #include @@ -27,14 +28,12 @@ #include "src/core/lib/address_utils/sockaddr_utils.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/gprpp/memory.h" +#include "src/core/lib/iomgr/resolve_address.h" #include "src/core/lib/iomgr/work_serializer.h" #include "test/core/util/test_config.h" constexpr int kMinResolutionPeriodMs = 1000; -extern grpc_address_resolver_vtable* grpc_resolve_address_impl; -static grpc_address_resolver_vtable* default_resolve_address; - static std::shared_ptr* g_work_serializer; static grpc_ares_request* (*g_default_dns_lookup_ares)( @@ -44,8 +43,8 @@ static grpc_ares_request* (*g_default_dns_lookup_ares)( std::unique_ptr* balancer_addresses, char** service_config_json, int query_timeout_ms); -// Counter incremented by test_resolve_address_impl indicating the number of -// times a system-level resolution has happened. +// Counter incremented by TestDNSResolver::ResolveName indicating the +// number of times a system-level resolution has happened. static int g_resolution_count; static struct iomgr_args { @@ -56,44 +55,49 @@ static struct iomgr_args { grpc_pollset_set* pollset_set; } g_iomgr_args; -// Wrapper around default resolve_address in order to count the number of -// times we incur in a system-level name resolution. -static void test_resolve_address_impl(const char* name, - const char* default_port, - grpc_pollset_set* /*interested_parties*/, - grpc_closure* on_done, - grpc_resolved_addresses** addrs) { - default_resolve_address->resolve_address( - name, default_port, g_iomgr_args.pollset_set, on_done, addrs); - ++g_resolution_count; - static grpc_millis last_resolution_time = 0; - if (last_resolution_time == 0) { - last_resolution_time = - grpc_timespec_to_millis_round_up(gpr_now(GPR_CLOCK_MONOTONIC)); - } else { - grpc_millis now = - grpc_timespec_to_millis_round_up(gpr_now(GPR_CLOCK_MONOTONIC)); - GPR_ASSERT(now - last_resolution_time >= kMinResolutionPeriodMs); - last_resolution_time = now; +namespace { + +grpc_core::DNSResolver* g_default_dns_resolver; + +class TestDNSResolver : public grpc_core::DNSResolver { + public: + // Wrapper around default resolve_address in order to count the number of + // times we incur in a system-level name resolution. + grpc_core::OrphanablePtr ResolveName( + absl::string_view name, absl::string_view default_port, + grpc_pollset_set* interested_parties, + std::function>)> + on_done) override { + auto result = g_default_dns_resolver->ResolveName( + name, default_port, interested_parties, std::move(on_done)); + ++g_resolution_count; + static grpc_millis last_resolution_time = 0; + if (last_resolution_time == 0) { + last_resolution_time = + grpc_timespec_to_millis_round_up(gpr_now(GPR_CLOCK_MONOTONIC)); + } else { + grpc_millis now = + grpc_timespec_to_millis_round_up(gpr_now(GPR_CLOCK_MONOTONIC)); + GPR_ASSERT(now - last_resolution_time >= kMinResolutionPeriodMs); + last_resolution_time = now; + } + // For correct time diff comparisons, make sure that any subsequent calls + // to grpc_core::ExecCtx::Get()->Now() on this thread don't return a time + // which is earlier than that returned by the call(s) to + // gpr_now(GPR_CLOCK_MONOTONIC) within this function. This is important + // because the resolver's last_resolution_timestamp_ will be taken from + // grpc_core::ExecCtx::Get()->Now() right after this returns. + grpc_core::ExecCtx::Get()->InvalidateNow(); + return result; } - // For correct time diff comparisons, make sure that any subsequent calls - // to grpc_core::ExecCtx::Get()->Now() on this thread don't return a time - // which is earlier than that returned by the call(s) to - // gpr_now(GPR_CLOCK_MONOTONIC) within this function. This is important - // because the resolver's last_resolution_timestamp_ will be taken from - // grpc_core::ExecCtx::Get()->Now() right after this returns. - grpc_core::ExecCtx::Get()->InvalidateNow(); -} -static grpc_error_handle test_blocking_resolve_address_impl( - const char* name, const char* default_port, - grpc_resolved_addresses** addresses) { - return default_resolve_address->blocking_resolve_address(name, default_port, - addresses); -} + absl::StatusOr> ResolveNameBlocking( + absl::string_view name, absl::string_view default_port) override { + return g_default_dns_resolver->ResolveNameBlocking(name, default_port); + } +}; -static grpc_address_resolver_vtable test_resolver = { - test_resolve_address_impl, test_blocking_resolve_address_impl}; +} // namespace static grpc_ares_request* test_dns_lookup_ares( const char* dns_server, const char* name, const char* default_port, @@ -334,8 +338,8 @@ int main(int argc, char** argv) { g_default_dns_lookup_ares = grpc_dns_lookup_ares; grpc_dns_lookup_ares = test_dns_lookup_ares; - default_resolve_address = grpc_resolve_address_impl; - grpc_set_resolver_impl(&test_resolver); + g_default_dns_resolver = grpc_core::GetDNSResolver(); + grpc_core::SetDNSResolver(new TestDNSResolver()); test_cooldown(); diff --git a/test/core/end2end/dualstack_socket_test.cc b/test/core/end2end/dualstack_socket_test.cc index 4d1aaf074fd..a56989b785b 100644 --- a/test/core/end2end/dualstack_socket_test.cc +++ b/test/core/end2end/dualstack_socket_test.cc @@ -42,6 +42,7 @@ #include "src/core/lib/iomgr/resolve_address.h" #include "src/core/lib/iomgr/socket_utils_posix.h" #include "src/core/lib/slice/slice_string_helpers.h" +#include "src/core/lib/transport/error_utils.h" #include "test/core/end2end/cq_verifier.h" #include "test/core/util/port.h" #include "test/core/util/test_config.h" @@ -59,17 +60,16 @@ static void drain_cq(grpc_completion_queue* cq) { } static void log_resolved_addrs(const char* label, const char* hostname) { - grpc_resolved_addresses* res = nullptr; - grpc_error_handle error = grpc_blocking_resolve_address(hostname, "80", &res); - if (error != GRPC_ERROR_NONE || res == nullptr) { - GRPC_LOG_IF_ERROR(hostname, error); + absl::StatusOr> addresses_or = + grpc_core::GetDNSResolver()->ResolveNameBlocking(hostname, "80"); + if (!addresses_or.ok()) { + GRPC_LOG_IF_ERROR(hostname, + absl_status_to_grpc_error(addresses_or.status())); return; } - for (size_t i = 0; i < res->naddrs; ++i) { - gpr_log(GPR_INFO, "%s: %s", label, - grpc_sockaddr_to_uri(&res->addrs[i]).c_str()); + for (const auto& addr : *addresses_or) { + gpr_log(GPR_INFO, "%s: %s", label, grpc_sockaddr_to_uri(&addr).c_str()); } - grpc_resolved_addresses_destroy(res); } void test_connect(const char* server_host, const char* client_host, int port, @@ -276,20 +276,19 @@ void test_connect(const char* server_host, const char* client_host, int port, } int external_dns_works(const char* host) { - grpc_resolved_addresses* res = nullptr; - grpc_error_handle error = grpc_blocking_resolve_address(host, "80", &res); - GRPC_ERROR_UNREF(error); - if (res == nullptr) { + auto addresses_or = + grpc_core::GetDNSResolver()->ResolveNameBlocking(host, "80"); + if (!addresses_or.ok()) { return 0; } int result = 1; - for (size_t i = 0; i < res->naddrs; ++i) { + for (const auto& addr : *addresses_or) { // Kokoro on Macservice uses Google DNS64 servers by default // (https://en.wikipedia.org/wiki/Google_Public_DNS) and that breaks // "dualstack_socket_test" due to loopback4.unittest.grpc.io resolving to // [64:ff9b::7f00:1]. (Working as expected for DNS64, but it prevents the // dualstack_socket_test from functioning correctly). See b/201064791. - if (grpc_sockaddr_to_uri(&res->addrs[i]) == "ipv6:[64:ff9b::7f00:1]:80") { + if (grpc_sockaddr_to_uri(&addr) == "ipv6:[64:ff9b::7f00:1]:80") { gpr_log( GPR_INFO, "Detected DNS64 server response. Tests that depend on " @@ -298,7 +297,6 @@ int external_dns_works(const char* host) { break; } } - grpc_resolved_addresses_destroy(res); return result; } diff --git a/test/core/end2end/fixtures/http_proxy_fixture.cc b/test/core/end2end/fixtures/http_proxy_fixture.cc index 1c46122e7d1..647cbc9e09a 100644 --- a/test/core/end2end/fixtures/http_proxy_fixture.cc +++ b/test/core/end2end/fixtures/http_proxy_fixture.cc @@ -520,16 +520,15 @@ static void on_read_request_done_locked(void* arg, grpc_error_handle error) { } } // Resolve address. - grpc_resolved_addresses* resolved_addresses = nullptr; - error = grpc_blocking_resolve_address(conn->http_request.path, "80", - &resolved_addresses); - if (error != GRPC_ERROR_NONE) { + absl::StatusOr> addresses_or = + grpc_core::GetDNSResolver()->ResolveNameBlocking(conn->http_request.path, + "80"); + if (!addresses_or.ok()) { proxy_connection_failed(conn, SETUP_FAILED, "HTTP proxy DNS lookup", GRPC_ERROR_REF(error)); - GRPC_ERROR_UNREF(error); return; } - GPR_ASSERT(resolved_addresses->naddrs >= 1); + GPR_ASSERT(!addresses_or->empty()); // Connect to requested address. // The connection callback inherits our reference to conn. const grpc_millis deadline = @@ -540,10 +539,9 @@ static void on_read_request_done_locked(void* arg, grpc_error_handle error) { .channel_args_preconditioning() .PreconditionChannelArgs(nullptr); grpc_tcp_client_connect(&conn->on_server_connect_done, &conn->server_endpoint, - conn->pollset_set, args, - &resolved_addresses->addrs[0], deadline); + conn->pollset_set, args, &(*addresses_or)[0], + deadline); grpc_channel_args_destroy(args); - grpc_resolved_addresses_destroy(resolved_addresses); } static void on_read_request_done(void* arg, grpc_error_handle error) { diff --git a/test/core/end2end/fuzzers/api_fuzzer.cc b/test/core/end2end/fuzzers/api_fuzzer.cc index a4fdf316e25..31a3e2086c8 100644 --- a/test/core/end2end/fuzzers/api_fuzzer.cc +++ b/test/core/end2end/fuzzers/api_fuzzer.cc @@ -87,7 +87,6 @@ typedef struct addr_req { grpc_timer timer; char* addr; grpc_closure* on_done; - grpc_resolved_addresses** addrs; std::unique_ptr* addresses; } addr_req; @@ -95,21 +94,11 @@ static void finish_resolve(void* arg, grpc_error_handle error) { addr_req* r = static_cast(arg); if (error == GRPC_ERROR_NONE && 0 == strcmp(r->addr, "server")) { - if (r->addrs != nullptr) { - grpc_resolved_addresses* addrs = - static_cast(gpr_malloc(sizeof(*addrs))); - addrs->naddrs = 1; - addrs->addrs = static_cast( - gpr_malloc(sizeof(*addrs->addrs))); - addrs->addrs[0].len = 0; - *r->addrs = addrs; - } else if (r->addresses != nullptr) { - *r->addresses = absl::make_unique(); - grpc_resolved_address fake_resolved_address; - memset(&fake_resolved_address, 0, sizeof(fake_resolved_address)); - fake_resolved_address.len = 0; - (*r->addresses)->emplace_back(fake_resolved_address, nullptr); - } + *r->addresses = absl::make_unique(); + grpc_resolved_address fake_resolved_address; + memset(&fake_resolved_address, 0, sizeof(fake_resolved_address)); + fake_resolved_address.len = 0; + (*r->addresses)->emplace_back(fake_resolved_address, nullptr); grpc_core::ExecCtx::Run(DEBUG_LOCATION, r->on_done, GRPC_ERROR_NONE); } else { grpc_core::ExecCtx::Run(DEBUG_LOCATION, r->on_done, @@ -121,21 +110,73 @@ static void finish_resolve(void* arg, grpc_error_handle error) { delete r; } -void my_resolve_address(const char* addr, const char* /*default_port*/, - grpc_pollset_set* /*interested_parties*/, - grpc_closure* on_done, - grpc_resolved_addresses** addrs) { - addr_req* r = new addr_req(); - r->addr = gpr_strdup(addr); - r->on_done = on_done; - r->addrs = addrs; - grpc_timer_init( - &r->timer, GPR_MS_PER_SEC + grpc_core::ExecCtx::Get()->Now(), - GRPC_CLOSURE_CREATE(finish_resolve, r, grpc_schedule_on_exec_ctx)); -} +namespace { + +class FuzzerDNSResolver : public grpc_core::DNSResolver { + public: + class FuzzerDNSRequest : public grpc_core::DNSResolver::Request { + public: + FuzzerDNSRequest( + absl::string_view name, + std::function>)> + on_done) + : name_(std::string(name)), on_done_(std::move(on_done)) {} + + void Start() override { + Ref().release(); // ref held by timer callback + grpc_timer_init( + &timer_, GPR_MS_PER_SEC + grpc_core::ExecCtx::Get()->Now(), + GRPC_CLOSURE_CREATE(FinishResolve, this, grpc_schedule_on_exec_ctx)); + } + + // cancellation not implemented + void Orphan() override { Unref(); } + + private: + static void FinishResolve(void* arg, grpc_error_handle error) { + FuzzerDNSRequest* self = static_cast(arg); + if (error == GRPC_ERROR_NONE && self->name_ == "server") { + std::vector addrs; + grpc_resolved_address addr; + addr.len = 0; + addrs.push_back(addr); + self->on_done_(std::move(addrs)); + } else { + self->on_done_(absl::UnknownError("Resolution failed")); + } + self->Unref(); + } + + const std::string name_; + const std::function>)> + on_done_; + grpc_timer timer_; + }; + + // Gets the singleton instance, possibly creating it first + static FuzzerDNSResolver* GetOrCreate() { + static FuzzerDNSResolver* instance = new FuzzerDNSResolver(); + return instance; + } + + grpc_core::OrphanablePtr ResolveName( + absl::string_view name, absl::string_view /* default_port */, + grpc_pollset_set* /* interested_parties */, + std::function>)> + on_done) override { + return grpc_core::MakeOrphanable(name, + std::move(on_done)); + } + + absl::StatusOr> ResolveNameBlocking( + absl::string_view /* name */, + absl::string_view /* default_port */) override { + GPR_ASSERT(0); + } +}; -static grpc_address_resolver_vtable fuzzer_resolver = {my_resolve_address, - nullptr}; +} // namespace grpc_ares_request* my_dns_lookup_ares( const char* /*dns_server*/, const char* addr, const char* /*default_port*/, @@ -146,7 +187,6 @@ grpc_ares_request* my_dns_lookup_ares( addr_req* r = new addr_req(); r->addr = gpr_strdup(addr); r->on_done = on_done; - r->addrs = nullptr; r->addresses = addresses; grpc_timer_init( &r->timer, GPR_MS_PER_SEC + grpc_core::ExecCtx::Get()->Now(), @@ -728,7 +768,7 @@ DEFINE_PROTO_FUZZER(const api_fuzzer::Msg& msg) { grpc_core::ExecCtx exec_ctx; grpc_core::Executor::SetThreadingAll(false); } - grpc_set_resolver_impl(&fuzzer_resolver); + grpc_core::SetDNSResolver(FuzzerDNSResolver::GetOrCreate()); grpc_dns_lookup_ares = my_dns_lookup_ares; grpc_cancel_ares_request = my_cancel_ares_request; diff --git a/test/core/end2end/goaway_server_test.cc b/test/core/end2end/goaway_server_test.cc index 9924ab61656..bc804f84db8 100644 --- a/test/core/end2end/goaway_server_test.cc +++ b/test/core/end2end/goaway_server_test.cc @@ -29,15 +29,13 @@ #include "src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.h" #include "src/core/ext/filters/client_channel/server_address.h" #include "src/core/lib/iomgr/resolve_address.h" +#include "src/core/lib/iomgr/resolve_address_impl.h" #include "src/core/lib/iomgr/sockaddr.h" #include "src/core/lib/iomgr/socket_utils.h" #include "test/core/end2end/cq_verifier.h" #include "test/core/util/port.h" #include "test/core/util/test_config.h" -extern grpc_address_resolver_vtable* grpc_resolve_address_impl; -static grpc_address_resolver_vtable* default_resolver; - static void* tag(intptr_t t) { return reinterpret_cast(t); } static gpr_mu g_mu; @@ -58,47 +56,66 @@ static void set_resolve_port(int port) { gpr_mu_unlock(&g_mu); } -static void my_resolve_address(const char* addr, const char* default_port, - grpc_pollset_set* interested_parties, - grpc_closure* on_done, - grpc_resolved_addresses** addrs) { - if (0 != strcmp(addr, "test")) { - default_resolver->resolve_address(addr, default_port, interested_parties, - on_done, addrs); - return; +namespace { + +grpc_core::DNSResolver* g_default_dns_resolver; + +class TestDNSResolver : public grpc_core::DNSResolver { + public: + class TestDNSRequest : public grpc_core::DNSResolver::Request { + public: + explicit TestDNSRequest( + std::function>)> + on_done) + : on_done_(std::move(on_done)) {} + + void Start() override { + gpr_mu_lock(&g_mu); + if (g_resolve_port < 0) { + gpr_mu_unlock(&g_mu); + new grpc_core::DNSCallbackExecCtxScheduler( + std::move(on_done_), absl::UnknownError("Forced Failure")); + } else { + std::vector addrs; + grpc_resolved_address addr; + grpc_sockaddr_in* sa = reinterpret_cast(&addr); + sa->sin_family = GRPC_AF_INET; + sa->sin_addr.s_addr = 0x100007f; + sa->sin_port = grpc_htons(static_cast(g_resolve_port)); + addr.len = static_cast(sizeof(*sa)); + addrs.push_back(addr); + gpr_mu_unlock(&g_mu); + new grpc_core::DNSCallbackExecCtxScheduler(std::move(on_done_), + std::move(addrs)); + } + } + + void Orphan() override { Unref(); } + + private: + std::function>)> + on_done_; + }; + + grpc_core::OrphanablePtr ResolveName( + absl::string_view name, absl::string_view default_port, + grpc_pollset_set* interested_parties, + std::function>)> + on_done) override { + if (name != "test") { + return g_default_dns_resolver->ResolveName( + name, default_port, interested_parties, std::move(on_done)); + } + return grpc_core::MakeOrphanable(std::move(on_done)); } - grpc_error_handle 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 { - *addrs = static_cast(gpr_malloc(sizeof(**addrs))); - (*addrs)->naddrs = 1; - (*addrs)->addrs = static_cast( - gpr_malloc(sizeof(*(*addrs)->addrs))); - memset((*addrs)->addrs, 0, sizeof(*(*addrs)->addrs)); - grpc_sockaddr_in* sa = - reinterpret_cast((*addrs)->addrs[0].addr); - sa->sin_family = GRPC_AF_INET; - sa->sin_addr.s_addr = 0x100007f; - sa->sin_port = grpc_htons(static_cast(g_resolve_port)); - (*addrs)->addrs[0].len = static_cast(sizeof(*sa)); - gpr_mu_unlock(&g_mu); + absl::StatusOr> ResolveNameBlocking( + absl::string_view name, absl::string_view default_port) override { + return g_default_dns_resolver->ResolveNameBlocking(name, default_port); } - grpc_core::ExecCtx::Run(DEBUG_LOCATION, on_done, error); -} - -static grpc_error_handle my_blocking_resolve_address( - const char* name, const char* default_port, - grpc_resolved_addresses** addresses) { - return default_resolver->blocking_resolve_address(name, default_port, - addresses); -} +}; -static grpc_address_resolver_vtable test_resolver = { - my_resolve_address, my_blocking_resolve_address}; +} // namespace static grpc_ares_request* my_dns_lookup_ares( const char* dns_server, const char* addr, const char* default_port, @@ -147,8 +164,8 @@ int main(int argc, char** argv) { gpr_mu_init(&g_mu); grpc_init(); - default_resolver = grpc_resolve_address_impl; - grpc_set_resolver_impl(&test_resolver); + g_default_dns_resolver = grpc_core::GetDNSResolver(); + grpc_core::SetDNSResolver(new TestDNSResolver()); iomgr_dns_lookup_ares = grpc_dns_lookup_ares; iomgr_cancel_ares_request = grpc_cancel_ares_request; grpc_dns_lookup_ares = my_dns_lookup_ares; diff --git a/test/core/iomgr/BUILD b/test/core/iomgr/BUILD index 4b56aa1fae4..2879faf7177 100644 --- a/test/core/iomgr/BUILD +++ b/test/core/iomgr/BUILD @@ -174,6 +174,9 @@ grpc_cc_test( args = [ "--resolver=ares", ], + external_deps = [ + "absl/strings", + ], language = "C++", tags = ["no_windows"], deps = [ @@ -189,6 +192,9 @@ grpc_cc_test( args = [ "--resolver=native", ], + external_deps = [ + "absl/strings", + ], language = "C++", tags = ["no_windows"], deps = [ @@ -201,28 +207,34 @@ grpc_cc_test( grpc_cc_test( name = "resolve_address_using_ares_resolver_test", srcs = ["resolve_address_test.cc"], - args = [ - "--resolver=ares", + external_deps = [ + "absl/strings", + "gtest", ], language = "C++", deps = [ "//:gpr", "//:grpc", + "//test/core/util:fake_udp_and_tcp_server", "//test/core/util:grpc_test_util", + "//test/cpp/util:test_config", ], ) grpc_cc_test( name = "resolve_address_using_native_resolver_test", srcs = ["resolve_address_test.cc"], - args = [ - "--resolver=native", + external_deps = [ + "absl/strings", + "gtest", ], language = "C++", deps = [ "//:gpr", "//:grpc", + "//test/core/util:fake_udp_and_tcp_server", "//test/core/util:grpc_test_util", + "//test/cpp/util:test_config", ], ) diff --git a/test/core/iomgr/resolve_address_posix_test.cc b/test/core/iomgr/resolve_address_posix_test.cc index 7e34a02737b..a79dedb2eaf 100644 --- a/test/core/iomgr/resolve_address_posix_test.cc +++ b/test/core/iomgr/resolve_address_posix_test.cc @@ -48,7 +48,6 @@ static gpr_timespec test_deadline(void) { typedef struct args_struct { grpc_core::Thread thd; gpr_event ev; - grpc_resolved_addresses* addrs; gpr_mu* mu; bool done; // guarded by mu grpc_pollset* pollset; // guarded by mu @@ -63,7 +62,6 @@ void args_init(args_struct* args) { grpc_pollset_init(args->pollset, &args->mu); args->pollset_set = grpc_pollset_set_create(); grpc_pollset_set_add_pollset(args->pollset_set, args->pollset); - args->addrs = nullptr; args->done = false; } @@ -72,7 +70,6 @@ void args_finish(args_struct* args) { args->thd.Join(); // Don't need to explicitly destruct args->thd since // args is actually going to be destructed, not just freed - grpc_resolved_addresses_destroy(args->addrs); grpc_pollset_set_del_pollset(args->pollset_set, args->pollset); grpc_pollset_set_destroy(args->pollset_set); grpc_closure do_nothing_cb; @@ -117,33 +114,30 @@ static void poll_pollset_until_request_done(args_struct* args) { args->thd.Start(); } -static void must_succeed(void* argsp, grpc_error_handle err) { - args_struct* args = static_cast(argsp); - GPR_ASSERT(err == GRPC_ERROR_NONE); - GPR_ASSERT(args->addrs != nullptr); - GPR_ASSERT(args->addrs->naddrs > 0); - grpc_core::MutexLockForGprMu lock(args->mu); - args->done = true; - GRPC_LOG_IF_ERROR("pollset_kick", grpc_pollset_kick(args->pollset, nullptr)); -} +namespace { -static void must_fail(void* argsp, grpc_error_handle err) { - args_struct* args = static_cast(argsp); - GPR_ASSERT(err != GRPC_ERROR_NONE); +void MustSucceed(args_struct* args, + absl::StatusOr> result) { + GPR_ASSERT(result.ok()); + GPR_ASSERT(!result->empty()); grpc_core::MutexLockForGprMu lock(args->mu); args->done = true; GRPC_LOG_IF_ERROR("pollset_kick", grpc_pollset_kick(args->pollset, nullptr)); } +} // namespace + static void resolve_address_must_succeed(const char* target) { grpc_core::ExecCtx exec_ctx; args_struct args; args_init(&args); poll_pollset_until_request_done(&args); - grpc_resolve_address( + auto r = grpc_core::GetDNSResolver()->ResolveName( target, "1" /* port number */, args.pollset_set, - GRPC_CLOSURE_CREATE(must_succeed, &args, grpc_schedule_on_exec_ctx), - &args.addrs); + [&args](absl::StatusOr> result) { + MustSucceed(&args, std::move(result)); + }); + r->Start(); grpc_core::ExecCtx::Get()->Flush(); args_finish(&args); } diff --git a/test/core/iomgr/resolve_address_test.cc b/test/core/iomgr/resolve_address_test.cc index 6e633f52659..f86b7e42ea9 100644 --- a/test/core/iomgr/resolve_address_test.cc +++ b/test/core/iomgr/resolve_address_test.cc @@ -21,6 +21,11 @@ #include #include +#include +#include + +#include "absl/functional/bind_front.h" +#include "absl/strings/match.h" #include #include @@ -28,6 +33,7 @@ #include #include +#include "src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.h" #include "src/core/ext/filters/client_channel/resolver/dns/dns_resolver_selection.h" #include "src/core/lib/event_engine/sockaddr.h" #include "src/core/lib/gpr/string.h" @@ -35,368 +41,397 @@ #include "src/core/lib/iomgr/executor.h" #include "src/core/lib/iomgr/iomgr.h" #include "test/core/util/cmdline.h" +#include "test/core/util/fake_udp_and_tcp_server.h" #include "test/core/util/test_config.h" +#include "test/cpp/util/test_config.h" -static gpr_timespec test_deadline(void) { - return grpc_timeout_seconds_to_deadline(100); -} - -typedef struct args_struct { - gpr_event ev; - grpc_resolved_addresses* addrs; - gpr_mu* mu; - bool done; // guarded by mu - grpc_pollset* pollset; // guarded by mu - grpc_pollset_set* pollset_set; -} args_struct; - -static void do_nothing(void* /*arg*/, grpc_error_handle /*error*/) {} - -void args_init(args_struct* args) { - gpr_event_init(&args->ev); - args->pollset = static_cast(gpr_zalloc(grpc_pollset_size())); - grpc_pollset_init(args->pollset, &args->mu); - args->pollset_set = grpc_pollset_set_create(); - grpc_pollset_set_add_pollset(args->pollset_set, args->pollset); - args->addrs = nullptr; - args->done = false; -} - -void args_finish(args_struct* args) { - GPR_ASSERT(gpr_event_wait(&args->ev, test_deadline())); - grpc_resolved_addresses_destroy(args->addrs); - grpc_pollset_set_del_pollset(args->pollset_set, args->pollset); - grpc_pollset_set_destroy(args->pollset_set); - grpc_closure do_nothing_cb; - GRPC_CLOSURE_INIT(&do_nothing_cb, do_nothing, nullptr, - grpc_schedule_on_exec_ctx); - gpr_mu_lock(args->mu); - grpc_pollset_shutdown(args->pollset, &do_nothing_cb); - gpr_mu_unlock(args->mu); - // exec_ctx needs to be flushed before calling grpc_pollset_destroy() - grpc_core::ExecCtx::Get()->Flush(); - grpc_pollset_destroy(args->pollset); - gpr_free(args->pollset); -} +namespace { -static grpc_millis n_sec_deadline(int seconds) { +grpc_millis NSecDeadline(int seconds) { return grpc_timespec_to_millis_round_up( grpc_timeout_seconds_to_deadline(seconds)); } -static void poll_pollset_until_request_done(args_struct* args) { - // Try to give enough time for c-ares to run through its retries - // a few times if needed. - grpc_millis deadline = n_sec_deadline(90); - while (true) { +const char* g_resolver_type = ""; + +class ResolveAddressTest : public ::testing::Test { + public: + ResolveAddressTest() { + grpc_init(); grpc_core::ExecCtx exec_ctx; + pollset_ = static_cast(gpr_zalloc(grpc_pollset_size())); + grpc_pollset_init(pollset_, &mu_); + pollset_set_ = grpc_pollset_set_create(); + grpc_pollset_set_add_pollset(pollset_set_, pollset_); + default_inject_config_ = grpc_ares_test_only_inject_config; + } + + ~ResolveAddressTest() override { { - grpc_core::MutexLockForGprMu lock(args->mu); - if (args->done) { - break; + grpc_core::ExecCtx exec_ctx; + grpc_pollset_set_del_pollset(pollset_set_, pollset_); + grpc_pollset_set_destroy(pollset_set_); + grpc_closure do_nothing_cb; + GRPC_CLOSURE_INIT(&do_nothing_cb, DoNothing, nullptr, + grpc_schedule_on_exec_ctx); + gpr_mu_lock(mu_); + grpc_pollset_shutdown(pollset_, &do_nothing_cb); + gpr_mu_unlock(mu_); + // exec_ctx needs to be flushed before calling grpc_pollset_destroy() + grpc_core::ExecCtx::Get()->Flush(); + grpc_pollset_destroy(pollset_); + gpr_free(pollset_); + // reset this since it might have been altered + grpc_ares_test_only_inject_config = default_inject_config_; + } + grpc_shutdown(); + } + + void PollPollsetUntilRequestDone() { + // Try to give enough time for c-ares to run through its retries + // a few times if needed. + grpc_millis deadline = NSecDeadline(90); + while (true) { + grpc_core::ExecCtx exec_ctx; + { + grpc_core::MutexLockForGprMu lock(mu_); + if (done_) { + break; + } + grpc_millis time_left = deadline - grpc_core::ExecCtx::Get()->Now(); + gpr_log(GPR_DEBUG, "done=%d, time_left=%" PRId64, done_, time_left); + ASSERT_GE(time_left, 0); + grpc_pollset_worker* worker = nullptr; + GRPC_LOG_IF_ERROR("pollset_work", grpc_pollset_work(pollset_, &worker, + NSecDeadline(1))); } - grpc_millis time_left = deadline - grpc_core::ExecCtx::Get()->Now(); - gpr_log(GPR_DEBUG, "done=%d, time_left=%" PRId64, args->done, time_left); - GPR_ASSERT(time_left >= 0); - grpc_pollset_worker* worker = nullptr; - GRPC_LOG_IF_ERROR( - "pollset_work", - grpc_pollset_work(args->pollset, &worker, n_sec_deadline(1))); } } - gpr_event_set(&args->ev, reinterpret_cast(1)); + + void MustSucceed(absl::StatusOr> result) { + EXPECT_EQ(result.status(), absl::OkStatus()); + EXPECT_FALSE(result->empty()); + Finish(); + } + + void MustFail(absl::StatusOr> result) { + EXPECT_NE(result.status(), absl::OkStatus()); + Finish(); + } + + void MustFailExpectCancelledErrorMessage( + absl::StatusOr> result) { + EXPECT_NE(result.status(), absl::OkStatus()); + EXPECT_THAT(result.status().ToString(), + testing::HasSubstr("DNS query cancelled")); + Finish(); + } + + void DontCare( + absl::StatusOr> /* result */) { + Finish(); + } + + // This test assumes the environment has an ipv6 loopback + void MustSucceedWithIPv6First( + absl::StatusOr> result) { + EXPECT_EQ(result.status(), absl::OkStatus()); + EXPECT_TRUE(!result->empty() && + reinterpret_cast((*result)[0].addr) + ->sa_family == AF_INET6); + Finish(); + } + + void MustSucceedWithIPv4First( + absl::StatusOr> result) { + EXPECT_EQ(result.status(), absl::OkStatus()); + EXPECT_TRUE(!result->empty() && + reinterpret_cast((*result)[0].addr) + ->sa_family == AF_INET); + Finish(); + } + + grpc_pollset_set* pollset_set() const { return pollset_set_; } + + private: + static void DoNothing(void* /*arg*/, grpc_error_handle /*error*/) {} + + void Finish() { + grpc_core::MutexLockForGprMu lock(mu_); + done_ = true; + GRPC_LOG_IF_ERROR("pollset_kick", grpc_pollset_kick(pollset_, nullptr)); + } + + gpr_mu* mu_; + bool done_ = false; // guarded by mu + grpc_pollset* pollset_; // guarded by mu + grpc_pollset_set* pollset_set_; + // the default value of grpc_ares_test_only_inject_config, which might + // be modified during a test + void (*default_inject_config_)(ares_channel channel) = nullptr; +}; + +} // namespace + +TEST_F(ResolveAddressTest, Localhost) { + grpc_core::ExecCtx exec_ctx; + auto r = grpc_core::GetDNSResolver()->ResolveName( + "localhost:1", "", pollset_set(), + absl::bind_front(&ResolveAddressTest::MustSucceed, this)); + r->Start(); + grpc_core::ExecCtx::Get()->Flush(); + PollPollsetUntilRequestDone(); } -static void must_succeed(void* argsp, grpc_error_handle err) { - args_struct* args = static_cast(argsp); - GPR_ASSERT(err == GRPC_ERROR_NONE); - GPR_ASSERT(args->addrs != nullptr); - GPR_ASSERT(args->addrs->naddrs > 0); - grpc_core::MutexLockForGprMu lock(args->mu); - args->done = true; - GRPC_LOG_IF_ERROR("pollset_kick", grpc_pollset_kick(args->pollset, nullptr)); +TEST_F(ResolveAddressTest, DefaultPort) { + grpc_core::ExecCtx exec_ctx; + auto r = grpc_core::GetDNSResolver()->ResolveName( + "localhost", "1", pollset_set(), + absl::bind_front(&ResolveAddressTest::MustSucceed, this)); + r->Start(); + grpc_core::ExecCtx::Get()->Flush(); + PollPollsetUntilRequestDone(); } -static void must_fail(void* argsp, grpc_error_handle err) { - args_struct* args = static_cast(argsp); - GPR_ASSERT(err != GRPC_ERROR_NONE); - grpc_core::MutexLockForGprMu lock(args->mu); - args->done = true; - GRPC_LOG_IF_ERROR("pollset_kick", grpc_pollset_kick(args->pollset, nullptr)); +TEST_F(ResolveAddressTest, LocalhostResultHasIPv6First) { + if (std::string(g_resolver_type) != "ares") { + GTEST_SKIP() << "this test is only valid with the c-ares resolver"; + } + grpc_core::ExecCtx exec_ctx; + auto r = grpc_core::GetDNSResolver()->ResolveName( + "localhost:1", "", pollset_set(), + absl::bind_front(&ResolveAddressTest::MustSucceedWithIPv6First, this)); + r->Start(); + grpc_core::ExecCtx::Get()->Flush(); + PollPollsetUntilRequestDone(); } -// This test assumes the environment has an ipv6 loopback -static void must_succeed_with_ipv6_first(void* argsp, grpc_error_handle 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); - grpc_core::MutexLockForGprMu lock(args->mu); - args->done = true; - GRPC_LOG_IF_ERROR("pollset_kick", grpc_pollset_kick(args->pollset, nullptr)); +namespace { + +bool IPv6DisabledGetSourceAddr(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; } -static void must_succeed_with_ipv4_first(void* argsp, grpc_error_handle 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); - grpc_core::MutexLockForGprMu lock(args->mu); - args->done = true; - GRPC_LOG_IF_ERROR("pollset_kick", grpc_pollset_kick(args->pollset, nullptr)); +void DeleteSourceAddrFactory(address_sorting_source_addr_factory* factory) { + delete factory; } -static void test_localhost(void) { +const address_sorting_source_addr_factory_vtable + kMockIpv6DisabledSourceAddrFactoryVtable = { + IPv6DisabledGetSourceAddr, + DeleteSourceAddrFactory, +}; + +} // namespace + +TEST_F(ResolveAddressTest, LocalhostResultHasIPv4FirstWhenIPv6IsntAvalailable) { + if (std::string(g_resolver_type) != "ares") { + GTEST_SKIP() << "this test is only valid with the c-ares resolver"; + } + // Mock the kernel source address selection. Note that source addr factory + // is reset to its default value during grpc initialization for each test. + address_sorting_source_addr_factory* mock = + new address_sorting_source_addr_factory(); + mock->vtable = &kMockIpv6DisabledSourceAddrFactoryVtable; + address_sorting_override_source_addr_factory_for_testing(mock); + // run the test 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, &args, grpc_schedule_on_exec_ctx), - &args.addrs); + auto r = grpc_core::GetDNSResolver()->ResolveName( + "localhost:1", "", pollset_set(), + absl::bind_front(&ResolveAddressTest::MustSucceedWithIPv4First, this)); + r->Start(); grpc_core::ExecCtx::Get()->Flush(); - poll_pollset_until_request_done(&args); - args_finish(&args); + PollPollsetUntilRequestDone(); } -static void test_default_port(void) { +TEST_F(ResolveAddressTest, NonNumericDefaultPort) { grpc_core::ExecCtx exec_ctx; - args_struct args; - args_init(&args); - grpc_resolve_address( - "localhost", "1", args.pollset_set, - GRPC_CLOSURE_CREATE(must_succeed, &args, grpc_schedule_on_exec_ctx), - &args.addrs); + auto r = grpc_core::GetDNSResolver()->ResolveName( + "localhost", "http", pollset_set(), + absl::bind_front(&ResolveAddressTest::MustSucceed, this)); + r->Start(); grpc_core::ExecCtx::Get()->Flush(); - poll_pollset_until_request_done(&args); - args_finish(&args); + PollPollsetUntilRequestDone(); } -static void test_localhost_result_has_ipv6_first(void) { +TEST_F(ResolveAddressTest, MissingDefaultPort) { 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); + auto r = grpc_core::GetDNSResolver()->ResolveName( + "localhost", "", pollset_set(), + absl::bind_front(&ResolveAddressTest::MustFail, this)); + r->Start(); grpc_core::ExecCtx::Get()->Flush(); - poll_pollset_until_request_done(&args); - args_finish(&args); + PollPollsetUntilRequestDone(); } -static void test_localhost_result_has_ipv4_first_when_ipv6_isnt_available( - void) { +TEST_F(ResolveAddressTest, IPv6WithPort) { 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); + auto r = grpc_core::GetDNSResolver()->ResolveName( + "[2001:db8::1]:1", "", pollset_set(), + absl::bind_front(&ResolveAddressTest::MustSucceed, this)); + r->Start(); grpc_core::ExecCtx::Get()->Flush(); - poll_pollset_until_request_done(&args); - args_finish(&args); + PollPollsetUntilRequestDone(); } -static void test_non_numeric_default_port(void) { +void TestIPv6WithoutPort(ResolveAddressTest* test, const char* target) { grpc_core::ExecCtx exec_ctx; - args_struct args; - args_init(&args); - grpc_resolve_address( - "localhost", "https", args.pollset_set, - GRPC_CLOSURE_CREATE(must_succeed, &args, grpc_schedule_on_exec_ctx), - &args.addrs); + auto r = grpc_core::GetDNSResolver()->ResolveName( + target, "80", test->pollset_set(), + absl::bind_front(&ResolveAddressTest::MustSucceed, test)); + r->Start(); grpc_core::ExecCtx::Get()->Flush(); - poll_pollset_until_request_done(&args); - args_finish(&args); + test->PollPollsetUntilRequestDone(); +} + +TEST_F(ResolveAddressTest, IPv6WithoutPortNoBrackets) { + TestIPv6WithoutPort(this, "2001:db8::1"); } -static void test_missing_default_port(void) { +TEST_F(ResolveAddressTest, IPv6WithoutPortWithBrackets) { + TestIPv6WithoutPort(this, "[2001:db8::1]"); +} + +TEST_F(ResolveAddressTest, IPv6WithoutPortV4MappedV6) { + TestIPv6WithoutPort(this, "2001:db8::1.2.3.4"); +} + +void TestInvalidIPAddress(ResolveAddressTest* test, const char* target) { grpc_core::ExecCtx exec_ctx; - args_struct args; - args_init(&args); - grpc_resolve_address( - "localhost", nullptr, args.pollset_set, - GRPC_CLOSURE_CREATE(must_fail, &args, grpc_schedule_on_exec_ctx), - &args.addrs); + auto r = grpc_core::GetDNSResolver()->ResolveName( + target, "", test->pollset_set(), + absl::bind_front(&ResolveAddressTest::MustFail, test)); + r->Start(); grpc_core::ExecCtx::Get()->Flush(); - poll_pollset_until_request_done(&args); - args_finish(&args); + test->PollPollsetUntilRequestDone(); } -static void test_ipv6_with_port(void) { +TEST_F(ResolveAddressTest, InvalidIPv4Addresses) { + TestInvalidIPAddress(this, "293.283.1238.3:1"); +} + +TEST_F(ResolveAddressTest, InvalidIPv6Addresses) { + TestInvalidIPAddress(this, "[2001:db8::11111]:1"); +} + +void TestUnparseableHostPort(ResolveAddressTest* test, const char* target) { grpc_core::ExecCtx exec_ctx; - args_struct args; - args_init(&args); - grpc_resolve_address( - "[2001:db8::1]:1", nullptr, args.pollset_set, - GRPC_CLOSURE_CREATE(must_succeed, &args, grpc_schedule_on_exec_ctx), - &args.addrs); + auto r = grpc_core::GetDNSResolver()->ResolveName( + target, "1", test->pollset_set(), + absl::bind_front(&ResolveAddressTest::MustFail, test)); + r->Start(); grpc_core::ExecCtx::Get()->Flush(); - poll_pollset_until_request_done(&args); - args_finish(&args); + test->PollPollsetUntilRequestDone(); } -static void test_ipv6_without_port(void) { - const char* const kCases[] = { - "2001:db8::1", - "2001:db8::1.2.3.4", - "[2001:db8::1]", - }; - unsigned i; - for (i = 0; i < sizeof(kCases) / sizeof(*kCases); i++) { - grpc_core::ExecCtx exec_ctx; - args_struct args; - args_init(&args); - grpc_resolve_address( - kCases[i], "80", args.pollset_set, - GRPC_CLOSURE_CREATE(must_succeed, &args, grpc_schedule_on_exec_ctx), - &args.addrs); - grpc_core::ExecCtx::Get()->Flush(); - poll_pollset_until_request_done(&args); - args_finish(&args); - } +TEST_F(ResolveAddressTest, UnparseableHostPortsOnlyBracket) { + TestUnparseableHostPort(this, "["); } -static void test_invalid_ip_addresses(void) { - const char* const kCases[] = { - "293.283.1238.3:1", - "[2001:db8::11111]:1", - }; - unsigned i; - for (i = 0; i < sizeof(kCases) / sizeof(*kCases); i++) { - grpc_core::ExecCtx exec_ctx; - args_struct args; - args_init(&args); - grpc_resolve_address( - kCases[i], nullptr, args.pollset_set, - GRPC_CLOSURE_CREATE(must_fail, &args, grpc_schedule_on_exec_ctx), - &args.addrs); - grpc_core::ExecCtx::Get()->Flush(); - poll_pollset_until_request_done(&args); - args_finish(&args); - } +TEST_F(ResolveAddressTest, UnparseableHostPortsMissingRightBracket) { + TestUnparseableHostPort(this, "[::1"); } -static void test_unparseable_hostports(void) { - const char* const kCases[] = { - "[", "[::1", "[::1]bad", "[1.2.3.4]", "[localhost]", "[localhost]:1", - }; - unsigned i; - for (i = 0; i < sizeof(kCases) / sizeof(*kCases); i++) { - grpc_core::ExecCtx exec_ctx; - args_struct args; - args_init(&args); - grpc_resolve_address( - kCases[i], "1", args.pollset_set, - GRPC_CLOSURE_CREATE(must_fail, &args, grpc_schedule_on_exec_ctx), - &args.addrs); - grpc_core::ExecCtx::Get()->Flush(); - poll_pollset_until_request_done(&args); - args_finish(&args); - } +TEST_F(ResolveAddressTest, UnparseableHostPortsBadPort) { + TestUnparseableHostPort(this, "[::1]bad"); } -typedef struct mock_ipv6_disabled_source_addr_factory { - address_sorting_source_addr_factory base; -} mock_ipv6_disabled_source_addr_factory; +TEST_F(ResolveAddressTest, UnparseableHostPortsBadIPv6) { + TestUnparseableHostPort(this, "[1.2.3.4]"); +} -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; +TEST_F(ResolveAddressTest, UnparseableHostPortsBadLocalhost) { + TestUnparseableHostPort(this, "[localhost]"); } -void mock_ipv6_disabled_source_addr_factory_destroy( - address_sorting_source_addr_factory* factory) { - mock_ipv6_disabled_source_addr_factory* f = - reinterpret_cast(factory); - gpr_free(f); +TEST_F(ResolveAddressTest, UnparseableHostPortsBadLocalhostWithPort) { + TestUnparseableHostPort(this, "[localhost]:1"); } -const address_sorting_source_addr_factory_vtable - kMockIpv6DisabledSourceAddrFactoryVtable = { - mock_ipv6_disabled_source_addr_factory_get_source_addr, - mock_ipv6_disabled_source_addr_factory_destroy, -}; +// Kick off a simple DNS resolution and then immediately cancel. This +// test doesn't care what the result is, just that we don't crash etc. +TEST_F(ResolveAddressTest, ImmediateCancel) { + grpc_core::ExecCtx exec_ctx; + auto r = grpc_core::GetDNSResolver()->ResolveName( + "localhost:1", "1", pollset_set(), + absl::bind_front(&ResolveAddressTest::DontCare, this)); + r->Start(); + r.reset(); // cancel the resolution + grpc_core::ExecCtx::Get()->Flush(); + PollPollsetUntilRequestDone(); +} -int main(int argc, char** argv) { - // First set the resolver type based off of --resolver - const char* resolver_type = nullptr; - gpr_cmdline* cl = gpr_cmdline_create("resolve address test"); - gpr_cmdline_add_string(cl, "resolver", "Resolver type (ares or native)", - &resolver_type); - // In case that there are more than one argument on the command line, - // --resolver will always be the first one, so only parse the first argument - // (other arguments may be unknown to cl) - gpr_cmdline_parse(cl, argc > 2 ? 2 : argc, argv); - grpc_core::UniquePtr resolver = - GPR_GLOBAL_CONFIG_GET(grpc_dns_resolver); - if (strlen(resolver.get()) != 0) { - gpr_log(GPR_INFO, "Warning: overriding resolver setting of %s", - resolver.get()); +namespace { + +int g_fake_non_responsive_dns_server_port; + +void InjectNonResponsiveDNSServer(ares_channel channel) { + gpr_log(GPR_DEBUG, + "Injecting broken nameserver list. Bad server address:|[::1]:%d|.", + g_fake_non_responsive_dns_server_port); + // Configure a non-responsive DNS server at the front of c-ares's nameserver + // list. + struct ares_addr_port_node dns_server_addrs[1]; + memset(dns_server_addrs, 0, sizeof(dns_server_addrs)); + dns_server_addrs[0].family = AF_INET6; + (reinterpret_cast(&dns_server_addrs[0].addr.addr6))[15] = 0x1; + dns_server_addrs[0].tcp_port = g_fake_non_responsive_dns_server_port; + dns_server_addrs[0].udp_port = g_fake_non_responsive_dns_server_port; + dns_server_addrs[0].next = nullptr; + ASSERT_EQ(ares_set_servers_ports(channel, dns_server_addrs), ARES_SUCCESS); +} + +} // namespace + +TEST_F(ResolveAddressTest, CancelWithNonResponsiveDNSServer) { + if (std::string(g_resolver_type) != "ares") { + GTEST_SKIP() << "the native resolver doesn't support cancellation, so we " + "can only test this with c-ares"; } - if (resolver_type != nullptr && gpr_stricmp(resolver_type, "native") == 0) { - GPR_GLOBAL_CONFIG_SET(grpc_dns_resolver, "native"); - } else if (resolver_type != nullptr && - gpr_stricmp(resolver_type, "ares") == 0) { - GPR_GLOBAL_CONFIG_SET(grpc_dns_resolver, "ares"); + // Inject an unresponsive DNS server into the resolver's DNS server config + grpc_core::testing::FakeUdpAndTcpServer fake_dns_server( + grpc_core::testing::FakeUdpAndTcpServer::AcceptMode:: + kWaitForClientToSendFirstBytes, + grpc_core::testing::FakeUdpAndTcpServer::CloseSocketUponCloseFromPeer); + g_fake_non_responsive_dns_server_port = fake_dns_server.port(); + grpc_ares_test_only_inject_config = InjectNonResponsiveDNSServer; + // Run the test + grpc_core::ExecCtx exec_ctx; + auto r = grpc_core::GetDNSResolver()->ResolveName( + "foo.bar.com:1", "1", pollset_set(), + absl::bind_front(&ResolveAddressTest::MustFailExpectCancelledErrorMessage, + this)); + r->Start(); + grpc_core::ExecCtx::Get()->Flush(); // initiate DNS requests + r.reset(); // cancel the resolution + grpc_core::ExecCtx::Get()->Flush(); // let cancellation work finish + PollPollsetUntilRequestDone(); +} + +int main(int argc, char** argv) { + // Configure the DNS resolver (c-ares vs. native) based on the + // name of the binary. TODO(apolcyn): is there a way to pass command + // line flags to a gtest that it works in all of our test environments? + if (absl::StrContains(std::string(argv[0]), "using_native_resolver")) { + g_resolver_type = "native"; + } else if (absl::StrContains(std::string(argv[0]), "using_ares_resolver")) { + g_resolver_type = "ares"; } else { - gpr_log(GPR_ERROR, "--resolver_type was not set to ares or native"); - abort(); + GPR_ASSERT(0); } - // Run the test. + GPR_GLOBAL_CONFIG_SET(grpc_dns_resolver, g_resolver_type); + ::testing::InitGoogleTest(&argc, argv); grpc::testing::TestEnvironment env(argc, argv); - grpc_init(); - { - grpc_core::ExecCtx exec_ctx; - test_localhost(); - test_default_port(); - test_non_numeric_default_port(); - test_missing_default_port(); - test_ipv6_with_port(); - test_ipv6_without_port(); - test_invalid_ip_addresses(); - test_unparseable_hostports(); - if (gpr_stricmp(resolver_type, "ares") == 0) { - // This behavior expectation is specific to c-ares. - 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 = - static_cast( - gpr_malloc(sizeof(mock_ipv6_disabled_source_addr_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; + const auto result = RUN_ALL_TESTS(); + return result; } diff --git a/test/core/surface/server_test.cc b/test/core/surface/server_test.cc index 630d9f82d2a..1c267794767 100644 --- a/test/core/surface/server_test.cc +++ b/test/core/surface/server_test.cc @@ -127,15 +127,8 @@ void test_bind_server_to_addr(const char* host, bool secure) { grpc_completion_queue_destroy(cq); } -static int external_dns_works(const char* host) { - grpc_resolved_addresses* res = nullptr; - grpc_error_handle error = grpc_blocking_resolve_address(host, "80", &res); - GRPC_ERROR_UNREF(error); - if (res != nullptr) { - grpc_resolved_addresses_destroy(res); - return 1; - } - return 0; +static bool external_dns_works(const char* host) { + return grpc_core::GetDNSResolver()->ResolveNameBlocking(host, "80").ok(); } static void test_bind_server_to_addrs(const char** addrs, size_t n) { diff --git a/test/core/transport/chttp2/settings_timeout_test.cc b/test/core/transport/chttp2/settings_timeout_test.cc index b36c309aa2a..4fc8e70b31a 100644 --- a/test/core/transport/chttp2/settings_timeout_test.cc +++ b/test/core/transport/chttp2/settings_timeout_test.cc @@ -109,11 +109,11 @@ class Client { void Connect() { ExecCtx exec_ctx; - grpc_resolved_addresses* server_addresses = nullptr; - grpc_error_handle error = - grpc_blocking_resolve_address(server_address_, "80", &server_addresses); - ASSERT_EQ(GRPC_ERROR_NONE, error) << grpc_error_std_string(error); - ASSERT_GE(server_addresses->naddrs, 1UL); + absl::StatusOr> addresses_or = + GetDNSResolver()->ResolveNameBlocking(server_address_, "80"); + ASSERT_EQ(absl::OkStatus(), addresses_or.status()) + << addresses_or.status().ToString(); + ASSERT_GE(addresses_or->size(), 1UL); pollset_ = static_cast(gpr_zalloc(grpc_pollset_size())); grpc_pollset_init(pollset_, &mu_); grpc_pollset_set* pollset_set = grpc_pollset_set_create(); @@ -123,8 +123,7 @@ class Client { .channel_args_preconditioning() .PreconditionChannelArgs(nullptr); grpc_tcp_client_connect(state.closure(), &endpoint_, pollset_set, args, - server_addresses->addrs, - ExecCtx::Get()->Now() + 1000); + addresses_or->data(), ExecCtx::Get()->Now() + 1000); grpc_channel_args_destroy(args); ASSERT_TRUE(PollUntilDone( &state, @@ -132,7 +131,6 @@ class Client { ASSERT_EQ(GRPC_ERROR_NONE, state.error()); grpc_pollset_set_destroy(pollset_set); grpc_endpoint_add_to_pollset(endpoint_, pollset_); - grpc_resolved_addresses_destroy(server_addresses); } // Reads until an error is returned. diff --git a/test/core/util/resolve_localhost_ip46.cc b/test/core/util/resolve_localhost_ip46.cc index bf3f621c173..bcfbd077671 100644 --- a/test/core/util/resolve_localhost_ip46.cc +++ b/test/core/util/resolve_localhost_ip46.cc @@ -32,20 +32,18 @@ bool localhost_to_ipv6 = false; gpr_once g_resolve_localhost_ipv46 = GPR_ONCE_INIT; void InitResolveLocalhost() { - grpc_resolved_addresses* addresses; - grpc_error_handle err = - grpc_blocking_resolve_address("localhost", "https", &addresses); - GPR_ASSERT(err == GRPC_ERROR_NONE); - for (size_t i = 0; i < addresses->naddrs; i++) { - grpc_sockaddr* addr = - reinterpret_cast(addresses->addrs[i].addr); - if (addr->sa_family == GRPC_AF_INET) { + absl::StatusOr> addresses_or = + GetDNSResolver()->ResolveNameBlocking("localhost", "https"); + GPR_ASSERT(addresses_or.ok()); + for (const auto& addr : *addresses_or) { + const grpc_sockaddr* sock_addr = + reinterpret_cast(&addr); + if (sock_addr->sa_family == GRPC_AF_INET) { localhost_to_ipv4 = true; - } else if (addr->sa_family == GRPC_AF_INET6) { + } else if (sock_addr->sa_family == GRPC_AF_INET6) { localhost_to_ipv6 = true; } } - grpc_resolved_addresses_destroy(addresses); } } // namespace diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index d745d23a2ff..2f156d5c3bd 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -1925,6 +1925,7 @@ src/core/lib/iomgr/event_engine/promise.h \ src/core/lib/iomgr/event_engine/resolved_address_internal.cc \ src/core/lib/iomgr/event_engine/resolved_address_internal.h \ src/core/lib/iomgr/event_engine/resolver.cc \ +src/core/lib/iomgr/event_engine/resolver.h \ src/core/lib/iomgr/event_engine/tcp.cc \ src/core/lib/iomgr/event_engine/timer.cc \ src/core/lib/iomgr/exec_ctx.cc \ @@ -1984,8 +1985,11 @@ src/core/lib/iomgr/resolve_address.cc \ src/core/lib/iomgr/resolve_address.h \ src/core/lib/iomgr/resolve_address_custom.cc \ src/core/lib/iomgr/resolve_address_custom.h \ +src/core/lib/iomgr/resolve_address_impl.h \ src/core/lib/iomgr/resolve_address_posix.cc \ +src/core/lib/iomgr/resolve_address_posix.h \ src/core/lib/iomgr/resolve_address_windows.cc \ +src/core/lib/iomgr/resolve_address_windows.h \ src/core/lib/iomgr/sockaddr.h \ src/core/lib/iomgr/sockaddr_posix.h \ src/core/lib/iomgr/sockaddr_windows.h \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index a4ec7f7eb43..6b47fe4e0ee 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -1724,6 +1724,7 @@ src/core/lib/iomgr/event_engine/promise.h \ src/core/lib/iomgr/event_engine/resolved_address_internal.cc \ src/core/lib/iomgr/event_engine/resolved_address_internal.h \ src/core/lib/iomgr/event_engine/resolver.cc \ +src/core/lib/iomgr/event_engine/resolver.h \ src/core/lib/iomgr/event_engine/tcp.cc \ src/core/lib/iomgr/event_engine/timer.cc \ src/core/lib/iomgr/exec_ctx.cc \ @@ -1783,8 +1784,11 @@ src/core/lib/iomgr/resolve_address.cc \ src/core/lib/iomgr/resolve_address.h \ src/core/lib/iomgr/resolve_address_custom.cc \ src/core/lib/iomgr/resolve_address_custom.h \ +src/core/lib/iomgr/resolve_address_impl.h \ src/core/lib/iomgr/resolve_address_posix.cc \ +src/core/lib/iomgr/resolve_address_posix.h \ src/core/lib/iomgr/resolve_address_windows.cc \ +src/core/lib/iomgr/resolve_address_windows.h \ src/core/lib/iomgr/sockaddr.h \ src/core/lib/iomgr/sockaddr_posix.h \ src/core/lib/iomgr/sockaddr_windows.h \ diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index a17bc8f8e1d..3e8c641d75d 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -2009,32 +2009,6 @@ ], "uses_polling": true }, - { - "args": [ - "--resolver=ares" - ], - "benchmark": false, - "ci_platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "gtest": false, - "language": "c", - "name": "resolve_address_using_ares_resolver_test", - "platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "uses_polling": true - }, { "args": [ "--resolver=native" @@ -2059,32 +2033,6 @@ ], "uses_polling": true }, - { - "args": [ - "--resolver=native" - ], - "benchmark": false, - "ci_platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "gtest": false, - "language": "c", - "name": "resolve_address_using_native_resolver_test", - "platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "uses_polling": true - }, { "args": [], "benchmark": false, @@ -6009,6 +5957,54 @@ ], "uses_polling": true }, + { + "args": [], + "benchmark": false, + "ci_platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "gtest": true, + "language": "c++", + "name": "resolve_address_using_ares_resolver_test", + "platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "uses_polling": true + }, + { + "args": [], + "benchmark": false, + "ci_platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "gtest": true, + "language": "c++", + "name": "resolve_address_using_native_resolver_test", + "platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "uses_polling": true + }, { "args": [], "benchmark": false,