From 49164c8aebb464b39c110832b8afbdf8172fea01 Mon Sep 17 00:00:00 2001 From: Adam Lemmon Date: Fri, 15 Mar 2019 09:45:30 -0600 Subject: [PATCH 01/30] Update Channel.cs Simple spelling or wording fixes to make comments read more cleanly --- src/csharp/Grpc.Core/Channel.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/csharp/Grpc.Core/Channel.cs b/src/csharp/Grpc.Core/Channel.cs index 7ce929dfa31..97f79b0fb4f 100644 --- a/src/csharp/Grpc.Core/Channel.cs +++ b/src/csharp/Grpc.Core/Channel.cs @@ -59,7 +59,7 @@ namespace Grpc.Core /// /// Creates a channel that connects to a specific host. - /// Port will default to 80 for an unsecure channel and to 443 for a secure channel. + /// Port will default to 80 for an unsecure channel or to 443 for a secure channel. /// /// Target of the channel. /// Credentials to secure the channel. @@ -112,7 +112,7 @@ namespace Grpc.Core /// /// Gets current connectivity state of this channel. - /// After channel is has been shutdown, ChannelState.Shutdown will be returned. + /// After channel has been shutdown, ChannelState.Shutdown will be returned. /// public ChannelState State { @@ -132,7 +132,7 @@ namespace Grpc.Core /// /// Returned tasks completes once channel state has become different from /// given lastObservedState. - /// If deadline is reached or and error occurs, returned task is cancelled. + /// If deadline is reached or an error occurs, returned task is cancelled. /// public async Task WaitForStateChangedAsync(ChannelState lastObservedState, DateTime? deadline = null) { From 3954b3ef5dd62ea67611715db03f568940ff48fd Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Wed, 20 Mar 2019 17:37:36 -0700 Subject: [PATCH 02/30] Remove unnecessary else condition --- src/core/lib/gprpp/fork.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/core/lib/gprpp/fork.cc b/src/core/lib/gprpp/fork.cc index 3b9c16510a7..c4b1cbc2233 100644 --- a/src/core/lib/gprpp/fork.cc +++ b/src/core/lib/gprpp/fork.cc @@ -160,8 +160,6 @@ void Fork::GlobalInit() { if (!override_enabled_) { #ifdef GRPC_ENABLE_FORK_SUPPORT support_enabled_ = true; -#else - support_enabled_ = false; #endif bool env_var_set = false; char* env = gpr_getenv("GRPC_ENABLE_FORK_SUPPORT"); From d93959853f45d7e0725030a3ca6bd9f341ba6255 Mon Sep 17 00:00:00 2001 From: Bill Feng Date: Thu, 21 Mar 2019 18:00:48 -0700 Subject: [PATCH 03/30] Enabled Windows Bazel build for cpp tests --- bazel/BUILD | 13 ++++++++ bazel/grpc_build_system.bzl | 31 ++++++++++++++++--- test/core/bad_connection/BUILD | 1 + test/core/client_channel/BUILD | 1 + test/core/end2end/generate_tests.bzl | 23 +++++++++++--- test/core/iomgr/BUILD | 10 ++++++ test/cpp/common/BUILD | 1 + test/cpp/end2end/BUILD | 2 ++ test/cpp/interop/BUILD | 1 + test/cpp/microbenchmarks/BUILD | 18 +++++++++++ .../generate_resolver_component_tests.bzl | 5 ++- test/cpp/performance/BUILD | 1 + test/cpp/qps/qps_benchmark_script.bzl | 1 + test/cpp/server/BUILD | 3 ++ test/cpp/server/load_reporter/BUILD | 1 + .../cpp/thread_manager/thread_manager_test.cc | 3 +- tools/remote_build/README.md | 6 ++++ tools/remote_build/windows.bazelrc | 3 ++ 18 files changed, 113 insertions(+), 11 deletions(-) create mode 100644 tools/remote_build/windows.bazelrc diff --git a/bazel/BUILD b/bazel/BUILD index 32402892cc3..4b581709f20 100644 --- a/bazel/BUILD +++ b/bazel/BUILD @@ -29,3 +29,16 @@ cc_grpc_library( proto_only = True, deps = [], ) + +environment(name = "windows_msvc") + +environment(name = "non_windows_msvc") + +environment_group( + name = "os", + defaults = [":non_windows_msvc"], + environments = [ + ":non_windows_msvc", + ":windows_msvc", + ], +) diff --git a/bazel/grpc_build_system.bzl b/bazel/grpc_build_system.bzl index 2a09022c64c..a2f3c200fa7 100644 --- a/bazel/grpc_build_system.bzl +++ b/bazel/grpc_build_system.bzl @@ -28,6 +28,12 @@ load("//bazel:cc_grpc_library.bzl", "cc_grpc_library") # The set of pollers to test against if a test exercises polling POLLERS = ["epollex", "epoll1", "poll"] +def is_msvc(): + return select({ + "//:windows_msvc": True, + "//conditions:default": False, + }) + def if_not_windows(a): return select({ "//:windows": [], @@ -80,7 +86,8 @@ def grpc_cc_library( visibility = None, alwayslink = 0, data = [], - use_cfstream = False): + use_cfstream = False, + tags = []): copts = [] if use_cfstream: copts = if_mac(["-DGRPC_CFSTREAM"]) @@ -117,6 +124,7 @@ def grpc_cc_library( ], alwayslink = alwayslink, data = data, + tags = tags, ) def grpc_proto_plugin(name, srcs = [], deps = []): @@ -159,9 +167,23 @@ def grpc_cc_test(name, srcs = [], deps = [], external_deps = [], args = [], data "size": size, "timeout": timeout, "exec_compatible_with": exec_compatible_with, + "tags": tags, } if uses_polling: - native.cc_test(testonly = True, tags = ["manual"], **args) + native.cc_test( + testonly = True, + tags = (["manual"] if not is_msvc() else tags), + name = name, + srcs = srcs, + args = args["args"], + data = data, + deps = deps + _get_external_deps(external_deps), + copts = copts, + linkopts = if_not_windows(["-pthread"]), + size = size, + timeout = timeout, + exec_compatible_with = exec_compatible_with, + ) for poller in POLLERS: native.sh_test( name = name + "@poller=" + poller, @@ -175,13 +197,13 @@ def grpc_cc_test(name, srcs = [], deps = [], external_deps = [], args = [], data poller, "$(location %s)" % name, ] + args["args"], - tags = tags, + tags = tags if is_msvc() else tags + ["no_windows"], exec_compatible_with = exec_compatible_with, ) else: native.cc_test(**args) -def grpc_cc_binary(name, srcs = [], deps = [], external_deps = [], args = [], data = [], language = "C++", testonly = False, linkshared = False, linkopts = []): +def grpc_cc_binary(name, srcs = [], deps = [], external_deps = [], args = [], data = [], language = "C++", testonly = False, linkshared = False, linkopts = [], tags = []): copts = [] if language.upper() == "C": copts = ["-std=c99"] @@ -195,6 +217,7 @@ def grpc_cc_binary(name, srcs = [], deps = [], external_deps = [], args = [], da deps = deps + _get_external_deps(external_deps), copts = copts, linkopts = if_not_windows(["-pthread"]) + linkopts, + tags = tags, ) def grpc_generate_one_off_targets(): diff --git a/test/core/bad_connection/BUILD b/test/core/bad_connection/BUILD index 8ada933e796..82b38ccc469 100644 --- a/test/core/bad_connection/BUILD +++ b/test/core/bad_connection/BUILD @@ -29,4 +29,5 @@ grpc_cc_binary( "//:grpc", "//test/core/util:grpc_test_util", ], + tags = ["no_windows"], ) diff --git a/test/core/client_channel/BUILD b/test/core/client_channel/BUILD index 57e5191af4c..68a71632daf 100644 --- a/test/core/client_channel/BUILD +++ b/test/core/client_channel/BUILD @@ -52,6 +52,7 @@ grpc_cc_test( "//:grpc", "//test/core/util:grpc_test_util", ], + tags = ["no_windows"], ) grpc_cc_test( diff --git a/test/core/end2end/generate_tests.bzl b/test/core/end2end/generate_tests.bzl index 79c079bcc73..8020780a909 100755 --- a/test/core/end2end/generate_tests.bzl +++ b/test/core/end2end/generate_tests.bzl @@ -15,7 +15,7 @@ """Generates the appropriate build.json data for all the end2end tests.""" -load("//bazel:grpc_build_system.bzl", "grpc_cc_binary", "grpc_cc_library") +load("//bazel:grpc_build_system.bzl", "grpc_cc_binary", "grpc_cc_library", "is_msvc") POLLERS = ["epollex", "epoll1", "poll"] @@ -31,7 +31,8 @@ def _fixture_options( is_http2 = True, supports_proxy_auth = False, supports_write_buffering = True, - client_channel = True): + client_channel = True, + supports_msvc = True): return struct( fullstack = fullstack, includes_proxy = includes_proxy, @@ -44,6 +45,7 @@ def _fixture_options( supports_proxy_auth = supports_proxy_auth, supports_write_buffering = supports_write_buffering, client_channel = client_channel, + supports_msvc = supports_msvc, #_platforms=_platforms, ) @@ -120,10 +122,11 @@ END2END_NOSEC_FIXTURES = { client_channel = False, secure = False, _platforms = ["linux", "mac", "posix"], + supports_msvc = False, ), "h2_full": _fixture_options(secure = False), - "h2_full+pipe": _fixture_options(secure = False, _platforms = ["linux"]), - "h2_full+trace": _fixture_options(secure = False, tracing = True), + "h2_full+pipe": _fixture_options(secure = False, _platforms = ["linux"], supports_msvc = False), + "h2_full+trace": _fixture_options(secure = False, tracing = True, supports_msvc = False), "h2_full+workarounds": _fixture_options(secure = False), "h2_http_proxy": _fixture_options(secure = False, supports_proxy_auth = True), "h2_proxy": _fixture_options(secure = False, includes_proxy = True), @@ -152,6 +155,7 @@ END2END_NOSEC_FIXTURES = { dns_resolver = False, _platforms = ["linux", "mac", "posix"], secure = False, + supports_msvc = False, ), } @@ -453,6 +457,16 @@ def grpc_end2end_nosec_tests(): continue if topt.secure: continue + native.sh_test( + name = "%s_nosec_test@%s" % (f, t), + data = [":%s_nosec_test" % f], + srcs = ["end2end_test.sh"], + args = [ + "$(location %s_nosec_test)" % f, + t, + ], + tags = ["no_windows"], + ) for poller in POLLERS: native.sh_test( name = "%s_nosec_test@%s@poller=%s" % (f, t, poller), @@ -463,4 +477,5 @@ def grpc_end2end_nosec_tests(): t, poller, ], + tags = ["no_windows"], ) diff --git a/test/core/iomgr/BUILD b/test/core/iomgr/BUILD index 5e4338aee37..1aefa0ab224 100644 --- a/test/core/iomgr/BUILD +++ b/test/core/iomgr/BUILD @@ -81,6 +81,7 @@ grpc_cc_test( "//:grpc", "//test/core/util:grpc_test_util", ], + tags = ["no_windows"], ) grpc_cc_test( @@ -92,6 +93,7 @@ grpc_cc_test( "//:grpc", "//test/core/util:grpc_test_util", ], + tags = ["no_windows"], ) grpc_cc_test( @@ -103,6 +105,7 @@ grpc_cc_test( "//:grpc", "//test/core/util:grpc_test_util", ], + tags = ["no_windows"], ) grpc_cc_test( @@ -139,6 +142,7 @@ grpc_cc_test( "//:grpc", "//test/core/util:grpc_test_util", ], + tags = ["no_windows"], ) grpc_cc_test( @@ -153,6 +157,7 @@ grpc_cc_test( "//:grpc", "//test/core/util:grpc_test_util", ], + tags = ["no_windows"], ) grpc_cc_test( @@ -214,6 +219,7 @@ grpc_cc_test( "//:grpc", "//test/core/util:grpc_test_util", ], + tags = ["no_windows"], ) grpc_cc_test( @@ -225,6 +231,7 @@ grpc_cc_test( "//:grpc", "//test/core/util:grpc_test_util", ], + tags = ["no_windows"], ) grpc_cc_test( @@ -237,6 +244,7 @@ grpc_cc_test( "//:grpc", "//test/core/util:grpc_test_util", ], + tags = ["no_windows"], ) grpc_cc_test( @@ -259,6 +267,7 @@ grpc_cc_test( "//:grpc", "//test/core/util:grpc_test_util", ], + tags = ["no_windows"], ) grpc_cc_test( @@ -303,4 +312,5 @@ grpc_cc_test( "//:grpc", "//test/core/util:grpc_test_util", ], + tags = ["no_windows"], ) diff --git a/test/cpp/common/BUILD b/test/cpp/common/BUILD index 01699b26add..b67c1995ff7 100644 --- a/test/cpp/common/BUILD +++ b/test/cpp/common/BUILD @@ -28,6 +28,7 @@ grpc_cc_test( "//:grpc++_unsecure", "//test/core/util:grpc_test_util_unsecure", ], + tags = ["no_windows"], ) grpc_cc_test( diff --git a/test/cpp/end2end/BUILD b/test/cpp/end2end/BUILD index 998ba8e1e4e..707a628148e 100644 --- a/test/cpp/end2end/BUILD +++ b/test/cpp/end2end/BUILD @@ -99,6 +99,7 @@ grpc_cc_test( "//test/core/util:grpc_test_util", "//test/cpp/util:test_util", ], + tags = ["no_windows"], ) grpc_cc_test( @@ -633,6 +634,7 @@ grpc_cc_test( "//test/core/util:grpc_test_util", "//test/cpp/util:test_util", ], + tags = ["no_windows"], ) grpc_cc_test( diff --git a/test/cpp/interop/BUILD b/test/cpp/interop/BUILD index f36494d98db..6cf4719c17b 100644 --- a/test/cpp/interop/BUILD +++ b/test/cpp/interop/BUILD @@ -161,4 +161,5 @@ grpc_cc_test( "//test/cpp/util:test_config", "//test/cpp/util:test_util", ], + tags = ["no_windows"], ) diff --git a/test/cpp/microbenchmarks/BUILD b/test/cpp/microbenchmarks/BUILD index 70b4000780c..6e844a6dc62 100644 --- a/test/cpp/microbenchmarks/BUILD +++ b/test/cpp/microbenchmarks/BUILD @@ -45,6 +45,7 @@ grpc_cc_library( "//test/core/util:grpc_test_util_unsecure", "//test/cpp/util:test_config", ], + tags = ["no_windows"], ) grpc_cc_binary( @@ -52,6 +53,7 @@ grpc_cc_binary( testonly = 1, srcs = ["bm_closure.cc"], deps = [":helpers"], + tags = ["no_windows"], ) grpc_cc_binary( @@ -59,6 +61,7 @@ grpc_cc_binary( testonly = 1, srcs = ["bm_alarm.cc"], deps = [":helpers"], + tags = ["no_windows"], ) grpc_cc_binary( @@ -66,6 +69,7 @@ grpc_cc_binary( testonly = 1, srcs = ["bm_arena.cc"], deps = [":helpers"], + tags = ["no_windows"], ) grpc_cc_binary( @@ -73,6 +77,7 @@ grpc_cc_binary( testonly = 1, srcs = ["bm_byte_buffer.cc"], deps = [":helpers"], + tags = ["no_windows"], ) grpc_cc_binary( @@ -80,6 +85,7 @@ grpc_cc_binary( testonly = 1, srcs = ["bm_channel.cc"], deps = [":helpers"], + tags = ["no_windows"], ) grpc_cc_binary( @@ -87,6 +93,7 @@ grpc_cc_binary( testonly = 1, srcs = ["bm_call_create.cc"], deps = [":helpers"], + tags = ["no_windows"], ) grpc_cc_binary( @@ -94,6 +101,7 @@ grpc_cc_binary( testonly = 1, srcs = ["bm_cq.cc"], deps = [":helpers"], + tags = ["no_windows"], ) grpc_cc_binary( @@ -101,6 +109,7 @@ grpc_cc_binary( testonly = 1, srcs = ["bm_cq_multiple_threads.cc"], deps = [":helpers"], + tags = ["no_windows"], ) grpc_cc_binary( @@ -108,6 +117,7 @@ grpc_cc_binary( testonly = 1, srcs = ["bm_error.cc"], deps = [":helpers"], + tags = ["no_windows"], ) grpc_cc_library( @@ -117,6 +127,7 @@ grpc_cc_library( "fullstack_streaming_ping_pong.h", ], deps = [":helpers"], + tags = ["no_windows"], ) grpc_cc_binary( @@ -126,6 +137,7 @@ grpc_cc_binary( "bm_fullstack_streaming_ping_pong.cc", ], deps = [":fullstack_streaming_ping_pong_h"], + tags = ["no_windows"], ) grpc_cc_library( @@ -144,6 +156,7 @@ grpc_cc_binary( "bm_fullstack_streaming_pump.cc", ], deps = [":fullstack_streaming_pump_h"], + tags = ["no_windows"], ) grpc_cc_binary( @@ -151,6 +164,7 @@ grpc_cc_binary( testonly = 1, srcs = ["bm_fullstack_trickle.cc"], deps = [":helpers"], + tags = ["no_windows"], ) grpc_cc_library( @@ -169,6 +183,7 @@ grpc_cc_binary( "bm_fullstack_unary_ping_pong.cc", ], deps = [":fullstack_unary_ping_pong_h"], + tags = ["no_windows"], ) grpc_cc_binary( @@ -176,6 +191,7 @@ grpc_cc_binary( testonly = 1, srcs = ["bm_metadata.cc"], deps = [":helpers"], + tags = ["no_windows"], ) grpc_cc_binary( @@ -183,6 +199,7 @@ grpc_cc_binary( testonly = 1, srcs = ["bm_chttp2_hpack.cc"], deps = [":helpers"], + tags = ["no_windows"], ) grpc_cc_binary( @@ -202,4 +219,5 @@ grpc_cc_binary( testonly = 1, srcs = ["bm_timer.cc"], deps = [":helpers"], + tags = ["no_windows"], ) diff --git a/test/cpp/naming/generate_resolver_component_tests.bzl b/test/cpp/naming/generate_resolver_component_tests.bzl index f36021560c1..589176762e6 100755 --- a/test/cpp/naming/generate_resolver_component_tests.bzl +++ b/test/cpp/naming/generate_resolver_component_tests.bzl @@ -33,6 +33,7 @@ def generate_resolver_component_tests(): "//:gpr", "//test/cpp/util:test_config", ], + tags = ["no_windows"], ) # meant to be invoked only through the top-level shell script driver grpc_cc_binary( @@ -52,6 +53,7 @@ def generate_resolver_component_tests(): "//:gpr", "//test/cpp/util:test_config", ], + tags = ["no_windows"], ) grpc_cc_test( name = "resolver_component_tests_runner_invoker%s" % unsecure_build_config_suffix, @@ -77,5 +79,6 @@ def generate_resolver_component_tests(): args = [ "--test_bin_name=resolver_component_test%s" % unsecure_build_config_suffix, "--running_under_bazel=true", - ] + ], + tags = ["no_windows"], ) diff --git a/test/cpp/performance/BUILD b/test/cpp/performance/BUILD index 4fe95d5905e..6068c33f95f 100644 --- a/test/cpp/performance/BUILD +++ b/test/cpp/performance/BUILD @@ -31,4 +31,5 @@ grpc_cc_test( "//src/proto/grpc/testing:echo_proto", "//test/core/util:grpc_test_util_base", ], + tags = ["no_windows"], ) diff --git a/test/cpp/qps/qps_benchmark_script.bzl b/test/cpp/qps/qps_benchmark_script.bzl index 855caa0d37c..b4767ec8e09 100644 --- a/test/cpp/qps/qps_benchmark_script.bzl +++ b/test/cpp/qps/qps_benchmark_script.bzl @@ -75,5 +75,6 @@ def json_run_localhost_batch(): ], tags = [ "json_run_localhost", + "no_windows", ], ) diff --git a/test/cpp/server/BUILD b/test/cpp/server/BUILD index 050b83f5c4f..a4811031691 100644 --- a/test/cpp/server/BUILD +++ b/test/cpp/server/BUILD @@ -29,6 +29,7 @@ grpc_cc_test( "//src/proto/grpc/testing:echo_proto", "//test/core/util:grpc_test_util_unsecure", ], + tags = ["no_windows"], ) grpc_cc_test( @@ -42,6 +43,7 @@ grpc_cc_test( "//src/proto/grpc/testing:echo_proto", "//test/core/util:grpc_test_util_unsecure", ], + tags = ["no_windows"], ) grpc_cc_test( @@ -55,4 +57,5 @@ grpc_cc_test( "//src/proto/grpc/testing:echo_proto", "//test/core/util:grpc_test_util_unsecure", ], + tags = ["no_windows"], ) diff --git a/test/cpp/server/load_reporter/BUILD b/test/cpp/server/load_reporter/BUILD index 8d876c56d29..db5c93263ad 100644 --- a/test/cpp/server/load_reporter/BUILD +++ b/test/cpp/server/load_reporter/BUILD @@ -45,6 +45,7 @@ grpc_cc_test( "//:lb_server_load_reporting_filter", "//test/core/util:grpc_test_util", ], + tags = ["no_windows"], ) grpc_cc_test( diff --git a/test/cpp/thread_manager/thread_manager_test.cc b/test/cpp/thread_manager/thread_manager_test.cc index 99de5a3e010..ee7252bf78d 100644 --- a/test/cpp/thread_manager/thread_manager_test.cc +++ b/test/cpp/thread_manager/thread_manager_test.cc @@ -21,13 +21,12 @@ #include #include -#include #include #include #include -#include "src/cpp/thread_manager/thread_manager.h" #include "test/cpp/util/test_config.h" +#include "src/cpp/thread_manager/thread_manager.h" namespace grpc { diff --git a/tools/remote_build/README.md b/tools/remote_build/README.md index 19739e9ee12..8a236973946 100644 --- a/tools/remote_build/README.md +++ b/tools/remote_build/README.md @@ -29,5 +29,11 @@ Sanitizer runs (asan, msan, tsan, ubsan): bazel --bazelrc=tools/remote_build/manual.bazelrc test --config=asan //test/... ``` +Run on Windows MSVC: +``` +# local manual run only for C++ targets (RBE to be supported) +bazel --bazelrc=tools/remote_build/windows.bazelrc test //test/cpp/... +``` + Available command line options can be found in [Bazel command line reference](https://docs.bazel.build/versions/master/command-line-reference.html) diff --git a/tools/remote_build/windows.bazelrc b/tools/remote_build/windows.bazelrc new file mode 100644 index 00000000000..70575372d02 --- /dev/null +++ b/tools/remote_build/windows.bazelrc @@ -0,0 +1,3 @@ +# TODO(yfen): Merge with rbe_common.bazelrc and enable Windows RBE +build --test_tag_filters=-no_windows +build --build_tag_filters=-no_windows From 61431be32b89b6b1857b525b01b66e6aaede7018 Mon Sep 17 00:00:00 2001 From: billfeng327 Date: Fri, 22 Mar 2019 10:22:59 -0700 Subject: [PATCH 04/30] skipping upb on windows --- BUILD | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/BUILD b/BUILD index 9e052dcf0c2..41ab64193ae 100644 --- a/BUILD +++ b/BUILD @@ -2334,7 +2334,8 @@ grpc_cc_library( ":google_api_upb", ":proto_gen_validate_upb", ":envoy_type_upb", - ] + ], + tags = ["no_windows"], ) grpc_cc_library( @@ -2354,7 +2355,8 @@ grpc_cc_library( deps = [ ":google_api_upb", ":proto_gen_validate_upb" - ] + ], + tags = ["no_windows"], ) grpc_cc_library( @@ -2373,7 +2375,8 @@ grpc_cc_library( ], deps = [ ":google_api_upb", - ] + ], + tags = ["no_windows"], ) grpc_cc_library( @@ -2404,6 +2407,7 @@ grpc_cc_library( external_deps = [ "upb_lib", ], + tags = ["no_windows"], ) grpc_generate_one_off_targets() From 1a09899d3e5c8f7e489a25d92ff0e5f6ae00ec81 Mon Sep 17 00:00:00 2001 From: Bill Feng Date: Fri, 22 Mar 2019 11:18:39 -0700 Subject: [PATCH 05/30] reverted change on thread manager test --- test/cpp/thread_manager/thread_manager_test.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/cpp/thread_manager/thread_manager_test.cc b/test/cpp/thread_manager/thread_manager_test.cc index ee7252bf78d..99de5a3e010 100644 --- a/test/cpp/thread_manager/thread_manager_test.cc +++ b/test/cpp/thread_manager/thread_manager_test.cc @@ -21,12 +21,13 @@ #include #include +#include #include #include #include -#include "test/cpp/util/test_config.h" #include "src/cpp/thread_manager/thread_manager.h" +#include "test/cpp/util/test_config.h" namespace grpc { From d7e5cb70febcdc3f0ec267e2d5b01dc3e152c1d6 Mon Sep 17 00:00:00 2001 From: Bill Feng Date: Fri, 22 Mar 2019 11:27:24 -0700 Subject: [PATCH 06/30] flipped logic for is_msvc() --- bazel/grpc_build_system.bzl | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/bazel/grpc_build_system.bzl b/bazel/grpc_build_system.bzl index a2f3c200fa7..ca87428f375 100644 --- a/bazel/grpc_build_system.bzl +++ b/bazel/grpc_build_system.bzl @@ -167,22 +167,12 @@ def grpc_cc_test(name, srcs = [], deps = [], external_deps = [], args = [], data "size": size, "timeout": timeout, "exec_compatible_with": exec_compatible_with, - "tags": tags, } if uses_polling: native.cc_test( testonly = True, - tags = (["manual"] if not is_msvc() else tags), - name = name, - srcs = srcs, - args = args["args"], - data = data, - deps = deps + _get_external_deps(external_deps), - copts = copts, - linkopts = if_not_windows(["-pthread"]), - size = size, - timeout = timeout, - exec_compatible_with = exec_compatible_with, + tags = (["manual"] if is_msvc() else tags), + **args ) for poller in POLLERS: native.sh_test( @@ -197,11 +187,11 @@ def grpc_cc_test(name, srcs = [], deps = [], external_deps = [], args = [], data poller, "$(location %s)" % name, ] + args["args"], - tags = tags if is_msvc() else tags + ["no_windows"], + tags = (tags + ["no_windows"]) if is_msvc() else tags, exec_compatible_with = exec_compatible_with, ) else: - native.cc_test(**args) + native.cc_test(tags = tags, **args) def grpc_cc_binary(name, srcs = [], deps = [], external_deps = [], args = [], data = [], language = "C++", testonly = False, linkshared = False, linkopts = [], tags = []): copts = [] From d16597ab73708b683d6d3b37f0213d343678865b Mon Sep 17 00:00:00 2001 From: billfeng327 Date: Fri, 22 Mar 2019 14:33:37 -0700 Subject: [PATCH 07/30] corrected Bazel target generation logic for Windows --- bazel/grpc_build_system.bzl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bazel/grpc_build_system.bzl b/bazel/grpc_build_system.bzl index ca87428f375..c5eb9ec8bf3 100644 --- a/bazel/grpc_build_system.bzl +++ b/bazel/grpc_build_system.bzl @@ -169,9 +169,11 @@ def grpc_cc_test(name, srcs = [], deps = [], external_deps = [], args = [], data "exec_compatible_with": exec_compatible_with, } if uses_polling: + # Only run targets with pollers for non-MSVC + # Only run targets without pollers for MSVC native.cc_test( testonly = True, - tags = (["manual"] if is_msvc() else tags), + tags = tags if is_msvc() else ["manual"], **args ) for poller in POLLERS: From 30cc99d42cedece1362e9b0c519aecda3116c984 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Mon, 25 Mar 2019 10:43:10 -0700 Subject: [PATCH 08/30] Add Python example for error handling --- examples/python/errors/README.md | 96 +++++++++++++ .../python/errors/error_handling_client.py | 50 +++++++ .../python/errors/error_handling_server.py | 81 +++++++++++ examples/python/errors/helloworld_pb2.py | 134 ++++++++++++++++++ examples/python/errors/helloworld_pb2_grpc.py | 46 ++++++ 5 files changed, 407 insertions(+) create mode 100644 examples/python/errors/README.md create mode 100644 examples/python/errors/error_handling_client.py create mode 100644 examples/python/errors/error_handling_server.py create mode 100644 examples/python/errors/helloworld_pb2.py create mode 100644 examples/python/errors/helloworld_pb2_grpc.py diff --git a/examples/python/errors/README.md b/examples/python/errors/README.md new file mode 100644 index 00000000000..226f0e4a281 --- /dev/null +++ b/examples/python/errors/README.md @@ -0,0 +1,96 @@ +# gRPC Python Error Handling Example + +The goal of this example is sending error status from server that is more complicated than a code and detail string. + +The definition for an RPC method in proto files contains request message and response message. There are many error states that can be shared across RPC methods (e.g. stack trace, insufficient quota). Using a different path to handle error will make the code more maintainable. + +Ideally, the final status of an RPC should be described in the trailing headers of HTTP2, and gRPC Python provides helper functions in `grpcio-status` package to assist the packing and unpacking of error status. + +### Definition of Status Proto + +```proto +// The `Status` type defines a logical error model that is suitable for different +// programming environments, including REST APIs and RPC APIs. It is used by +// [gRPC](https://github.com/grpc). The error model is designed to be: +// +// - Simple to use and understand for most users +// - Flexible enough to meet unexpected needs +// +// # Overview +// +// The `Status` message contains three pieces of data: error code, error message, +// and error details. The error code should be an enum value of +// [google.rpc.Code][google.rpc.Code], but it may accept additional error codes if needed. The +// error message should be a developer-facing English message that helps +// developers *understand* and *resolve* the error. If a localized user-facing +// error message is needed, put the localized message in the error details or +// localize it in the client. The optional error details may contain arbitrary +// information about the error. There is a predefined set of error detail types +// in the package `google.rpc` that can be used for common error conditions. +// +// # Language mapping +// +// The `Status` message is the logical representation of the error model, but it +// is not necessarily the actual wire format. When the `Status` message is +// exposed in different client libraries and different wire protocols, it can be +// mapped differently. For example, it will likely be mapped to some exceptions +// in Java, but more likely mapped to some error codes in C. +// +// # Other uses +// +// The error model and the `Status` message can be used in a variety of +// environments, either with or without APIs, to provide a +// consistent developer experience across different environments. +// +// Example uses of this error model include: +// +// - Partial errors. If a service needs to return partial errors to the client, +// it may embed the `Status` in the normal response to indicate the partial +// errors. +// +// - Workflow errors. A typical workflow has multiple steps. Each step may +// have a `Status` message for error reporting. +// +// - Batch operations. If a client uses batch request and batch response, the +// `Status` message should be used directly inside batch response, one for +// each error sub-response. +// +// - Asynchronous operations. If an API call embeds asynchronous operation +// results in its response, the status of those operations should be +// represented directly using the `Status` message. +// +// - Logging. If some API errors are stored in logs, the message `Status` could +// be used directly after any stripping needed for security/privacy reasons. +message Status { + // The status code, which should be an enum value of [google.rpc.Code][google.rpc.Code]. + int32 code = 1; + + // A developer-facing error message, which should be in English. Any + // user-facing error message should be localized and sent in the + // [google.rpc.Status.details][google.rpc.Status.details] field, or localized by the client. + string message = 2; + + // A list of messages that carry the error details. There is a common set of + // message types for APIs to use. + repeated google.protobuf.Any details = 3; +} +``` + +### Usage of Well-Known-Proto `Any` + +Please check [ProtoBuf Document: Any](https://developers.google.com/protocol-buffers/docs/reference/python-generated#any) + +```Python +any_message.Pack(message) +any_message.Unpack(message) +assert any_message.Is(message.DESCRIPTOR) +``` + +### Common Protos + +These protos are defined by Google Cloud API. Most error cases are covered in `error_dettails.proto`, but you may provide any custom error detail proto you want. + +Please refer to: +* [code.proto](https://github.com/googleapis/api-common-protos/blob/master/google/rpc/code.proto). +* [status.proto](https://github.com/googleapis/api-common-protos/blob/master/google/rpc/status.proto). +* [error_details.proto](https://github.com/googleapis/api-common-protos/blob/master/google/rpc/error_details.proto). diff --git a/examples/python/errors/error_handling_client.py b/examples/python/errors/error_handling_client.py new file mode 100644 index 00000000000..22a865a89bf --- /dev/null +++ b/examples/python/errors/error_handling_client.py @@ -0,0 +1,50 @@ +# Copyright 2019 The 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. +"""This example handles rich error status in client-side.""" + +from __future__ import print_function +import logging + +import grpc +from grpc_status import rpc_status +from google.rpc import error_details_pb2 + +import helloworld_pb2 +import helloworld_pb2_grpc + + +def run(): + # NOTE(gRPC Python Team): .close() is possible on a channel and should be + # used in circumstances in which the with statement does not fit the needs + # of the code. + with grpc.insecure_channel('localhost:50051') as channel: + stub = helloworld_pb2_grpc.GreeterStub(channel) + try: + response = stub.SayHello(helloworld_pb2.HelloRequest(name='Alice')) + print('Call success:', response.message) + except grpc.RpcError as rpc_error: + print('Call failure:', rpc_error) + status = rpc_status.from_call(rpc_error) + for detail in status.details: + if detail.Is(error_details_pb2.QuotaFailure.DESCRIPTOR): + info = error_details_pb2.QuotaFailure() + detail.Unpack(info) + print('Quota failure:', info) + else: + print('Unexpected failure:', detail) + + +if __name__ == '__main__': + logging.basicConfig() + run() diff --git a/examples/python/errors/error_handling_server.py b/examples/python/errors/error_handling_server.py new file mode 100644 index 00000000000..c782f7d1a2d --- /dev/null +++ b/examples/python/errors/error_handling_server.py @@ -0,0 +1,81 @@ +# Copyright 2019 The 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. +"""This example sends out rich error status from server-side.""" + +from concurrent import futures +import time +import logging +import threading + +import grpc +from grpc_status import rpc_status + +from google.protobuf import any_pb2 +from google.rpc import code_pb2, status_pb2, error_details_pb2 + +import helloworld_pb2 +import helloworld_pb2_grpc + +_ONE_DAY_IN_SECONDS = 60 * 60 * 24 + + +def create_greet_limit_exceed_error_status(name): + detail = any_pb2.Any() + detail.Pack( + error_details_pb2.QuotaFailure( + violations=[ + error_details_pb2.QuotaFailure.Violation( + subject="name:%s" % name, + description="Limit one greeting per person", + ) + ],)) + return status_pb2.Status( + code=code_pb2.RESOURCE_EXHAUSTED, + message='Request limit exceeded.', + details=[detail], + ) + + +class LimitedGreeter(helloworld_pb2_grpc.GreeterServicer): + + def __init__(self): + self._lock = threading.RLock() + self._greeted = set() + + def SayHello(self, request, context): + with self._lock: + if request.name in self._greeted: + rich_status = create_greet_limit_exceed_error_status( + request.name) + context.abort_with_status(rpc_status.to_status(rich_status)) + else: + self._greeted.add(request.name) + return helloworld_pb2.HelloReply(message='Hello, %s!' % request.name) + + +def serve(): + server = grpc.server(futures.ThreadPoolExecutor()) + helloworld_pb2_grpc.add_GreeterServicer_to_server(LimitedGreeter(), server) + server.add_insecure_port('[::]:50051') + server.start() + try: + while True: + time.sleep(_ONE_DAY_IN_SECONDS) + except KeyboardInterrupt: + server.stop(None) + + +if __name__ == '__main__': + logging.basicConfig() + serve() diff --git a/examples/python/errors/helloworld_pb2.py b/examples/python/errors/helloworld_pb2.py new file mode 100644 index 00000000000..e18ab9acc7a --- /dev/null +++ b/examples/python/errors/helloworld_pb2.py @@ -0,0 +1,134 @@ +# Generated by the protocol buffer compiler. DO NOT EDIT! +# source: helloworld.proto + +import sys +_b=sys.version_info[0]<3 and (lambda x:x) or (lambda x:x.encode('latin1')) +from google.protobuf import descriptor as _descriptor +from google.protobuf import message as _message +from google.protobuf import reflection as _reflection +from google.protobuf import symbol_database as _symbol_database +from google.protobuf import descriptor_pb2 +# @@protoc_insertion_point(imports) + +_sym_db = _symbol_database.Default() + + + + +DESCRIPTOR = _descriptor.FileDescriptor( + name='helloworld.proto', + package='helloworld', + syntax='proto3', + serialized_pb=_b('\n\x10helloworld.proto\x12\nhelloworld\"\x1c\n\x0cHelloRequest\x12\x0c\n\x04name\x18\x01 \x01(\t\"\x1d\n\nHelloReply\x12\x0f\n\x07message\x18\x01 \x01(\t2I\n\x07Greeter\x12>\n\x08SayHello\x12\x18.helloworld.HelloRequest\x1a\x16.helloworld.HelloReply\"\x00\x42\x36\n\x1bio.grpc.examples.helloworldB\x0fHelloWorldProtoP\x01\xa2\x02\x03HLWb\x06proto3') +) + + + + +_HELLOREQUEST = _descriptor.Descriptor( + name='HelloRequest', + full_name='helloworld.HelloRequest', + filename=None, + file=DESCRIPTOR, + containing_type=None, + fields=[ + _descriptor.FieldDescriptor( + name='name', full_name='helloworld.HelloRequest.name', index=0, + number=1, type=9, cpp_type=9, label=1, + has_default_value=False, default_value=_b("").decode('utf-8'), + message_type=None, enum_type=None, containing_type=None, + is_extension=False, extension_scope=None, + options=None), + ], + extensions=[ + ], + nested_types=[], + enum_types=[ + ], + options=None, + is_extendable=False, + syntax='proto3', + extension_ranges=[], + oneofs=[ + ], + serialized_start=32, + serialized_end=60, +) + + +_HELLOREPLY = _descriptor.Descriptor( + name='HelloReply', + full_name='helloworld.HelloReply', + filename=None, + file=DESCRIPTOR, + containing_type=None, + fields=[ + _descriptor.FieldDescriptor( + name='message', full_name='helloworld.HelloReply.message', index=0, + number=1, type=9, cpp_type=9, label=1, + has_default_value=False, default_value=_b("").decode('utf-8'), + message_type=None, enum_type=None, containing_type=None, + is_extension=False, extension_scope=None, + options=None), + ], + extensions=[ + ], + nested_types=[], + enum_types=[ + ], + options=None, + is_extendable=False, + syntax='proto3', + extension_ranges=[], + oneofs=[ + ], + serialized_start=62, + serialized_end=91, +) + +DESCRIPTOR.message_types_by_name['HelloRequest'] = _HELLOREQUEST +DESCRIPTOR.message_types_by_name['HelloReply'] = _HELLOREPLY +_sym_db.RegisterFileDescriptor(DESCRIPTOR) + +HelloRequest = _reflection.GeneratedProtocolMessageType('HelloRequest', (_message.Message,), dict( + DESCRIPTOR = _HELLOREQUEST, + __module__ = 'helloworld_pb2' + # @@protoc_insertion_point(class_scope:helloworld.HelloRequest) + )) +_sym_db.RegisterMessage(HelloRequest) + +HelloReply = _reflection.GeneratedProtocolMessageType('HelloReply', (_message.Message,), dict( + DESCRIPTOR = _HELLOREPLY, + __module__ = 'helloworld_pb2' + # @@protoc_insertion_point(class_scope:helloworld.HelloReply) + )) +_sym_db.RegisterMessage(HelloReply) + + +DESCRIPTOR.has_options = True +DESCRIPTOR._options = _descriptor._ParseOptions(descriptor_pb2.FileOptions(), _b('\n\033io.grpc.examples.helloworldB\017HelloWorldProtoP\001\242\002\003HLW')) + +_GREETER = _descriptor.ServiceDescriptor( + name='Greeter', + full_name='helloworld.Greeter', + file=DESCRIPTOR, + index=0, + options=None, + serialized_start=93, + serialized_end=166, + methods=[ + _descriptor.MethodDescriptor( + name='SayHello', + full_name='helloworld.Greeter.SayHello', + index=0, + containing_service=None, + input_type=_HELLOREQUEST, + output_type=_HELLOREPLY, + options=None, + ), +]) +_sym_db.RegisterServiceDescriptor(_GREETER) + +DESCRIPTOR.services_by_name['Greeter'] = _GREETER + +# @@protoc_insertion_point(module_scope) diff --git a/examples/python/errors/helloworld_pb2_grpc.py b/examples/python/errors/helloworld_pb2_grpc.py new file mode 100644 index 00000000000..18e07d16797 --- /dev/null +++ b/examples/python/errors/helloworld_pb2_grpc.py @@ -0,0 +1,46 @@ +# Generated by the gRPC Python protocol compiler plugin. DO NOT EDIT! +import grpc + +import helloworld_pb2 as helloworld__pb2 + + +class GreeterStub(object): + """The greeting service definition. + """ + + def __init__(self, channel): + """Constructor. + + Args: + channel: A grpc.Channel. + """ + self.SayHello = channel.unary_unary( + '/helloworld.Greeter/SayHello', + request_serializer=helloworld__pb2.HelloRequest.SerializeToString, + response_deserializer=helloworld__pb2.HelloReply.FromString, + ) + + +class GreeterServicer(object): + """The greeting service definition. + """ + + def SayHello(self, request, context): + """Sends a greeting + """ + context.set_code(grpc.StatusCode.UNIMPLEMENTED) + context.set_details('Method not implemented!') + raise NotImplementedError('Method not implemented!') + + +def add_GreeterServicer_to_server(servicer, server): + rpc_method_handlers = { + 'SayHello': grpc.unary_unary_rpc_method_handler( + servicer.SayHello, + request_deserializer=helloworld__pb2.HelloRequest.FromString, + response_serializer=helloworld__pb2.HelloReply.SerializeToString, + ), + } + generic_handler = grpc.method_handlers_generic_handler( + 'helloworld.Greeter', rpc_method_handlers) + server.add_generic_rpc_handlers((generic_handler,)) From f527cfbbac29a88e5a3a89a49a0338f5e7e9e256 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Mon, 25 Mar 2019 12:24:11 -0700 Subject: [PATCH 09/30] Adopt review's advice * Add a unit test * Integrate with Bazel * Polish README.md --- examples/protos/BUILD.bazel | 25 ++++ examples/python/errors/BUILD.bazel | 54 +++++++ examples/python/errors/README.md | 29 ++-- .../{error_handling_client.py => client.py} | 40 +++--- examples/python/errors/helloworld_pb2.py | 134 ------------------ examples/python/errors/helloworld_pb2_grpc.py | 46 ------ .../{error_handling_server.py => server.py} | 21 ++- .../test/_error_handling_example_test.py | 54 +++++++ src/python/grpcio_tests/tests/BUILD.bazel | 1 + 9 files changed, 192 insertions(+), 212 deletions(-) create mode 100644 examples/protos/BUILD.bazel create mode 100644 examples/python/errors/BUILD.bazel rename examples/python/errors/{error_handling_client.py => client.py} (57%) delete mode 100644 examples/python/errors/helloworld_pb2.py delete mode 100644 examples/python/errors/helloworld_pb2_grpc.py rename examples/python/errors/{error_handling_server.py => server.py} (86%) create mode 100644 examples/python/errors/test/_error_handling_example_test.py diff --git a/examples/protos/BUILD.bazel b/examples/protos/BUILD.bazel new file mode 100644 index 00000000000..197274e034c --- /dev/null +++ b/examples/protos/BUILD.bazel @@ -0,0 +1,25 @@ +# Copyright 2019 The 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. + +load("@grpc_python_dependencies//:requirements.bzl", "requirement") +load("@org_pubref_rules_protobuf//python:rules.bzl", "py_proto_library") + +package(default_visibility = ["//visibility:public"]) + +py_proto_library( + name = "py_helloworld_proto", + protos = ["helloworld.proto",], + with_grpc = True, + deps = [requirement('protobuf'),], +) diff --git a/examples/python/errors/BUILD.bazel b/examples/python/errors/BUILD.bazel new file mode 100644 index 00000000000..a2c3534f9bd --- /dev/null +++ b/examples/python/errors/BUILD.bazel @@ -0,0 +1,54 @@ +# Copyright 2019 The 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. + +load("@grpc_python_dependencies//:requirements.bzl", "requirement") +load("@org_pubref_rules_protobuf//python:rules.bzl", "py_proto_library") + +py_library( + name = "client", + testonly = 1, + srcs = ["client.py"], + deps = [ + "//src/python/grpcio/grpc:grpcio", + "//src/python/grpcio_status/grpc_status:grpc_status", + "//examples/protos:py_helloworld_proto", + requirement('googleapis-common-protos'), + ], +) + +py_library( + name = "server", + testonly = 1, + srcs = ["server.py"], + deps = [ + "//src/python/grpcio/grpc:grpcio", + "//src/python/grpcio_status/grpc_status:grpc_status", + "//examples/protos:py_helloworld_proto", + ] + select({ + "//conditions:default": [requirement("futures")], + "//:python3": [], + }), +) + +py_test( + name = "test/_error_handling_example_test", + srcs = ["test/_error_handling_example_test.py"], + data = [ + ":client", + ":server", + "//src/python/grpcio_tests/tests:bazel_namespace_package_hack", + ], + size = "small", + imports = ["../../../src/python/grpcio_status",], +) diff --git a/examples/python/errors/README.md b/examples/python/errors/README.md index 226f0e4a281..3c53690b99c 100644 --- a/examples/python/errors/README.md +++ b/examples/python/errors/README.md @@ -6,8 +6,27 @@ The definition for an RPC method in proto files contains request message and res Ideally, the final status of an RPC should be described in the trailing headers of HTTP2, and gRPC Python provides helper functions in `grpcio-status` package to assist the packing and unpacking of error status. + +### Requirement +``` +grpcio>=1.18.0 +grpcio-status>=1.18.0 +googleapis-common-protos>=1.5.5 +``` + + +### Error Detail Proto + +You may provide any custom proto message as error detail in your implementation. Here are protos are defined by Google Cloud Library Team: + +* [code.proto](https://github.com/googleapis/api-common-protos/blob/master/google/rpc/code.proto) contains definition of RPC error codes. +* [error_details.proto](https://github.com/googleapis/api-common-protos/blob/master/google/rpc/error_details.proto) contains definitions of common error details. + + ### Definition of Status Proto +Here is the definition of Status proto. For full text, please see [status.proto](https://github.com/googleapis/api-common-protos/blob/master/google/rpc/status.proto). + ```proto // The `Status` type defines a logical error model that is suitable for different // programming environments, including REST APIs and RPC APIs. It is used by @@ -76,6 +95,7 @@ message Status { } ``` + ### Usage of Well-Known-Proto `Any` Please check [ProtoBuf Document: Any](https://developers.google.com/protocol-buffers/docs/reference/python-generated#any) @@ -85,12 +105,3 @@ any_message.Pack(message) any_message.Unpack(message) assert any_message.Is(message.DESCRIPTOR) ``` - -### Common Protos - -These protos are defined by Google Cloud API. Most error cases are covered in `error_dettails.proto`, but you may provide any custom error detail proto you want. - -Please refer to: -* [code.proto](https://github.com/googleapis/api-common-protos/blob/master/google/rpc/code.proto). -* [status.proto](https://github.com/googleapis/api-common-protos/blob/master/google/rpc/status.proto). -* [error_details.proto](https://github.com/googleapis/api-common-protos/blob/master/google/rpc/error_details.proto). diff --git a/examples/python/errors/error_handling_client.py b/examples/python/errors/client.py similarity index 57% rename from examples/python/errors/error_handling_client.py rename to examples/python/errors/client.py index 22a865a89bf..a79b8fce1bd 100644 --- a/examples/python/errors/error_handling_client.py +++ b/examples/python/errors/client.py @@ -20,31 +20,37 @@ import grpc from grpc_status import rpc_status from google.rpc import error_details_pb2 -import helloworld_pb2 -import helloworld_pb2_grpc +from examples.protos import helloworld_pb2 +from examples.protos import helloworld_pb2_grpc +_LOGGER = logging.getLogger(__name__) -def run(): + +def process(stub): + try: + response = stub.SayHello(helloworld_pb2.HelloRequest(name='Alice')) + _LOGGER.info('Call success: %s', response.message) + except grpc.RpcError as rpc_error: + _LOGGER.error('Call failure: %s', rpc_error) + status = rpc_status.from_call(rpc_error) + for detail in status.details: + if detail.Is(error_details_pb2.QuotaFailure.DESCRIPTOR): + info = error_details_pb2.QuotaFailure() + detail.Unpack(info) + _LOGGER.error('Quota failure: %s', info) + else: + raise RuntimeError('Unexpected failure: %s' % detail) + + +def main(): # NOTE(gRPC Python Team): .close() is possible on a channel and should be # used in circumstances in which the with statement does not fit the needs # of the code. with grpc.insecure_channel('localhost:50051') as channel: stub = helloworld_pb2_grpc.GreeterStub(channel) - try: - response = stub.SayHello(helloworld_pb2.HelloRequest(name='Alice')) - print('Call success:', response.message) - except grpc.RpcError as rpc_error: - print('Call failure:', rpc_error) - status = rpc_status.from_call(rpc_error) - for detail in status.details: - if detail.Is(error_details_pb2.QuotaFailure.DESCRIPTOR): - info = error_details_pb2.QuotaFailure() - detail.Unpack(info) - print('Quota failure:', info) - else: - print('Unexpected failure:', detail) + process(stub) if __name__ == '__main__': logging.basicConfig() - run() + main() diff --git a/examples/python/errors/helloworld_pb2.py b/examples/python/errors/helloworld_pb2.py deleted file mode 100644 index e18ab9acc7a..00000000000 --- a/examples/python/errors/helloworld_pb2.py +++ /dev/null @@ -1,134 +0,0 @@ -# Generated by the protocol buffer compiler. DO NOT EDIT! -# source: helloworld.proto - -import sys -_b=sys.version_info[0]<3 and (lambda x:x) or (lambda x:x.encode('latin1')) -from google.protobuf import descriptor as _descriptor -from google.protobuf import message as _message -from google.protobuf import reflection as _reflection -from google.protobuf import symbol_database as _symbol_database -from google.protobuf import descriptor_pb2 -# @@protoc_insertion_point(imports) - -_sym_db = _symbol_database.Default() - - - - -DESCRIPTOR = _descriptor.FileDescriptor( - name='helloworld.proto', - package='helloworld', - syntax='proto3', - serialized_pb=_b('\n\x10helloworld.proto\x12\nhelloworld\"\x1c\n\x0cHelloRequest\x12\x0c\n\x04name\x18\x01 \x01(\t\"\x1d\n\nHelloReply\x12\x0f\n\x07message\x18\x01 \x01(\t2I\n\x07Greeter\x12>\n\x08SayHello\x12\x18.helloworld.HelloRequest\x1a\x16.helloworld.HelloReply\"\x00\x42\x36\n\x1bio.grpc.examples.helloworldB\x0fHelloWorldProtoP\x01\xa2\x02\x03HLWb\x06proto3') -) - - - - -_HELLOREQUEST = _descriptor.Descriptor( - name='HelloRequest', - full_name='helloworld.HelloRequest', - filename=None, - file=DESCRIPTOR, - containing_type=None, - fields=[ - _descriptor.FieldDescriptor( - name='name', full_name='helloworld.HelloRequest.name', index=0, - number=1, type=9, cpp_type=9, label=1, - has_default_value=False, default_value=_b("").decode('utf-8'), - message_type=None, enum_type=None, containing_type=None, - is_extension=False, extension_scope=None, - options=None), - ], - extensions=[ - ], - nested_types=[], - enum_types=[ - ], - options=None, - is_extendable=False, - syntax='proto3', - extension_ranges=[], - oneofs=[ - ], - serialized_start=32, - serialized_end=60, -) - - -_HELLOREPLY = _descriptor.Descriptor( - name='HelloReply', - full_name='helloworld.HelloReply', - filename=None, - file=DESCRIPTOR, - containing_type=None, - fields=[ - _descriptor.FieldDescriptor( - name='message', full_name='helloworld.HelloReply.message', index=0, - number=1, type=9, cpp_type=9, label=1, - has_default_value=False, default_value=_b("").decode('utf-8'), - message_type=None, enum_type=None, containing_type=None, - is_extension=False, extension_scope=None, - options=None), - ], - extensions=[ - ], - nested_types=[], - enum_types=[ - ], - options=None, - is_extendable=False, - syntax='proto3', - extension_ranges=[], - oneofs=[ - ], - serialized_start=62, - serialized_end=91, -) - -DESCRIPTOR.message_types_by_name['HelloRequest'] = _HELLOREQUEST -DESCRIPTOR.message_types_by_name['HelloReply'] = _HELLOREPLY -_sym_db.RegisterFileDescriptor(DESCRIPTOR) - -HelloRequest = _reflection.GeneratedProtocolMessageType('HelloRequest', (_message.Message,), dict( - DESCRIPTOR = _HELLOREQUEST, - __module__ = 'helloworld_pb2' - # @@protoc_insertion_point(class_scope:helloworld.HelloRequest) - )) -_sym_db.RegisterMessage(HelloRequest) - -HelloReply = _reflection.GeneratedProtocolMessageType('HelloReply', (_message.Message,), dict( - DESCRIPTOR = _HELLOREPLY, - __module__ = 'helloworld_pb2' - # @@protoc_insertion_point(class_scope:helloworld.HelloReply) - )) -_sym_db.RegisterMessage(HelloReply) - - -DESCRIPTOR.has_options = True -DESCRIPTOR._options = _descriptor._ParseOptions(descriptor_pb2.FileOptions(), _b('\n\033io.grpc.examples.helloworldB\017HelloWorldProtoP\001\242\002\003HLW')) - -_GREETER = _descriptor.ServiceDescriptor( - name='Greeter', - full_name='helloworld.Greeter', - file=DESCRIPTOR, - index=0, - options=None, - serialized_start=93, - serialized_end=166, - methods=[ - _descriptor.MethodDescriptor( - name='SayHello', - full_name='helloworld.Greeter.SayHello', - index=0, - containing_service=None, - input_type=_HELLOREQUEST, - output_type=_HELLOREPLY, - options=None, - ), -]) -_sym_db.RegisterServiceDescriptor(_GREETER) - -DESCRIPTOR.services_by_name['Greeter'] = _GREETER - -# @@protoc_insertion_point(module_scope) diff --git a/examples/python/errors/helloworld_pb2_grpc.py b/examples/python/errors/helloworld_pb2_grpc.py deleted file mode 100644 index 18e07d16797..00000000000 --- a/examples/python/errors/helloworld_pb2_grpc.py +++ /dev/null @@ -1,46 +0,0 @@ -# Generated by the gRPC Python protocol compiler plugin. DO NOT EDIT! -import grpc - -import helloworld_pb2 as helloworld__pb2 - - -class GreeterStub(object): - """The greeting service definition. - """ - - def __init__(self, channel): - """Constructor. - - Args: - channel: A grpc.Channel. - """ - self.SayHello = channel.unary_unary( - '/helloworld.Greeter/SayHello', - request_serializer=helloworld__pb2.HelloRequest.SerializeToString, - response_deserializer=helloworld__pb2.HelloReply.FromString, - ) - - -class GreeterServicer(object): - """The greeting service definition. - """ - - def SayHello(self, request, context): - """Sends a greeting - """ - context.set_code(grpc.StatusCode.UNIMPLEMENTED) - context.set_details('Method not implemented!') - raise NotImplementedError('Method not implemented!') - - -def add_GreeterServicer_to_server(servicer, server): - rpc_method_handlers = { - 'SayHello': grpc.unary_unary_rpc_method_handler( - servicer.SayHello, - request_deserializer=helloworld__pb2.HelloRequest.FromString, - response_serializer=helloworld__pb2.HelloReply.SerializeToString, - ), - } - generic_handler = grpc.method_handlers_generic_handler( - 'helloworld.Greeter', rpc_method_handlers) - server.add_generic_rpc_handlers((generic_handler,)) diff --git a/examples/python/errors/error_handling_server.py b/examples/python/errors/server.py similarity index 86% rename from examples/python/errors/error_handling_server.py rename to examples/python/errors/server.py index c782f7d1a2d..f49586b848a 100644 --- a/examples/python/errors/error_handling_server.py +++ b/examples/python/errors/server.py @@ -24,8 +24,8 @@ from grpc_status import rpc_status from google.protobuf import any_pb2 from google.rpc import code_pb2, status_pb2, error_details_pb2 -import helloworld_pb2 -import helloworld_pb2_grpc +from examples.protos import helloworld_pb2 +from examples.protos import helloworld_pb2_grpc _ONE_DAY_IN_SECONDS = 60 * 60 * 24 @@ -36,7 +36,7 @@ def create_greet_limit_exceed_error_status(name): error_details_pb2.QuotaFailure( violations=[ error_details_pb2.QuotaFailure.Violation( - subject="name:%s" % name, + subject="name: %s" % name, description="Limit one greeting per person", ) ],)) @@ -64,10 +64,14 @@ class LimitedGreeter(helloworld_pb2_grpc.GreeterServicer): return helloworld_pb2.HelloReply(message='Hello, %s!' % request.name) -def serve(): +def create_server(server_address): server = grpc.server(futures.ThreadPoolExecutor()) helloworld_pb2_grpc.add_GreeterServicer_to_server(LimitedGreeter(), server) - server.add_insecure_port('[::]:50051') + port = server.add_insecure_port(server_address) + return server, port + + +def serve(server): server.start() try: while True: @@ -76,6 +80,11 @@ def serve(): server.stop(None) +def main(): + server, unused_port = create_server('[::]:50051') + serve(server) + + if __name__ == '__main__': logging.basicConfig() - serve() + main() diff --git a/examples/python/errors/test/_error_handling_example_test.py b/examples/python/errors/test/_error_handling_example_test.py new file mode 100644 index 00000000000..da7f58a002e --- /dev/null +++ b/examples/python/errors/test/_error_handling_example_test.py @@ -0,0 +1,54 @@ +# Copyright 2019 The 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. +"""Tests of the error handling example.""" + +import unittest +import logging + +import grpc + +from examples.protos import helloworld_pb2_grpc +from examples.python.errors import client as error_handling_client +from examples.python.errors import server as error_handling_server + +# NOTE(lidiz) This module only exists in Bazel BUILD file, for more details +# please refer to comments in the "bazel_namespace_package_hack" module. +try: + from tests import bazel_namespace_package_hack + bazel_namespace_package_hack.sys_path_to_site_dir_hack() +except ImportError: + pass + + +class ErrorHandlingExampleTest(unittest.TestCase): + + def setUp(self): + self._server, port = error_handling_server.create_server('[::]:0') + self._server.start() + self._channel = grpc.insecure_channel('localhost:%d' % port) + + def tearDown(self): + self._channel.close() + self._server.stop(None) + + def test_error_handling_example(self): + stub = helloworld_pb2_grpc.GreeterStub(self._channel) + error_handling_client.process(stub) + error_handling_client.process(stub) + # No unhandled exception raised, test passed! + + +if __name__ == '__main__': + logging.basicConfig() + unittest.main(verbosity=2) diff --git a/src/python/grpcio_tests/tests/BUILD.bazel b/src/python/grpcio_tests/tests/BUILD.bazel index b908ab85173..b2b36ad10f3 100644 --- a/src/python/grpcio_tests/tests/BUILD.bazel +++ b/src/python/grpcio_tests/tests/BUILD.bazel @@ -4,5 +4,6 @@ py_library( visibility = [ "//src/python/grpcio_tests/tests/status:__subpackages__", "//src/python/grpcio_tests/tests/interop:__subpackages__", + "//examples/python/errors:__subpackages__", ], ) From 7771290fcd3b682e8b90ef5337895d96bc68eb68 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Mon, 25 Mar 2019 12:38:20 -0700 Subject: [PATCH 10/30] Pin the proto definitions to a specific commit --- examples/python/errors/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/python/errors/README.md b/examples/python/errors/README.md index 3c53690b99c..2cfc26a93b0 100644 --- a/examples/python/errors/README.md +++ b/examples/python/errors/README.md @@ -19,13 +19,13 @@ googleapis-common-protos>=1.5.5 You may provide any custom proto message as error detail in your implementation. Here are protos are defined by Google Cloud Library Team: -* [code.proto](https://github.com/googleapis/api-common-protos/blob/master/google/rpc/code.proto) contains definition of RPC error codes. -* [error_details.proto](https://github.com/googleapis/api-common-protos/blob/master/google/rpc/error_details.proto) contains definitions of common error details. +* [code.proto]([https://github.com/googleapis/api-common-protos/blob/master/google/rpc/code.proto](https://github.com/googleapis/api-common-protos/blob/87185dfffad4afa5a33a8c153f0e1ea53b4f85dc/google/rpc/code.proto)) contains definition of RPC error codes. +* [error_details.proto]([https://github.com/googleapis/api-common-protos/blob/master/google/rpc/error_details.proto](https://github.com/googleapis/api-common-protos/blob/87185dfffad4afa5a33a8c153f0e1ea53b4f85dc/google/rpc/error_details.proto)) contains definitions of common error details. ### Definition of Status Proto -Here is the definition of Status proto. For full text, please see [status.proto](https://github.com/googleapis/api-common-protos/blob/master/google/rpc/status.proto). +Here is the definition of Status proto. For full text, please see [status.proto](https://github.com/googleapis/api-common-protos/blob/87185dfffad4afa5a33a8c153f0e1ea53b4f85dc/google/rpc/status.proto). ```proto // The `Status` type defines a logical error model that is suitable for different From d7429dbb4ab257d5cb7c5ba86e20b24a12657293 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Mon, 25 Mar 2019 14:18:35 -0700 Subject: [PATCH 11/30] Fix the proto rules conflict --- examples/BUILD | 9 +++++++++ examples/protos/BUILD.bazel | 25 ------------------------- examples/python/errors/BUILD.bazel | 5 ++--- 3 files changed, 11 insertions(+), 28 deletions(-) delete mode 100644 examples/protos/BUILD.bazel diff --git a/examples/BUILD b/examples/BUILD index 0a1ca94a649..5e08b6cdec6 100644 --- a/examples/BUILD +++ b/examples/BUILD @@ -16,7 +16,9 @@ licenses(["notice"]) # 3-clause BSD package(default_visibility = ["//visibility:public"]) +load("@grpc_python_dependencies//:requirements.bzl", "requirement") load("//bazel:grpc_build_system.bzl", "grpc_proto_library") +load("@org_pubref_rules_protobuf//python:rules.bzl", "py_proto_library") grpc_proto_library( name = "auth_sample", @@ -43,6 +45,13 @@ grpc_proto_library( srcs = ["protos/keyvaluestore.proto"], ) +py_proto_library( + name = "py_helloworld", + protos = ["protos/helloworld.proto"], + with_grpc = True, + deps = [requirement('protobuf'),], +) + cc_binary( name = "greeter_client", srcs = ["cpp/helloworld/greeter_client.cc"], diff --git a/examples/protos/BUILD.bazel b/examples/protos/BUILD.bazel deleted file mode 100644 index 197274e034c..00000000000 --- a/examples/protos/BUILD.bazel +++ /dev/null @@ -1,25 +0,0 @@ -# Copyright 2019 The 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. - -load("@grpc_python_dependencies//:requirements.bzl", "requirement") -load("@org_pubref_rules_protobuf//python:rules.bzl", "py_proto_library") - -package(default_visibility = ["//visibility:public"]) - -py_proto_library( - name = "py_helloworld_proto", - protos = ["helloworld.proto",], - with_grpc = True, - deps = [requirement('protobuf'),], -) diff --git a/examples/python/errors/BUILD.bazel b/examples/python/errors/BUILD.bazel index a2c3534f9bd..0b56e76ee4d 100644 --- a/examples/python/errors/BUILD.bazel +++ b/examples/python/errors/BUILD.bazel @@ -13,7 +13,6 @@ # limitations under the License. load("@grpc_python_dependencies//:requirements.bzl", "requirement") -load("@org_pubref_rules_protobuf//python:rules.bzl", "py_proto_library") py_library( name = "client", @@ -22,7 +21,7 @@ py_library( deps = [ "//src/python/grpcio/grpc:grpcio", "//src/python/grpcio_status/grpc_status:grpc_status", - "//examples/protos:py_helloworld_proto", + "//examples:py_helloworld", requirement('googleapis-common-protos'), ], ) @@ -34,7 +33,7 @@ py_library( deps = [ "//src/python/grpcio/grpc:grpcio", "//src/python/grpcio_status/grpc_status:grpc_status", - "//examples/protos:py_helloworld_proto", + "//examples:py_helloworld", ] + select({ "//conditions:default": [requirement("futures")], "//:python3": [], From b3d907dce965f4e6635bbefbcf0a983e7c4094c5 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Mon, 25 Mar 2019 14:41:39 -0700 Subject: [PATCH 12/30] Explicitly depend on :grpcio --- examples/python/errors/BUILD.bazel | 2 ++ 1 file changed, 2 insertions(+) diff --git a/examples/python/errors/BUILD.bazel b/examples/python/errors/BUILD.bazel index 0b56e76ee4d..1ef36a06471 100644 --- a/examples/python/errors/BUILD.bazel +++ b/examples/python/errors/BUILD.bazel @@ -46,6 +46,8 @@ py_test( data = [ ":client", ":server", + "//src/python/grpcio/grpc:grpcio", + "//examples:py_helloworld", "//src/python/grpcio_tests/tests:bazel_namespace_package_hack", ], size = "small", From 8f77928e04fe47256a4ea6d7b02b6c7d02980a8b Mon Sep 17 00:00:00 2001 From: yang-g Date: Tue, 26 Mar 2019 14:13:40 -0700 Subject: [PATCH 13/30] Log error to stderr --- .../util/proto_reflection_descriptor_database.cc | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/test/cpp/util/proto_reflection_descriptor_database.cc b/test/cpp/util/proto_reflection_descriptor_database.cc index 119272ca42e..d0c1a847253 100644 --- a/test/cpp/util/proto_reflection_descriptor_database.cc +++ b/test/cpp/util/proto_reflection_descriptor_database.cc @@ -44,14 +44,17 @@ ProtoReflectionDescriptorDatabase::~ProtoReflectionDescriptorDatabase() { Status status = stream_->Finish(); if (!status.ok()) { if (status.error_code() == StatusCode::UNIMPLEMENTED) { - gpr_log(GPR_INFO, + fprintf(stderr, "Reflection request not implemented; " - "is the ServerReflection service enabled?"); + "is the ServerReflection service enabled?\n"); + } else { + fprintf(stderr, + "ServerReflectionInfo rpc failed. Error code: %d, message: %s, " + "debug info: %s\n", + static_cast(status.error_code()), + status.error_message().c_str(), + ctx_.debug_error_string().c_str()); } - gpr_log(GPR_INFO, - "ServerReflectionInfo rpc failed. Error code: %d, details: %s", - static_cast(status.error_code()), - status.error_message().c_str()); } } } From 4e9e662729e82bb88570e4151e11fc62d46f6ae7 Mon Sep 17 00:00:00 2001 From: Prashant Jaikumar Date: Thu, 14 Mar 2019 16:44:04 -0700 Subject: [PATCH 14/30] Fixed bug in CFStream endpoint. We were failing to return an error when the transport tried to write to an endpoint that was in an errored state. --- src/core/lib/iomgr/cfstream_handle.cc | 27 ++++- test/cpp/end2end/cfstream_test.cc | 150 +++++++++++++++++++++++++- test/cpp/end2end/test_service_impl.cc | 1 + 3 files changed, 170 insertions(+), 8 deletions(-) diff --git a/src/core/lib/iomgr/cfstream_handle.cc b/src/core/lib/iomgr/cfstream_handle.cc index 87b7b9fb334..cf21c4fc511 100644 --- a/src/core/lib/iomgr/cfstream_handle.cc +++ b/src/core/lib/iomgr/cfstream_handle.cc @@ -29,6 +29,7 @@ #include "src/core/lib/debug/trace.h" #include "src/core/lib/iomgr/closure.h" +#include "src/core/lib/iomgr/error_cfstream.h" #include "src/core/lib/iomgr/exec_ctx.h" extern grpc_core::TraceFlag grpc_tcp_trace; @@ -54,6 +55,8 @@ void CFStreamHandle::ReadCallback(CFReadStreamRef stream, void* client_callback_info) { grpc_core::ApplicationCallbackExecCtx callback_exec_ctx; grpc_core::ExecCtx exec_ctx; + grpc_error* error; + CFErrorRef stream_error; CFStreamHandle* handle = static_cast(client_callback_info); if (grpc_tcp_trace.enabled()) { gpr_log(GPR_DEBUG, "CFStream ReadCallback (%p, %p, %lu, %p)", handle, @@ -68,8 +71,15 @@ void CFStreamHandle::ReadCallback(CFReadStreamRef stream, handle->read_event_.SetReady(); break; case kCFStreamEventErrorOccurred: - handle->open_event_.SetReady(); - handle->read_event_.SetReady(); + stream_error = CFReadStreamCopyError(stream); + error = grpc_error_set_int( + GRPC_ERROR_CREATE_FROM_CFERROR(stream_error, "read error"), + GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE); + CFRelease(stream_error); + handle->open_event_.SetShutdown(GRPC_ERROR_REF(error)); + handle->write_event_.SetShutdown(GRPC_ERROR_REF(error)); + handle->read_event_.SetShutdown(GRPC_ERROR_REF(error)); + GRPC_ERROR_UNREF(error); break; default: GPR_UNREACHABLE_CODE(return ); @@ -80,6 +90,8 @@ void CFStreamHandle::WriteCallback(CFWriteStreamRef stream, void* clientCallBackInfo) { grpc_core::ApplicationCallbackExecCtx callback_exec_ctx; grpc_core::ExecCtx exec_ctx; + grpc_error* error; + CFErrorRef stream_error; CFStreamHandle* handle = static_cast(clientCallBackInfo); if (grpc_tcp_trace.enabled()) { gpr_log(GPR_DEBUG, "CFStream WriteCallback (%p, %p, %lu, %p)", handle, @@ -94,8 +106,15 @@ void CFStreamHandle::WriteCallback(CFWriteStreamRef stream, handle->write_event_.SetReady(); break; case kCFStreamEventErrorOccurred: - handle->open_event_.SetReady(); - handle->write_event_.SetReady(); + stream_error = CFWriteStreamCopyError(stream); + error = grpc_error_set_int( + GRPC_ERROR_CREATE_FROM_CFERROR(stream_error, "write error"), + GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE); + CFRelease(stream_error); + handle->open_event_.SetShutdown(GRPC_ERROR_REF(error)); + handle->write_event_.SetShutdown(GRPC_ERROR_REF(error)); + handle->read_event_.SetShutdown(GRPC_ERROR_REF(error)); + GRPC_ERROR_UNREF(error); break; default: GPR_UNREACHABLE_CODE(return ); diff --git a/test/cpp/end2end/cfstream_test.cc b/test/cpp/end2end/cfstream_test.cc index 6ca206e5f36..a6ed7c66d84 100644 --- a/test/cpp/end2end/cfstream_test.cc +++ b/test/cpp/end2end/cfstream_test.cc @@ -47,8 +47,10 @@ #include "test/cpp/end2end/test_service_impl.h" #ifdef GRPC_CFSTREAM +using grpc::ClientAsyncResponseReader; using grpc::testing::EchoRequest; using grpc::testing::EchoResponse; +using grpc::testing::RequestParams; using std::chrono::system_clock; namespace grpc { @@ -60,8 +62,7 @@ class CFStreamTest : public ::testing::Test { CFStreamTest() : server_host_("grpctest"), interface_("lo0"), - ipv4_address_("10.0.0.1"), - netmask_("/32"), + ipv4_address_("127.0.0.2"), kRequestMessage_("🖖") {} void DNSUp() { @@ -92,11 +93,13 @@ class CFStreamTest : public ::testing::Test { } void NetworkUp() { + gpr_log(GPR_DEBUG, "Bringing network up"); InterfaceUp(); DNSUp(); } void NetworkDown() { + gpr_log(GPR_DEBUG, "Bringing network down"); InterfaceDown(); DNSDown(); } @@ -149,6 +152,27 @@ class CFStreamTest : public ::testing::Test { EXPECT_TRUE(status.ok()); } } + void SendAsyncRpc( + const std::unique_ptr& stub, + RequestParams param = RequestParams()) { + EchoRequest request; + auto msg = std::to_string(ctr.load()); + request.set_message(msg); + ctr++; + *request.mutable_param() = std::move(param); + AsyncClientCall* call = new AsyncClientCall; + + call->response_reader = + stub->PrepareAsyncEcho(&call->context, request, &cq_); + + call->response_reader->StartCall(); + gpr_log(GPR_DEBUG, "Sending request: %s", msg.c_str()); + call->response_reader->Finish(&call->reply, &call->status, (void*)call); + } + + void ShutdownCQ() { cq_.Shutdown(); } + + bool CQNext(void** tag, bool* ok) { return cq_.Next(tag, ok); } bool WaitForChannelNotReady(Channel* channel, int timeout_seconds = 5) { const gpr_timespec deadline = @@ -172,6 +196,13 @@ class CFStreamTest : public ::testing::Test { return true; } + struct AsyncClientCall { + EchoResponse reply; + ClientContext context; + Status status; + std::unique_ptr> response_reader; + }; + private: struct ServerData { int port_; @@ -214,14 +245,14 @@ class CFStreamTest : public ::testing::Test { } }; + CompletionQueue cq_; const grpc::string server_host_; const grpc::string interface_; const grpc::string ipv4_address_; - const grpc::string netmask_; - std::unique_ptr stub_; std::unique_ptr server_; int port_; const grpc::string kRequestMessage_; + std::atomic_int ctr{0}; }; // gRPC should automatically detech network flaps (without enabling keepalives) @@ -261,6 +292,117 @@ TEST_F(CFStreamTest, NetworkTransition) { sender.join(); } +// Network flaps while RPCs are in flight +TEST_F(CFStreamTest, NetworkFlapRpcsInFlight) { + auto channel = BuildChannel(); + auto stub = BuildStub(channel); + std::atomic_int rpcs_sent{0}; + + // Channel should be in READY state after we send some RPCs + for (int i = 0; i < 10; ++i) { + SendAsyncRpc(stub); + ++rpcs_sent; + } + EXPECT_TRUE(WaitForChannelReady(channel.get())); + + // Bring down the network + NetworkDown(); + + std::thread thd = std::thread([this, &rpcs_sent]() { + void* got_tag; + bool ok = false; + bool network_down = true; + int total_completions = 0; + + while (CQNext(&got_tag, &ok)) { + ++total_completions; + GPR_ASSERT(ok); + AsyncClientCall* call = static_cast(got_tag); + if (call->status.ok()) { + gpr_log(GPR_DEBUG, "RPC response: %s", call->reply.message().c_str()); + } else { + gpr_log(GPR_DEBUG, "RPC failed with error: %s", + call->status.error_message().c_str()); + // Bring network up when RPCs start failing + if (network_down) { + NetworkUp(); + network_down = false; + } + } + delete call; + } + EXPECT_EQ(total_completions, rpcs_sent); + }); + + for (int i = 0; i < 100; ++i) { + SendAsyncRpc(stub); + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + ++rpcs_sent; + } + + ShutdownCQ(); + + thd.join(); +} + +// Send a bunch of RPCs, some of which are expected to fail. +// We should get back a response for all RPCs +TEST_F(CFStreamTest, ConcurrentRpc) { + auto channel = BuildChannel(); + auto stub = BuildStub(channel); + std::atomic_int rpcs_sent{0}; + std::thread thd = std::thread([this, &rpcs_sent]() { + void* got_tag; + bool ok = false; + bool network_down = true; + int total_completions = 0; + + while (CQNext(&got_tag, &ok)) { + ++total_completions; + GPR_ASSERT(ok); + AsyncClientCall* call = static_cast(got_tag); + if (call->status.ok()) { + gpr_log(GPR_DEBUG, "RPC response: %s", call->reply.message().c_str()); + } else { + gpr_log(GPR_DEBUG, "RPC failed: %s", + call->status.error_message().c_str()); + // Bring network up when RPCs start failing + if (network_down) { + NetworkUp(); + network_down = false; + } + } + delete call; + } + EXPECT_EQ(total_completions, rpcs_sent); + }); + + for (int i = 0; i < 10; ++i) { + if (i % 3 == 0) { + RequestParams param; + ErrorStatus* error = param.mutable_expected_error(); + error->set_code(StatusCode::INTERNAL); + error->set_error_message("internal error"); + SendAsyncRpc(stub, param); + } else if (i % 5 == 0) { + RequestParams param; + param.set_echo_metadata(true); + DebugInfo* info = param.mutable_debug_info(); + info->add_stack_entries("stack_entry1"); + info->add_stack_entries("stack_entry2"); + info->set_detail("detailed debug info"); + SendAsyncRpc(stub, param); + } else { + SendAsyncRpc(stub); + } + ++rpcs_sent; + } + + ShutdownCQ(); + + thd.join(); +} + } // namespace } // namespace testing } // namespace grpc diff --git a/test/cpp/end2end/test_service_impl.cc b/test/cpp/end2end/test_service_impl.cc index 1cbbc703076..abbb669cf5c 100644 --- a/test/cpp/end2end/test_service_impl.cc +++ b/test/cpp/end2end/test_service_impl.cc @@ -143,6 +143,7 @@ void LoopUntilCancelled(Alarm* alarm, ServerContext* context, Status TestServiceImpl::Echo(ServerContext* context, const EchoRequest* request, EchoResponse* response) { + gpr_log(GPR_DEBUG, "Request message was %s", request->message().c_str()); // A bit of sleep to make sure that short deadline tests fail if (request->has_param() && request->param().server_sleep_us() > 0) { gpr_sleep_until( From cdf3e001cb9aa26e8ef9af4f58cbdc252e139afa Mon Sep 17 00:00:00 2001 From: Bill Feng Date: Wed, 27 Mar 2019 10:26:41 -0700 Subject: [PATCH 15/30] wip selective build for windows --- bazel/grpc_build_system.bzl | 20 +++++++++++--------- test/core/end2end/generate_tests.bzl | 2 +- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/bazel/grpc_build_system.bzl b/bazel/grpc_build_system.bzl index c5eb9ec8bf3..84455cdafc2 100644 --- a/bazel/grpc_build_system.bzl +++ b/bazel/grpc_build_system.bzl @@ -28,12 +28,6 @@ load("//bazel:cc_grpc_library.bzl", "cc_grpc_library") # The set of pollers to test against if a test exercises polling POLLERS = ["epollex", "epoll1", "poll"] -def is_msvc(): - return select({ - "//:windows_msvc": True, - "//conditions:default": False, - }) - def if_not_windows(a): return select({ "//:windows": [], @@ -157,7 +151,6 @@ def grpc_cc_test(name, srcs = [], deps = [], external_deps = [], args = [], data if language.upper() == "C": copts = copts + if_not_windows(["-std=c99"]) args = { - "name": name, "srcs": srcs, "args": args, "data": data, @@ -172,8 +165,17 @@ def grpc_cc_test(name, srcs = [], deps = [], external_deps = [], args = [], data # Only run targets with pollers for non-MSVC # Only run targets without pollers for MSVC native.cc_test( + name = name, + testonly = True, + tags = [ + "manual", + "no_windows", + ], + **args + ) + native.cc_test( + name = name + "_windows", testonly = True, - tags = tags if is_msvc() else ["manual"], **args ) for poller in POLLERS: @@ -189,7 +191,7 @@ def grpc_cc_test(name, srcs = [], deps = [], external_deps = [], args = [], data poller, "$(location %s)" % name, ] + args["args"], - tags = (tags + ["no_windows"]) if is_msvc() else tags, + tags = (tags + ["no_windows"]), exec_compatible_with = exec_compatible_with, ) else: diff --git a/test/core/end2end/generate_tests.bzl b/test/core/end2end/generate_tests.bzl index 8020780a909..fa666bac976 100755 --- a/test/core/end2end/generate_tests.bzl +++ b/test/core/end2end/generate_tests.bzl @@ -15,7 +15,7 @@ """Generates the appropriate build.json data for all the end2end tests.""" -load("//bazel:grpc_build_system.bzl", "grpc_cc_binary", "grpc_cc_library", "is_msvc") +load("//bazel:grpc_build_system.bzl", "grpc_cc_binary", "grpc_cc_library") POLLERS = ["epollex", "epoll1", "poll"] From b0e75a42d28c232e8940f4578551cd2eb72a5c2e Mon Sep 17 00:00:00 2001 From: Soheil Hassas Yeganeh Date: Wed, 27 Mar 2019 14:10:46 -0400 Subject: [PATCH 16/30] Fix a NULL deref in tcp_client_windows.cc `grpc_sockaddr_to_uri(addr)` can return nullptr, and we are directly passing it to grpc_slice_from_copied_string. Clusterfuzz found this issue in https://clusterfuzz.com/testcase-detail/5188592759603200 Use "NULL" when target URI is nullptr, to avoid null deref. Fixes 18544 --- src/core/lib/iomgr/tcp_client_windows.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/core/lib/iomgr/tcp_client_windows.cc b/src/core/lib/iomgr/tcp_client_windows.cc index e5b5502597e..e24431b9a3e 100644 --- a/src/core/lib/iomgr/tcp_client_windows.cc +++ b/src/core/lib/iomgr/tcp_client_windows.cc @@ -213,10 +213,12 @@ static void tcp_connect(grpc_closure* on_done, grpc_endpoint** endpoint, failure: GPR_ASSERT(error != GRPC_ERROR_NONE); char* target_uri = grpc_sockaddr_to_uri(addr); - grpc_error* final_error = grpc_error_set_str( - GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING("Failed to connect", - &error, 1), - GRPC_ERROR_STR_TARGET_ADDRESS, grpc_slice_from_copied_string(target_uri)); + grpc_error* final_error = + grpc_error_set_str(GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING( + "Failed to connect", &error, 1), + GRPC_ERROR_STR_TARGET_ADDRESS, + grpc_slice_from_copied_string( + target_uri == nullptr ? "NULL" : target_uri)); GRPC_ERROR_UNREF(error); if (socket != NULL) { grpc_winsocket_destroy(socket); From 85bcce2e086d5bbb03eca386665ffd76bd4e957a Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 27 Mar 2019 12:19:00 -0700 Subject: [PATCH 17/30] Fix typo in BUILD.bazel --- examples/python/errors/BUILD.bazel | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/examples/python/errors/BUILD.bazel b/examples/python/errors/BUILD.bazel index 1ef36a06471..38bed132844 100644 --- a/examples/python/errors/BUILD.bazel +++ b/examples/python/errors/BUILD.bazel @@ -43,11 +43,9 @@ py_library( py_test( name = "test/_error_handling_example_test", srcs = ["test/_error_handling_example_test.py"], - data = [ + deps = [ ":client", ":server", - "//src/python/grpcio/grpc:grpcio", - "//examples:py_helloworld", "//src/python/grpcio_tests/tests:bazel_namespace_package_hack", ], size = "small", From 847b0155d96f1b372e9f69792a2e9bb750e41696 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Wed, 27 Mar 2019 16:30:07 -0700 Subject: [PATCH 18/30] Promise to call OnStarted and forbid Start* until after OnStarted --- include/grpcpp/impl/codegen/server_callback.h | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/include/grpcpp/impl/codegen/server_callback.h b/include/grpcpp/impl/codegen/server_callback.h index 335d5709db6..87edea84f41 100644 --- a/include/grpcpp/impl/codegen/server_callback.h +++ b/include/grpcpp/impl/codegen/server_callback.h @@ -169,9 +169,13 @@ class ServerCallbackReaderWriter { // The following classes are the reactor interfaces that are to be implemented // by the user, returned as the result of the method handler for a callback -// method, and activated by the call to OnStarted. Note that none of the classes -// are pure; all reactions have a default empty reaction so that the user class -// only needs to override those classes that it cares about. +// method, and activated by the call to OnStarted. The library guarantees that +// OnStarted will be called for any reactor that has been created using a +// method handler registered on a service. No operation initiation method may be +// called until after the call to OnStarted. +// Note that none of the classes are pure; all reactions have a default empty +// reaction so that the user class only needs to override those classes that it +// cares about. /// \a ServerBidiReactor is the interface for a bidirectional streaming RPC. template @@ -179,6 +183,9 @@ class ServerBidiReactor : public internal::ServerReactor { public: ~ServerBidiReactor() = default; + /// Do NOT call any operation initiation method (names that start with Start) + /// until after the library has called OnStarted on this object. + /// Send any initial metadata stored in the RPC context. If not invoked, /// any initial metadata will be passed along with the first Write or the /// Finish (if there are no writes). @@ -245,7 +252,8 @@ class ServerBidiReactor : public internal::ServerReactor { /// \param[in] s The status outcome of this RPC void Finish(Status s) { stream_->Finish(std::move(s)); } - /// Notify the application that a streaming RPC has started + /// Notify the application that a streaming RPC has started and that it is now + /// ok to call any operation initation method. /// /// \param[in] context The context object now associated with this RPC virtual void OnStarted(ServerContext* context) {} From 93bab217beb7ccc80a7cae60ba8e459ff2642e10 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 27 Mar 2019 16:37:37 -0700 Subject: [PATCH 19/30] Fix the import order to make hack work --- .../errors/test/_error_handling_example_test.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/examples/python/errors/test/_error_handling_example_test.py b/examples/python/errors/test/_error_handling_example_test.py index da7f58a002e..a79ca45e2a1 100644 --- a/examples/python/errors/test/_error_handling_example_test.py +++ b/examples/python/errors/test/_error_handling_example_test.py @@ -13,6 +13,14 @@ # limitations under the License. """Tests of the error handling example.""" +# NOTE(lidiz) This module only exists in Bazel BUILD file, for more details +# please refer to comments in the "bazel_namespace_package_hack" module. +try: + from tests import bazel_namespace_package_hack + bazel_namespace_package_hack.sys_path_to_site_dir_hack() +except ImportError: + pass + import unittest import logging @@ -22,14 +30,6 @@ from examples.protos import helloworld_pb2_grpc from examples.python.errors import client as error_handling_client from examples.python.errors import server as error_handling_server -# NOTE(lidiz) This module only exists in Bazel BUILD file, for more details -# please refer to comments in the "bazel_namespace_package_hack" module. -try: - from tests import bazel_namespace_package_hack - bazel_namespace_package_hack.sys_path_to_site_dir_hack() -except ImportError: - pass - class ErrorHandlingExampleTest(unittest.TestCase): From fb5c38520811aeda8d2e4d9db5c292f27cd8933e Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Wed, 27 Mar 2019 16:46:01 -0700 Subject: [PATCH 20/30] Remove unneeded header files --- src/cpp/client/channel_cc.cc | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/cpp/client/channel_cc.cc b/src/cpp/client/channel_cc.cc index a31d0b30b15..db59d4d8416 100644 --- a/src/cpp/client/channel_cc.cc +++ b/src/cpp/client/channel_cc.cc @@ -18,8 +18,6 @@ #include -#include -#include #include #include #include @@ -41,12 +39,7 @@ #include #include #include -#include -#include "src/core/lib/gpr/env.h" #include "src/core/lib/gpr/string.h" -#include "src/core/lib/gprpp/memory.h" -#include "src/core/lib/gprpp/thd.h" -#include "src/core/lib/profiling/timers.h" #include "src/core/lib/surface/completion_queue.h" namespace grpc { From 7bb9e33d1aefb40e98148092dcd9686a91474a48 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 27 Mar 2019 17:27:47 -0700 Subject: [PATCH 21/30] Add one more import path for py_test --- examples/python/errors/BUILD.bazel | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/examples/python/errors/BUILD.bazel b/examples/python/errors/BUILD.bazel index 38bed132844..b07dd12ebd3 100644 --- a/examples/python/errors/BUILD.bazel +++ b/examples/python/errors/BUILD.bazel @@ -49,5 +49,8 @@ py_test( "//src/python/grpcio_tests/tests:bazel_namespace_package_hack", ], size = "small", - imports = ["../../../src/python/grpcio_status",], + imports = [ + "../../../src/python/grpcio_status", + "../../../src/python/grpcio_tests", + ], ) From bb96e30434a83aa0c3ce712a18a5e7e69b113b47 Mon Sep 17 00:00:00 2001 From: Stanley Cheung Date: Wed, 27 Mar 2019 21:11:18 -0700 Subject: [PATCH 22/30] PHP: should use grpc_shutdown_blocking --- src/php/ext/grpc/php_grpc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/php/ext/grpc/php_grpc.c b/src/php/ext/grpc/php_grpc.c index fa6f0be837b..9c7a9d98615 100644 --- a/src/php/ext/grpc/php_grpc.c +++ b/src/php/ext/grpc/php_grpc.c @@ -180,7 +180,7 @@ void postfork_child() { grpc_php_shutdown_completion_queue(TSRMLS_C); // clean-up grpc_core - grpc_shutdown(); + grpc_shutdown_blocking(); if (grpc_is_initialized() > 0) { zend_throw_exception(spl_ce_UnexpectedValueException, "Oops, failed to shutdown gRPC Core after fork()", From 3107cda311853ce57f3a67847ce15dd14a5c9fd5 Mon Sep 17 00:00:00 2001 From: Kelly Norton Date: Wed, 27 Mar 2019 10:22:52 -0400 Subject: [PATCH 23/30] Add ini settings for fork support to PHP extension. --- src/php/ext/grpc/php_grpc.c | 48 ++++++++++---------- src/php/ext/grpc/php_grpc.h | 2 + src/php/ext/grpc/tests/grpc-default-ini.phpt | 15 ++++++ src/php/ext/grpc/tests/grpc-set-ini.phpt | 24 ++++++++++ 4 files changed, 66 insertions(+), 23 deletions(-) create mode 100644 src/php/ext/grpc/tests/grpc-default-ini.phpt create mode 100644 src/php/ext/grpc/tests/grpc-set-ini.phpt diff --git a/src/php/ext/grpc/php_grpc.c b/src/php/ext/grpc/php_grpc.c index 9c7a9d98615..3064563d03f 100644 --- a/src/php/ext/grpc/php_grpc.c +++ b/src/php/ext/grpc/php_grpc.c @@ -67,27 +67,22 @@ ZEND_GET_MODULE(grpc) /* {{{ PHP_INI */ -/* Remove comments and fill if you need to have entries in php.ini PHP_INI_BEGIN() - STD_PHP_INI_ENTRY("grpc.global_value", "42", PHP_INI_ALL, OnUpdateLong, - global_value, zend_grpc_globals, grpc_globals) - STD_PHP_INI_ENTRY("grpc.global_string", "foobar", PHP_INI_ALL, - OnUpdateString, global_string, zend_grpc_globals, - grpc_globals) + STD_PHP_INI_ENTRY("grpc.enable_fork_support", "0", PHP_INI_SYSTEM, OnUpdateBool, + enable_fork_support, zend_grpc_globals, grpc_globals) + STD_PHP_INI_ENTRY("grpc.poll_strategy", NULL, PHP_INI_SYSTEM, OnUpdateString, + poll_strategy, zend_grpc_globals, grpc_globals) PHP_INI_END() -*/ /* }}} */ /* {{{ php_grpc_init_globals */ -/* Uncomment this function if you have INI entries - static void php_grpc_init_globals(zend_grpc_globals *grpc_globals) - { - grpc_globals->global_value = 0; - grpc_globals->global_string = NULL; - } -*/ +static void php_grpc_init_globals(zend_grpc_globals *grpc_globals) { + grpc_globals->enable_fork_support = 0; + grpc_globals->poll_strategy = NULL; +} /* }}} */ + void create_new_channel( wrapped_grpc_channel *channel, char *target, @@ -208,12 +203,22 @@ void register_fork_handlers() { } } +void apply_ini_settings() { + if (GRPC_G(enable_fork_support)) { + setenv("GRPC_ENABLE_FORK_SUPPORT", "1", 1 /* overwrite? */); + } + + if (GRPC_G(poll_strategy)) { + setenv("GRPC_POLL_STRATEGY", GRPC_G(poll_strategy), 1 /* overwrite? */); + } +} + /* {{{ PHP_MINIT_FUNCTION */ PHP_MINIT_FUNCTION(grpc) { - /* If you have INI entries, uncomment these lines - REGISTER_INI_ENTRIES(); - */ + ZEND_INIT_MODULE_GLOBALS(grpc, php_grpc_init_globals, NULL); + REGISTER_INI_ENTRIES(); + /* Register call error constants */ REGISTER_LONG_CONSTANT("Grpc\\CALL_OK", GRPC_CALL_OK, CONST_CS | CONST_PERSISTENT); @@ -349,9 +354,7 @@ PHP_MINIT_FUNCTION(grpc) { /* {{{ PHP_MSHUTDOWN_FUNCTION */ PHP_MSHUTDOWN_FUNCTION(grpc) { - /* uncomment this line if you have INI entries - UNREGISTER_INI_ENTRIES(); - */ + UNREGISTER_INI_ENTRIES(); // WARNING: This function IS being called by PHP when the extension // is unloaded but the logs were somehow suppressed. if (GRPC_G(initialized)) { @@ -375,9 +378,7 @@ PHP_MINFO_FUNCTION(grpc) { php_info_print_table_row(2, "grpc support", "enabled"); php_info_print_table_row(2, "grpc module version", PHP_GRPC_VERSION); php_info_print_table_end(); - /* Remove comments if you have entries in php.ini - DISPLAY_INI_ENTRIES(); - */ + DISPLAY_INI_ENTRIES(); } /* }}} */ @@ -385,6 +386,7 @@ PHP_MINFO_FUNCTION(grpc) { */ PHP_RINIT_FUNCTION(grpc) { if (!GRPC_G(initialized)) { + apply_ini_settings(); grpc_init(); register_fork_handlers(); grpc_php_init_completion_queue(TSRMLS_C); diff --git a/src/php/ext/grpc/php_grpc.h b/src/php/ext/grpc/php_grpc.h index ecf5ebaa05b..2629b1bbd78 100644 --- a/src/php/ext/grpc/php_grpc.h +++ b/src/php/ext/grpc/php_grpc.h @@ -66,6 +66,8 @@ PHP_RINIT_FUNCTION(grpc); */ ZEND_BEGIN_MODULE_GLOBALS(grpc) zend_bool initialized; + zend_bool enable_fork_support; + char *poll_strategy; ZEND_END_MODULE_GLOBALS(grpc) /* In every utility function you add that needs to use variables diff --git a/src/php/ext/grpc/tests/grpc-default-ini.phpt b/src/php/ext/grpc/tests/grpc-default-ini.phpt new file mode 100644 index 00000000000..0fbcc1f119e --- /dev/null +++ b/src/php/ext/grpc/tests/grpc-default-ini.phpt @@ -0,0 +1,15 @@ +--TEST-- +Ensure default ini settings +--SKIPIF-- + +--FILE-- + +--INI-- +grpc.enable_fork_support = 1 +grpc.poll_strategy = epoll1 +--FILE-- + Date: Thu, 28 Mar 2019 07:54:25 -0700 Subject: [PATCH 24/30] Split data plane and control plane into their own combiners. --- .../filters/client_channel/client_channel.cc | 230 ++++++++++++------ .../ext/filters/client_channel/lb_policy.cc | 7 +- .../client_channel/lb_policy/grpclb/grpclb.cc | 11 +- 3 files changed, 168 insertions(+), 80 deletions(-) diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index 82ce253c83c..dea6e059693 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -100,49 +100,52 @@ struct QueuedPick { }; typedef struct client_channel_channel_data { + // + // Fields set at construction and never modified. + // bool deadline_checking_enabled; bool enable_retries; size_t per_rpc_retry_buffer_size; - - /** combiner protecting all variables below in this data structure */ - grpc_combiner* combiner; - /** owning stack */ grpc_channel_stack* owning_stack; - /** interested parties (owned) */ - grpc_pollset_set* interested_parties; - // Client channel factory. grpc_core::ClientChannelFactory* client_channel_factory; - // Subchannel pool. - grpc_core::RefCountedPtr subchannel_pool; grpc_core::channelz::ClientChannelNode* channelz_node; - // Resolving LB policy. - grpc_core::OrphanablePtr resolving_lb_policy; - // Subchannel picker from LB policy. + // + // Fields used in the data plane. Protected by data_plane_combiner. + // + grpc_combiner* data_plane_combiner; grpc_core::UniquePtr picker; - // Linked list of queued picks. - QueuedPick* queued_picks; - - bool have_service_config; - /** retry throttle data from service config */ + QueuedPick* queued_picks; // Linked list of queued picks. + // Data from service config. + bool received_service_config_data; grpc_core::RefCountedPtr retry_throttle_data; - /** per-method service config data */ grpc_core::RefCountedPtr method_params_table; - /* the following properties are guarded by a mutex since APIs require them - to be instantaneously available */ - gpr_mu info_mu; - grpc_core::UniquePtr info_lb_policy_name; - grpc_core::UniquePtr info_service_config_json; - + // + // Fields used in the control plane. Protected by combiner. + // + grpc_combiner* combiner; + grpc_pollset_set* interested_parties; + grpc_core::RefCountedPtr subchannel_pool; + grpc_core::OrphanablePtr resolving_lb_policy; grpc_connectivity_state_tracker state_tracker; - grpc_error* disconnect_error; - /* external_connectivity_watcher_list head is guarded by its own mutex, since - * counts need to be grabbed immediately without polling on a cq */ + // + // Fields accessed from both data plane and control plane combiners. + // + grpc_core::Atomic disconnect_error; + + // external_connectivity_watcher_list head is guarded by its own mutex, since + // counts need to be grabbed immediately without polling on a CQ. gpr_mu external_connectivity_watcher_list_mu; struct external_connectivity_watcher* external_connectivity_watcher_list_head; + + // The following properties are guarded by a mutex since APIs require them + // to be instantaneously available. + gpr_mu info_mu; + grpc_core::UniquePtr info_lb_policy_name; + grpc_core::UniquePtr info_service_config_json; } channel_data; // Forward declarations. @@ -166,30 +169,98 @@ static const char* get_channel_connectivity_state_change_string( GPR_UNREACHABLE_CODE(return "UNKNOWN"); } -static void set_connectivity_state_and_picker_locked( - channel_data* chand, grpc_connectivity_state state, grpc_error* state_error, - const char* reason, - grpc_core::UniquePtr picker) { - // Update connectivity state. - grpc_connectivity_state_set(&chand->state_tracker, state, state_error, - reason); - if (chand->channelz_node != nullptr) { - chand->channelz_node->AddTraceEvent( - grpc_core::channelz::ChannelTrace::Severity::Info, - grpc_slice_from_static_string( - get_channel_connectivity_state_change_string(state))); +namespace grpc_core { +namespace { + +// A fire-and-forget class that sets the channel's connectivity state +// and then hops into the data plane combiner to update the picker. +// Must be instantiated while holding the control plane combiner. +// Deletes itself when done. +class ConnectivityStateAndPickerSetter { + public: + ConnectivityStateAndPickerSetter( + channel_data* chand, grpc_connectivity_state state, + grpc_error* state_error, const char* reason, + UniquePtr picker) + : chand_(chand), picker_(std::move(picker)) { + // Update connectivity state here, while holding control plane combiner. + grpc_connectivity_state_set(&chand->state_tracker, state, state_error, + reason); + if (chand->channelz_node != nullptr) { + chand->channelz_node->AddTraceEvent( + channelz::ChannelTrace::Severity::Info, + grpc_slice_from_static_string( + get_channel_connectivity_state_change_string(state))); + } + // Bounce into the data plane combiner to reset the picker. + GRPC_CHANNEL_STACK_REF(chand->owning_stack, + "ConnectivityStateAndPickerSetter"); + GRPC_CLOSURE_INIT(&closure_, SetPicker, this, + grpc_combiner_scheduler(chand->data_plane_combiner)); + GRPC_CLOSURE_SCHED(&closure_, GRPC_ERROR_NONE); } - // Update picker. - chand->picker = std::move(picker); - // Re-process queued picks. - for (QueuedPick* pick = chand->queued_picks; pick != nullptr; - pick = pick->next) { - start_pick_locked(pick->elem, GRPC_ERROR_NONE); + + private: + static void SetPicker(void* arg, grpc_error* ignored) { + auto* self = static_cast(arg); + // Update picker. + self->chand_->picker = std::move(self->picker_); + // Re-process queued picks. + for (QueuedPick* pick = self->chand_->queued_picks; pick != nullptr; + pick = pick->next) { + start_pick_locked(pick->elem, GRPC_ERROR_NONE); + } + // Clean up. + GRPC_CHANNEL_STACK_UNREF(self->chand_->owning_stack, + "ConnectivityStateAndPickerSetter"); + Delete(self); } -} -namespace grpc_core { -namespace { + channel_data* chand_; + UniquePtr picker_; + grpc_closure closure_; +}; + +// A fire-and-forget class that sets the channel's service config data +// in the data plane combiner. Deletes itself when done. +class ServiceConfigSetter { + public: + ServiceConfigSetter( + channel_data* chand, + RefCountedPtr retry_throttle_data, + RefCountedPtr method_params_table) + : chand_(chand), + retry_throttle_data_(std::move(retry_throttle_data)), + method_params_table_(std::move(method_params_table)) { + GRPC_CHANNEL_STACK_REF(chand->owning_stack, "ServiceConfigSetter"); + GRPC_CLOSURE_INIT(&closure_, SetServiceConfigData, this, + grpc_combiner_scheduler(chand->data_plane_combiner)); + GRPC_CLOSURE_SCHED(&closure_, GRPC_ERROR_NONE); + } + + private: + static void SetServiceConfigData(void* arg, grpc_error* ignored) { + ServiceConfigSetter* self = static_cast(arg); + channel_data* chand = self->chand_; + // Update channel state. + chand->received_service_config_data = true; + chand->retry_throttle_data = std::move(self->retry_throttle_data_); + chand->method_params_table = std::move(self->method_params_table_); + // Apply service config to queued picks. + for (QueuedPick* pick = chand->queued_picks; pick != nullptr; + pick = pick->next) { + maybe_apply_service_config_to_call_locked(pick->elem); + } + // Clean up. + GRPC_CHANNEL_STACK_UNREF(self->chand_->owning_stack, "ServiceConfigSetter"); + Delete(self); + } + + channel_data* chand_; + RefCountedPtr retry_throttle_data_; + RefCountedPtr method_params_table_; + grpc_closure closure_; +}; class ClientChannelControlHelper : public LoadBalancingPolicy::ChannelControlHelper { @@ -222,8 +293,10 @@ class ClientChannelControlHelper void UpdateState( grpc_connectivity_state state, grpc_error* state_error, UniquePtr picker) override { + grpc_error* disconnect_error = + chand_->disconnect_error.Load(grpc_core::MemoryOrder::ACQUIRE); if (grpc_client_channel_routing_trace.enabled()) { - const char* extra = chand_->disconnect_error == GRPC_ERROR_NONE + const char* extra = disconnect_error == GRPC_ERROR_NONE ? "" : " (ignoring -- channel shutting down)"; gpr_log(GPR_INFO, "chand=%p: update: state=%s error=%s picker=%p%s", @@ -231,9 +304,10 @@ class ClientChannelControlHelper grpc_error_string(state_error), picker.get(), extra); } // Do update only if not shutting down. - if (chand_->disconnect_error == GRPC_ERROR_NONE) { - set_connectivity_state_and_picker_locked(chand_, state, state_error, - "helper", std::move(picker)); + if (disconnect_error == GRPC_ERROR_NONE) { + // Will delete itself. + New(chand_, state, state_error, + "helper", std::move(picker)); } else { GRPC_ERROR_UNREF(state_error); } @@ -255,7 +329,6 @@ static bool process_resolver_result_locked( void* arg, grpc_core::Resolver::Result* result, const char** lb_policy_name, grpc_core::RefCountedPtr* lb_policy_config) { channel_data* chand = static_cast(arg); - chand->have_service_config = true; ProcessedResolverResult resolver_result(result, chand->enable_retries); grpc_core::UniquePtr service_config_json = resolver_result.service_config_json(); @@ -263,9 +336,11 @@ static bool process_resolver_result_locked( gpr_log(GPR_INFO, "chand=%p: resolver returned service config: \"%s\"", chand, service_config_json.get()); } - // Update channel state. - chand->retry_throttle_data = resolver_result.retry_throttle_data(); - chand->method_params_table = resolver_result.method_params_table(); + // Create service config setter to update channel state in the data + // plane combiner. Destroys itself when done. + grpc_core::New( + chand, resolver_result.retry_throttle_data(), + resolver_result.method_params_table()); // Swap out the data used by cc_get_channel_info(). gpr_mu_lock(&chand->info_mu); chand->info_lb_policy_name = resolver_result.lb_policy_name(); @@ -280,11 +355,6 @@ static bool process_resolver_result_locked( // Return results. *lb_policy_name = chand->info_lb_policy_name.get(); *lb_policy_config = resolver_result.lb_policy_config(); - // Apply service config to queued picks. - for (QueuedPick* pick = chand->queued_picks; pick != nullptr; - pick = pick->next) { - maybe_apply_service_config_to_call_locked(pick->elem); - } return service_config_changed; } @@ -342,12 +412,16 @@ static void start_transport_op_locked(void* arg, grpc_error* error_ignored) { } if (op->disconnect_with_error != GRPC_ERROR_NONE) { - chand->disconnect_error = op->disconnect_with_error; + grpc_error* error = GRPC_ERROR_NONE; + GPR_ASSERT(chand->disconnect_error.CompareExchangeStrong( + &error, op->disconnect_with_error, grpc_core::MemoryOrder::ACQ_REL, + grpc_core::MemoryOrder::ACQUIRE)); grpc_pollset_set_del_pollset_set( chand->resolving_lb_policy->interested_parties(), chand->interested_parties); chand->resolving_lb_policy.reset(); - set_connectivity_state_and_picker_locked( + // Will delete itself. + grpc_core::New( chand, GRPC_CHANNEL_SHUTDOWN, GRPC_ERROR_REF(op->disconnect_with_error), "shutdown from API", grpc_core::UniquePtr( @@ -397,10 +471,12 @@ static grpc_error* cc_init_channel_elem(grpc_channel_element* elem, GPR_ASSERT(args->is_last); GPR_ASSERT(elem->filter == &grpc_client_channel_filter); // Initialize data members. + chand->data_plane_combiner = grpc_combiner_create(); chand->combiner = grpc_combiner_create(); grpc_connectivity_state_init(&chand->state_tracker, GRPC_CHANNEL_IDLE, "client_channel"); - chand->disconnect_error = GRPC_ERROR_NONE; + chand->disconnect_error.Store(GRPC_ERROR_NONE, + grpc_core::MemoryOrder::RELAXED); gpr_mu_init(&chand->info_mu); gpr_mu_init(&chand->external_connectivity_watcher_list_mu); @@ -511,8 +587,10 @@ static void cc_destroy_channel_elem(grpc_channel_element* elem) { chand->method_params_table.reset(); grpc_client_channel_stop_backup_polling(chand->interested_parties); grpc_pollset_set_destroy(chand->interested_parties); + GRPC_COMBINER_UNREF(chand->data_plane_combiner, "client_channel"); GRPC_COMBINER_UNREF(chand->combiner, "client_channel"); - GRPC_ERROR_UNREF(chand->disconnect_error); + GRPC_ERROR_UNREF( + chand->disconnect_error.Load(grpc_core::MemoryOrder::RELAXED)); grpc_connectivity_state_destroy(&chand->state_tracker); gpr_mu_destroy(&chand->info_mu); gpr_mu_destroy(&chand->external_connectivity_watcher_list_mu); @@ -1261,7 +1339,7 @@ static void do_retry(grpc_call_element* elem, } // Schedule retry after computed delay. GRPC_CLOSURE_INIT(&calld->pick_closure, start_pick_locked, elem, - grpc_combiner_scheduler(chand->combiner)); + grpc_combiner_scheduler(chand->data_plane_combiner)); grpc_timer_init(&calld->retry_timer, next_attempt_time, &calld->pick_closure); // Update bookkeeping. if (retry_state != nullptr) retry_state->retry_dispatched = true; @@ -2488,7 +2566,7 @@ class QueuedPickCanceller { auto* chand = static_cast(elem->channel_data); GRPC_CALL_STACK_REF(calld->owning_call, "QueuedPickCanceller"); GRPC_CLOSURE_INIT(&closure_, &CancelLocked, this, - grpc_combiner_scheduler(chand->combiner)); + grpc_combiner_scheduler(chand->data_plane_combiner)); grpc_call_combiner_set_notify_on_cancel(calld->call_combiner, &closure_); } @@ -2628,7 +2706,7 @@ static void maybe_apply_service_config_to_call_locked(grpc_call_element* elem) { call_data* calld = static_cast(elem->call_data); // Apply service config data to the call only once, and only if the // channel has the data available. - if (GPR_LIKELY(chand->have_service_config && + if (GPR_LIKELY(chand->received_service_config_data && !calld->service_config_applied)) { calld->service_config_applied = true; apply_service_config_to_call_locked(elem); @@ -2676,7 +2754,7 @@ static void start_pick_locked(void* arg, grpc_error* error) { .send_initial_metadata_flags; // Apply service config to call if needed. maybe_apply_service_config_to_call_locked(elem); - // When done, we schedule this closure to leave the channel combiner. + // When done, we schedule this closure to leave the data plane combiner. GRPC_CLOSURE_INIT(&calld->pick_closure, pick_done, elem, grpc_schedule_on_exec_ctx); // Attempt pick. @@ -2691,12 +2769,14 @@ static void start_pick_locked(void* arg, grpc_error* error) { grpc_error_string(error)); } switch (pick_result) { - case LoadBalancingPolicy::PICK_TRANSIENT_FAILURE: + case LoadBalancingPolicy::PICK_TRANSIENT_FAILURE: { // If we're shutting down, fail all RPCs. - if (chand->disconnect_error != GRPC_ERROR_NONE) { + grpc_error* disconnect_error = + chand->disconnect_error.Load(grpc_core::MemoryOrder::ACQUIRE); + if (disconnect_error != GRPC_ERROR_NONE) { GRPC_ERROR_UNREF(error); GRPC_CLOSURE_SCHED(&calld->pick_closure, - GRPC_ERROR_REF(chand->disconnect_error)); + GRPC_ERROR_REF(disconnect_error)); break; } // If wait_for_ready is false, then the error indicates the RPC @@ -2722,7 +2802,8 @@ static void start_pick_locked(void* arg, grpc_error* error) { // If wait_for_ready is true, then queue to retry when we get a new // picker. GRPC_ERROR_UNREF(error); - // Fallthrough + } + // Fallthrough case LoadBalancingPolicy::PICK_QUEUE: if (!calld->pick_queued) add_call_to_queued_picks_locked(elem); break; @@ -2816,7 +2897,8 @@ static void cc_start_transport_stream_op_batch( } GRPC_CLOSURE_SCHED( GRPC_CLOSURE_INIT(&batch->handler_private.closure, start_pick_locked, - elem, grpc_combiner_scheduler(chand->combiner)), + elem, + grpc_combiner_scheduler(chand->data_plane_combiner)), GRPC_ERROR_NONE); } else { // For all other batches, release the call combiner. diff --git a/src/core/ext/filters/client_channel/lb_policy.cc b/src/core/ext/filters/client_channel/lb_policy.cc index c8f8e82e5d7..6b657465891 100644 --- a/src/core/ext/filters/client_channel/lb_policy.cc +++ b/src/core/ext/filters/client_channel/lb_policy.cc @@ -140,10 +140,9 @@ LoadBalancingPolicy::PickResult LoadBalancingPolicy::QueuePicker::Pick( // the time this function returns, the pick will already have // been processed, and we'll be trying to re-process the same // pick again, leading to a crash. - // 2. In a subsequent PR, we will split the data plane and control - // plane synchronization into separate combiners, at which - // point this will need to hop from the data plane combiner into - // the control plane combiner. + // 2. We are currently running in the data plane combiner, but we + // need to bounce into the control plane combiner to call + // ExitIdleLocked(). if (!exit_idle_called_) { exit_idle_called_ = true; parent_->Ref().release(); // ref held by closure. diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc index 02fe06c4557..5bf15aa8f7f 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc @@ -234,12 +234,19 @@ class GrpcLb : public LoadBalancingPolicy { // Returns the LB token to use for a drop, or null if the call // should not be dropped. - // Intended to be called from picker, so calls will be externally - // synchronized. + // + // Note: This is called from the picker, so it will be invoked in + // the channel's data plane combiner, NOT the control plane + // combiner. It should not be accessed by any other part of the LB + // policy. const char* ShouldDrop(); private: grpc_grpclb_serverlist* serverlist_; + + // Guarded by the channel's data plane combiner, NOT the control + // plane combiner. It should not be accessed by anything but the + // picker via the ShouldDrop() method. size_t drop_index_ = 0; }; From 719bdcfd6a67f464f200e5d086c3d1890cc74b60 Mon Sep 17 00:00:00 2001 From: Bill Feng Date: Thu, 28 Mar 2019 16:07:36 -0700 Subject: [PATCH 25/30] temporarily disable all poller-enabled tests for Windows --- bazel/grpc_build_system.bzl | 5 ----- 1 file changed, 5 deletions(-) diff --git a/bazel/grpc_build_system.bzl b/bazel/grpc_build_system.bzl index 84455cdafc2..12a79259bb9 100644 --- a/bazel/grpc_build_system.bzl +++ b/bazel/grpc_build_system.bzl @@ -173,11 +173,6 @@ def grpc_cc_test(name, srcs = [], deps = [], external_deps = [], args = [], data ], **args ) - native.cc_test( - name = name + "_windows", - testonly = True, - **args - ) for poller in POLLERS: native.sh_test( name = name + "@poller=" + poller, From 29d855751ff435c8a48d28e0e0944e6757df3eaf Mon Sep 17 00:00:00 2001 From: Bill Feng Date: Thu, 28 Mar 2019 16:26:28 -0700 Subject: [PATCH 26/30] removed excessive end2end targets --- test/core/end2end/generate_tests.bzl | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/test/core/end2end/generate_tests.bzl b/test/core/end2end/generate_tests.bzl index fa666bac976..b3c58bf29b4 100755 --- a/test/core/end2end/generate_tests.bzl +++ b/test/core/end2end/generate_tests.bzl @@ -457,16 +457,6 @@ def grpc_end2end_nosec_tests(): continue if topt.secure: continue - native.sh_test( - name = "%s_nosec_test@%s" % (f, t), - data = [":%s_nosec_test" % f], - srcs = ["end2end_test.sh"], - args = [ - "$(location %s_nosec_test)" % f, - t, - ], - tags = ["no_windows"], - ) for poller in POLLERS: native.sh_test( name = "%s_nosec_test@%s@poller=%s" % (f, t, poller), From 5fba3425c01fc2ced6dc3cf7e28afa61b3d69f08 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 28 Mar 2019 16:36:39 -0700 Subject: [PATCH 27/30] Fix link to unexistent docs --- tools/remote_build/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/remote_build/README.md b/tools/remote_build/README.md index 19739e9ee12..6dba652e419 100644 --- a/tools/remote_build/README.md +++ b/tools/remote_build/README.md @@ -12,7 +12,7 @@ and tests run by Kokoro CI. - See [Installing Bazel](https://docs.bazel.build/versions/master/install.html) for instructions how to install bazel on your system. -- Setup application default credentials for running remote builds by following [RBE Credentials Setup](https://cloud.google.com/remote-build-execution/docs/getting-started#set_credentials) +- Setup application default credentials for running remote builds by following ["Set credentials" section.](https://cloud.google.com/remote-build-execution/docs/set-up/first-remote-build) ## Running remote build manually from dev workstation From 3d03c2d97c0f2e0f1c052c5ed35e38771817bb98 Mon Sep 17 00:00:00 2001 From: Bill Feng Date: Thu, 28 Mar 2019 17:05:26 -0700 Subject: [PATCH 28/30] fixed comments --- bazel/grpc_build_system.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bazel/grpc_build_system.bzl b/bazel/grpc_build_system.bzl index 12a79259bb9..bc4d1f1358a 100644 --- a/bazel/grpc_build_system.bzl +++ b/bazel/grpc_build_system.bzl @@ -163,7 +163,7 @@ def grpc_cc_test(name, srcs = [], deps = [], external_deps = [], args = [], data } if uses_polling: # Only run targets with pollers for non-MSVC - # Only run targets without pollers for MSVC + # TODO(yfen): Enable MSVC for poller-enabled targets without pollers native.cc_test( name = name, testonly = True, From d29de0ed0be34b33addc09bdf5051926a676d6cc Mon Sep 17 00:00:00 2001 From: Bill Feng Date: Thu, 28 Mar 2019 17:08:54 -0700 Subject: [PATCH 29/30] removed unused configs --- bazel/BUILD | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/bazel/BUILD b/bazel/BUILD index 4b581709f20..32402892cc3 100644 --- a/bazel/BUILD +++ b/bazel/BUILD @@ -29,16 +29,3 @@ cc_grpc_library( proto_only = True, deps = [], ) - -environment(name = "windows_msvc") - -environment(name = "non_windows_msvc") - -environment_group( - name = "os", - defaults = [":non_windows_msvc"], - environments = [ - ":non_windows_msvc", - ":windows_msvc", - ], -) From 4f2d8a0014864fda7bb1aa1c794783afade87d0a Mon Sep 17 00:00:00 2001 From: Prashant Jaikumar Date: Tue, 15 Jan 2019 14:10:43 -0800 Subject: [PATCH 30/30] Fix bazel build for examples Fixed the bazel build for some code in the examples directory. --- examples/BUILD | 112 ++++++++++++++++-- .../cpp/helloworld/greeter_async_client.cc | 4 + .../cpp/helloworld/greeter_async_client2.cc | 4 + .../cpp/helloworld/greeter_async_server.cc | 4 + examples/cpp/route_guide/helper.cc | 31 ++--- .../cpp/route_guide/route_guide_client.cc | 4 + .../cpp/route_guide/route_guide_server.cc | 4 + 7 files changed, 137 insertions(+), 26 deletions(-) diff --git a/examples/BUILD b/examples/BUILD index 0a1ca94a649..60c645d0441 100644 --- a/examples/BUILD +++ b/examples/BUILD @@ -47,56 +47,110 @@ cc_binary( name = "greeter_client", srcs = ["cpp/helloworld/greeter_client.cc"], defines = ["BAZEL_BUILD"], - deps = [":helloworld", "//:grpc++"], + deps = [ + ":helloworld", + "//:grpc++", + ], +) + +cc_binary( + name = "greeter_async_client", + srcs = ["cpp/helloworld/greeter_async_client.cc"], + defines = ["BAZEL_BUILD"], + deps = [ + ":helloworld", + "//:grpc++", + ], +) + +cc_binary( + name = "greeter_async_client2", + srcs = ["cpp/helloworld/greeter_async_client2.cc"], + defines = ["BAZEL_BUILD"], + deps = [ + ":helloworld", + "//:grpc++", + ], ) cc_binary( name = "greeter_server", srcs = ["cpp/helloworld/greeter_server.cc"], defines = ["BAZEL_BUILD"], - deps = [":helloworld", "//:grpc++"], + deps = [ + ":helloworld", + "//:grpc++", + ], +) + +cc_binary( + name = "greeter_async_server", + srcs = ["cpp/helloworld/greeter_async_server.cc"], + defines = ["BAZEL_BUILD"], + deps = [ + ":helloworld", + "//:grpc++", + ], ) cc_binary( name = "metadata_client", srcs = ["cpp/metadata/greeter_client.cc"], defines = ["BAZEL_BUILD"], - deps = [":helloworld", "//:grpc++"], + deps = [ + ":helloworld", + "//:grpc++", + ], ) cc_binary( name = "metadata_server", srcs = ["cpp/metadata/greeter_server.cc"], defines = ["BAZEL_BUILD"], - deps = [":helloworld", "//:grpc++"], + deps = [ + ":helloworld", + "//:grpc++", + ], ) cc_binary( name = "lb_client", srcs = ["cpp/load_balancing/greeter_client.cc"], defines = ["BAZEL_BUILD"], - deps = [":helloworld", "//:grpc++"], + deps = [ + ":helloworld", + "//:grpc++", + ], ) cc_binary( name = "lb_server", srcs = ["cpp/load_balancing/greeter_server.cc"], defines = ["BAZEL_BUILD"], - deps = [":helloworld", "//:grpc++"], + deps = [ + ":helloworld", + "//:grpc++", + ], ) cc_binary( name = "compression_client", srcs = ["cpp/compression/greeter_client.cc"], defines = ["BAZEL_BUILD"], - deps = [":helloworld", "//:grpc++"], + deps = [ + ":helloworld", + "//:grpc++", + ], ) cc_binary( name = "compression_server", srcs = ["cpp/compression/greeter_server.cc"], defines = ["BAZEL_BUILD"], - deps = [":helloworld", "//:grpc++"], + deps = [ + ":helloworld", + "//:grpc++", + ], ) cc_binary( @@ -104,12 +158,48 @@ cc_binary( srcs = ["cpp/keyvaluestore/caching_interceptor.h", "cpp/keyvaluestore/client.cc"], defines = ["BAZEL_BUILD"], - deps = [":keyvaluestore", "//:grpc++"], + deps = [ + ":keyvaluestore", + "//:grpc++", + ], ) cc_binary( name = "keyvaluestore_server", srcs = ["cpp/keyvaluestore/server.cc"], defines = ["BAZEL_BUILD"], - deps = [":keyvaluestore", "//:grpc++"], -) \ No newline at end of file + deps = [ + ":keyvaluestore", + "//:grpc++", + ], +) + +cc_binary( + name = "route_guide_client", + srcs = [ + "cpp/route_guide/helper.cc", + "cpp/route_guide/helper.h", + "cpp/route_guide/route_guide_client.cc", + ], + data = ["cpp/route_guide/route_guide_db.json"], + defines = ["BAZEL_BUILD"], + deps = [ + ":route_guide", + "//:grpc++", + ], +) + +cc_binary( + name = "route_guide_server", + srcs = [ + "cpp/route_guide/helper.cc", + "cpp/route_guide/helper.h", + "cpp/route_guide/route_guide_server.cc", + ], + data = ["cpp/route_guide/route_guide_db.json"], + defines = ["BAZEL_BUILD"], + deps = [ + ":route_guide", + "//:grpc++", + ], +) diff --git a/examples/cpp/helloworld/greeter_async_client.cc b/examples/cpp/helloworld/greeter_async_client.cc index d7a9d52836c..9b1def6d1da 100644 --- a/examples/cpp/helloworld/greeter_async_client.cc +++ b/examples/cpp/helloworld/greeter_async_client.cc @@ -23,7 +23,11 @@ #include #include +#ifdef BAZEL_BUILD +#include "examples/protos/helloworld.grpc.pb.h" +#else #include "helloworld.grpc.pb.h" +#endif using grpc::Channel; using grpc::ClientAsyncResponseReader; diff --git a/examples/cpp/helloworld/greeter_async_client2.cc b/examples/cpp/helloworld/greeter_async_client2.cc index d5098b9fc3f..97631f38889 100644 --- a/examples/cpp/helloworld/greeter_async_client2.cc +++ b/examples/cpp/helloworld/greeter_async_client2.cc @@ -24,7 +24,11 @@ #include #include +#ifdef BAZEL_BUILD +#include "examples/protos/helloworld.grpc.pb.h" +#else #include "helloworld.grpc.pb.h" +#endif using grpc::Channel; using grpc::ClientAsyncResponseReader; diff --git a/examples/cpp/helloworld/greeter_async_server.cc b/examples/cpp/helloworld/greeter_async_server.cc index a74673d8035..d4d1594c72b 100644 --- a/examples/cpp/helloworld/greeter_async_server.cc +++ b/examples/cpp/helloworld/greeter_async_server.cc @@ -24,7 +24,11 @@ #include #include +#ifdef BAZEL_BUILD +#include "examples/protos/helloworld.grpc.pb.h" +#else #include "helloworld.grpc.pb.h" +#endif using grpc::Server; using grpc::ServerAsyncResponseWriter; diff --git a/examples/cpp/route_guide/helper.cc b/examples/cpp/route_guide/helper.cc index 266f754b940..cd889f6a4ba 100644 --- a/examples/cpp/route_guide/helper.cc +++ b/examples/cpp/route_guide/helper.cc @@ -23,7 +23,11 @@ #include #include #include +#ifdef BAZEL_BUILD +#include "examples/protos/route_guide.grpc.pb.h" +#else #include "route_guide.grpc.pb.h" +#endif namespace routeguide { @@ -35,13 +39,16 @@ std::string GetDbFileContent(int argc, char** argv) { size_t start_position = argv_1.find(arg_str); if (start_position != std::string::npos) { start_position += arg_str.size(); - if (argv_1[start_position] == ' ' || - argv_1[start_position] == '=') { + if (argv_1[start_position] == ' ' || argv_1[start_position] == '=') { db_path = argv_1.substr(start_position + 1); } } } else { +#ifdef BAZEL_BUILD + db_path = "cpp/route_guide/route_guide_db.json"; +#else db_path = "route_guide_db.json"; +#endif } std::ifstream db_file(db_path); if (!db_file.is_open()) { @@ -60,17 +67,13 @@ class Parser { public: explicit Parser(const std::string& db) : db_(db) { // Remove all spaces. - db_.erase( - std::remove_if(db_.begin(), db_.end(), isspace), - db_.end()); + db_.erase(std::remove_if(db_.begin(), db_.end(), isspace), db_.end()); if (!Match("[")) { SetFailedAndReturnFalse(); } } - bool Finished() { - return current_ >= db_.size(); - } + bool Finished() { return current_ >= db_.size(); } bool TryParseOne(Feature* feature) { if (failed_ || Finished() || !Match("{")) { @@ -96,7 +99,7 @@ class Parser { if (current_ == db_.size()) { return SetFailedAndReturnFalse(); } - feature->set_name(db_.substr(name_start, current_-name_start-1)); + feature->set_name(db_.substr(name_start, current_ - name_start - 1)); if (!Match("},")) { if (db_[current_ - 1] == ']' && current_ == db_.size()) { return true; @@ -107,7 +110,6 @@ class Parser { } private: - bool SetFailedAndReturnFalse() { failed_ = true; return false; @@ -121,7 +123,8 @@ class Parser { void ReadLong(long* l) { size_t start = current_; - while (current_ != db_.size() && db_[current_] != ',' && db_[current_] != '}') { + while (current_ != db_.size() && db_[current_] != ',' && + db_[current_] != '}') { current_++; } // It will throw an exception if fails. @@ -154,10 +157,8 @@ void ParseDb(const std::string& db, std::vector* feature_list) { break; } } - std::cout << "DB parsed, loaded " << feature_list->size() - << " features." << std::endl; + std::cout << "DB parsed, loaded " << feature_list->size() << " features." + << std::endl; } - } // namespace routeguide - diff --git a/examples/cpp/route_guide/route_guide_client.cc b/examples/cpp/route_guide/route_guide_client.cc index a89ec164c62..439edb0dbdf 100644 --- a/examples/cpp/route_guide/route_guide_client.cc +++ b/examples/cpp/route_guide/route_guide_client.cc @@ -29,7 +29,11 @@ #include #include #include "helper.h" +#ifdef BAZEL_BUILD +#include "examples/protos/route_guide.grpc.pb.h" +#else #include "route_guide.grpc.pb.h" +#endif using grpc::Channel; using grpc::ClientContext; diff --git a/examples/cpp/route_guide/route_guide_server.cc b/examples/cpp/route_guide/route_guide_server.cc index 5867c167128..781642747f3 100644 --- a/examples/cpp/route_guide/route_guide_server.cc +++ b/examples/cpp/route_guide/route_guide_server.cc @@ -29,7 +29,11 @@ #include #include #include "helper.h" +#ifdef BAZEL_BUILD +#include "examples/protos/route_guide.grpc.pb.h" +#else #include "route_guide.grpc.pb.h" +#endif using grpc::Server; using grpc::ServerBuilder;