diff options
19 files changed, 87 insertions, 55 deletions
diff --git a/include/grpcpp/opencensus.h b/include/grpcpp/opencensus.h index 07a1333986..29b221f767 100644 --- a/include/grpcpp/opencensus.h +++ b/include/grpcpp/opencensus.h @@ -19,10 +19,6 @@ #ifndef GRPCPP_OPENCENSUS_H #define GRPCPP_OPENCENSUS_H -#ifndef GRPC_BAZEL_BUILD -#error OpenCensus for gRPC is only supported when building with bazel. -#endif - #include "opencensus/trace/span.h" namespace grpc { diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index bb3ea400d1..d63e2c66c2 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -2194,9 +2194,9 @@ static void add_retriable_send_initial_metadata_op( .grpc_previous_rpc_attempts); } if (GPR_UNLIKELY(calld->num_attempts_completed > 0)) { - grpc_mdelem retry_md = grpc_mdelem_from_slices( + grpc_mdelem retry_md = grpc_mdelem_create( GRPC_MDSTR_GRPC_PREVIOUS_RPC_ATTEMPTS, - *retry_count_strings[calld->num_attempts_completed - 1]); + *retry_count_strings[calld->num_attempts_completed - 1], nullptr); grpc_error* error = grpc_metadata_batch_add_tail( &retry_state->send_initial_metadata, &retry_state->send_initial_metadata_storage[calld->send_initial_metadata diff --git a/src/core/ext/filters/http/client_authority_filter.cc b/src/core/ext/filters/http/client_authority_filter.cc index 1ca20ebb26..6383f12594 100644 --- a/src/core/ext/filters/http/client_authority_filter.cc +++ b/src/core/ext/filters/http/client_authority_filter.cc @@ -59,9 +59,8 @@ void authority_start_transport_stream_op_batch( initial_metadata->idx.named.authority == nullptr) { grpc_error* error = grpc_metadata_batch_add_head( initial_metadata, &calld->authority_storage, - grpc_mdelem_from_slices( - GRPC_MDSTR_AUTHORITY, - grpc_slice_ref_internal(chand->default_authority))); + grpc_mdelem_create(GRPC_MDSTR_AUTHORITY, chand->default_authority, + nullptr)); if (error != GRPC_ERROR_NONE) { grpc_transport_stream_op_batch_finish_with_failure(batch, error, calld->call_combiner); diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index 2d4b4da4c6..8a481bb7d5 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -2128,8 +2128,7 @@ void grpc_chttp2_fake_status(grpc_chttp2_transport* t, grpc_chttp2_stream* s, "add_status_message", grpc_chttp2_incoming_metadata_buffer_replace_or_add( &s->metadata_buffer[1], - grpc_mdelem_from_slices(GRPC_MDSTR_GRPC_MESSAGE, - grpc_slice_ref_internal(slice)))); + grpc_mdelem_create(GRPC_MDSTR_GRPC_MESSAGE, slice, nullptr))); } s->published_metadata[1] = GRPC_METADATA_SYNTHESIZED_FROM_FAKE; grpc_chttp2_maybe_complete_recv_trailing_metadata(t, s); diff --git a/src/core/lib/channel/channelz.h b/src/core/lib/channel/channelz.h index fddef793fb..88551befc8 100644 --- a/src/core/lib/channel/channelz.h +++ b/src/core/lib/channel/channelz.h @@ -42,13 +42,13 @@ /** This is the default value for whether or not to enable channelz. If * GRPC_ARG_ENABLE_CHANNELZ is set, it will override this default value. */ -#define GRPC_ENABLE_CHANNELZ_DEFAULT false +#define GRPC_ENABLE_CHANNELZ_DEFAULT true /** This is the default value for the maximum amount of memory used by trace * events per channel trace node. If * GRPC_ARG_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE is set, it will override * this default value. */ -#define GRPC_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE_DEFAULT 0 +#define GRPC_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE_DEFAULT 1024 * 4 namespace grpc_core { diff --git a/src/core/lib/security/credentials/plugin/plugin_credentials.cc b/src/core/lib/security/credentials/plugin/plugin_credentials.cc index 73946ce039..4015124298 100644 --- a/src/core/lib/security/credentials/plugin/plugin_credentials.cc +++ b/src/core/lib/security/credentials/plugin/plugin_credentials.cc @@ -102,8 +102,7 @@ static grpc_error* process_plugin_result( } else { for (size_t i = 0; i < num_md; ++i) { grpc_mdelem mdelem = - grpc_mdelem_from_slices(grpc_slice_ref_internal(md[i].key), - grpc_slice_ref_internal(md[i].value)); + grpc_mdelem_create(md[i].key, md[i].value, nullptr); grpc_credentials_mdelem_array_add(r->md_array, mdelem); GRPC_MDELEM_UNREF(mdelem); } diff --git a/src/core/lib/surface/channel.cc b/src/core/lib/surface/channel.cc index 4294434504..d7095c24d4 100644 --- a/src/core/lib/surface/channel.cc +++ b/src/core/lib/surface/channel.cc @@ -336,9 +336,8 @@ grpc_call* grpc_channel_create_call(grpc_channel* channel, grpc_core::ExecCtx exec_ctx; grpc_call* call = grpc_channel_create_call_internal( channel, parent_call, propagation_mask, cq, nullptr, - grpc_mdelem_from_slices(GRPC_MDSTR_PATH, grpc_slice_ref_internal(method)), - host != nullptr ? grpc_mdelem_from_slices(GRPC_MDSTR_AUTHORITY, - grpc_slice_ref_internal(*host)) + grpc_mdelem_create(GRPC_MDSTR_PATH, method, nullptr), + host != nullptr ? grpc_mdelem_create(GRPC_MDSTR_AUTHORITY, *host, nullptr) : GRPC_MDNULL, grpc_timespec_to_millis_round_up(deadline)); @@ -347,14 +346,13 @@ grpc_call* grpc_channel_create_call(grpc_channel* channel, grpc_call* grpc_channel_create_pollset_set_call( grpc_channel* channel, grpc_call* parent_call, uint32_t propagation_mask, - grpc_pollset_set* pollset_set, grpc_slice method, const grpc_slice* host, - grpc_millis deadline, void* reserved) { + grpc_pollset_set* pollset_set, const grpc_slice& method, + const grpc_slice* host, grpc_millis deadline, void* reserved) { GPR_ASSERT(!reserved); return grpc_channel_create_call_internal( channel, parent_call, propagation_mask, nullptr, pollset_set, - grpc_mdelem_from_slices(GRPC_MDSTR_PATH, grpc_slice_ref_internal(method)), - host != nullptr ? grpc_mdelem_from_slices(GRPC_MDSTR_AUTHORITY, - grpc_slice_ref_internal(*host)) + grpc_mdelem_create(GRPC_MDSTR_PATH, method, nullptr), + host != nullptr ? grpc_mdelem_create(GRPC_MDSTR_AUTHORITY, *host, nullptr) : GRPC_MDNULL, deadline); } diff --git a/src/core/lib/surface/channel.h b/src/core/lib/surface/channel.h index e5ff2c3596..4ac76b8a29 100644 --- a/src/core/lib/surface/channel.h +++ b/src/core/lib/surface/channel.h @@ -45,8 +45,8 @@ grpc_channel* grpc_channel_create_with_builder( value of \a propagation_mask (see propagation_bits.h for possible values) */ grpc_call* grpc_channel_create_pollset_set_call( grpc_channel* channel, grpc_call* parent_call, uint32_t propagation_mask, - grpc_pollset_set* pollset_set, grpc_slice method, const grpc_slice* host, - grpc_millis deadline, void* reserved); + grpc_pollset_set* pollset_set, const grpc_slice& method, + const grpc_slice* host, grpc_millis deadline, void* reserved); /** Get a (borrowed) pointer to this channels underlying channel stack */ grpc_channel_stack* grpc_channel_get_channel_stack(grpc_channel* channel); diff --git a/src/core/lib/transport/metadata.cc b/src/core/lib/transport/metadata.cc index d164502280..60af22393e 100644 --- a/src/core/lib/transport/metadata.cc +++ b/src/core/lib/transport/metadata.cc @@ -237,7 +237,7 @@ static void rehash_mdtab(mdtab_shard* shard) { } grpc_mdelem grpc_mdelem_create( - grpc_slice key, grpc_slice value, + const grpc_slice& key, const grpc_slice& value, grpc_mdelem_data* compatible_external_backing_store) { if (!grpc_slice_is_interned(key) || !grpc_slice_is_interned(value)) { if (compatible_external_backing_store != nullptr) { @@ -324,7 +324,8 @@ grpc_mdelem grpc_mdelem_create( return GRPC_MAKE_MDELEM(md, GRPC_MDELEM_STORAGE_INTERNED); } -grpc_mdelem grpc_mdelem_from_slices(grpc_slice key, grpc_slice value) { +grpc_mdelem grpc_mdelem_from_slices(const grpc_slice& key, + const grpc_slice& value) { grpc_mdelem out = grpc_mdelem_create(key, value, nullptr); grpc_slice_unref_internal(key); grpc_slice_unref_internal(value); diff --git a/src/core/lib/transport/metadata.h b/src/core/lib/transport/metadata.h index 338082276c..989c7544c1 100644 --- a/src/core/lib/transport/metadata.h +++ b/src/core/lib/transport/metadata.h @@ -109,7 +109,8 @@ struct grpc_mdelem { (uintptr_t)GRPC_MDELEM_STORAGE_INTERNED_BIT)) /* Unrefs the slices. */ -grpc_mdelem grpc_mdelem_from_slices(grpc_slice key, grpc_slice value); +grpc_mdelem grpc_mdelem_from_slices(const grpc_slice& key, + const grpc_slice& value); /* Cheaply convert a grpc_metadata to a grpc_mdelem; may use the grpc_metadata object as backing storage (so lifetimes should align) */ @@ -120,7 +121,7 @@ grpc_mdelem grpc_mdelem_from_grpc_metadata(grpc_metadata* metadata); compatible_external_backing_store if it is non-NULL (in which case it's the users responsibility to ensure that it outlives usage) */ grpc_mdelem grpc_mdelem_create( - grpc_slice key, grpc_slice value, + const grpc_slice& key, const grpc_slice& value, grpc_mdelem_data* compatible_external_backing_store); bool grpc_mdelem_eq(grpc_mdelem a, grpc_mdelem b); diff --git a/src/python/grpcio/grpc/_channel.py b/src/python/grpcio/grpc/_channel.py index 3494c9b15a..eeeb4ddb33 100644 --- a/src/python/grpcio/grpc/_channel.py +++ b/src/python/grpcio/grpc/_channel.py @@ -981,4 +981,7 @@ class Channel(grpc.Channel): # then deletion of this grpc._channel.Channel instance can be made to # effect closure of the underlying cygrpc.Channel instance. cygrpc.fork_unregister_channel(self) - _moot(self._connectivity_state) + # This prevent the failed-at-initializing object removal from failing. + # Though the __init__ failed, the removal will still trigger __del__. + if hasattr(self, "_connectivity_state"): + _moot(self._connectivity_state) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/arguments.pxd.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/arguments.pxd.pxi index 6cb1bc0c05..e0e068e452 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/arguments.pxd.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/arguments.pxd.pxi @@ -32,7 +32,7 @@ cdef class _ArgumentProcessor: cdef grpc_arg c_argument - cdef void c(self, argument, grpc_arg_pointer_vtable *vtable, references) + cdef void c(self, argument, grpc_arg_pointer_vtable *vtable, references) except * cdef class _ArgumentsProcessor: @@ -42,5 +42,5 @@ cdef class _ArgumentsProcessor: cdef readonly list _references cdef grpc_channel_args _c_arguments - cdef grpc_channel_args *c(self, grpc_arg_pointer_vtable *vtable) + cdef grpc_channel_args *c(self, grpc_arg_pointer_vtable *vtable) except * cdef un_c(self) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/arguments.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/arguments.pyx.pxi index 2239e26b32..b7a4277ff6 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/arguments.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/arguments.pyx.pxi @@ -52,7 +52,7 @@ cdef grpc_arg _unwrap_grpc_arg(tuple wrapped_arg): cdef class _ArgumentProcessor: - cdef void c(self, argument, grpc_arg_pointer_vtable *vtable, references): + cdef void c(self, argument, grpc_arg_pointer_vtable *vtable, references) except *: key, value = argument cdef bytes encoded_key = _encode(key) if encoded_key is not key: @@ -89,7 +89,7 @@ cdef class _ArgumentsProcessor: self._argument_processors = [] self._references = [] - cdef grpc_channel_args *c(self, grpc_arg_pointer_vtable *vtable): + cdef grpc_channel_args *c(self, grpc_arg_pointer_vtable *vtable) except *: self._c_arguments.arguments_length = len(self._arguments) if self._c_arguments.arguments_length == 0: return NULL diff --git a/src/python/grpcio_tests/commands.py b/src/python/grpcio_tests/commands.py index 0dfbf3180b..6931d93ef0 100644 --- a/src/python/grpcio_tests/commands.py +++ b/src/python/grpcio_tests/commands.py @@ -116,6 +116,8 @@ class TestGevent(setuptools.Command): # eventually succeed, but need to dig into performance issues. 'unit._cython._no_messages_server_completion_queue_per_call_test.Test.test_rpcs', 'unit._cython._no_messages_single_server_completion_queue_test.Test.test_rpcs', + # TODO(https://github.com/grpc/grpc/issues/16890) enable this test + 'unit._cython._channel_test.ChannelTest.test_multiple_channels_lonely_connectivity', # I have no idea why this doesn't work in gevent, but it shouldn't even be # using the c-core 'testing._client_test.ClientTest.test_infinite_request_stream_real_time', diff --git a/src/python/grpcio_tests/tests/unit/_channel_args_test.py b/src/python/grpcio_tests/tests/unit/_channel_args_test.py index 869c2f4d2f..dd1d2969a2 100644 --- a/src/python/grpcio_tests/tests/unit/_channel_args_test.py +++ b/src/python/grpcio_tests/tests/unit/_channel_args_test.py @@ -33,6 +33,14 @@ TEST_CHANNEL_ARGS = ( ('arg6', TestPointerWrapper()), ) +INVALID_TEST_CHANNEL_ARGS = [ + { + 'foo': 'bar' + }, + (('key',),), + 'str', +] + class ChannelArgsTest(unittest.TestCase): @@ -44,6 +52,14 @@ class ChannelArgsTest(unittest.TestCase): futures.ThreadPoolExecutor(max_workers=1), options=TEST_CHANNEL_ARGS) + def test_invalid_client_args(self): + for invalid_arg in INVALID_TEST_CHANNEL_ARGS: + self.assertRaises( + ValueError, + grpc.insecure_channel, + 'localhost:8080', + options=invalid_arg) + if __name__ == '__main__': unittest.main(verbosity=2) diff --git a/test/core/channel/channelz_test.cc b/test/core/channel/channelz_test.cc index aed1fba47c..057d53b4ab 100644 --- a/test/core/channel/channelz_test.cc +++ b/test/core/channel/channelz_test.cc @@ -126,12 +126,12 @@ void ValidateGetServers(size_t expected_servers) { class ChannelFixture { public: ChannelFixture(int max_tracer_event_memory = 0) { - grpc_arg client_a[2]; - client_a[0] = grpc_channel_arg_integer_create( - const_cast<char*>(GRPC_ARG_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE), - max_tracer_event_memory); - client_a[1] = grpc_channel_arg_integer_create( - const_cast<char*>(GRPC_ARG_ENABLE_CHANNELZ), true); + grpc_arg client_a[] = { + grpc_channel_arg_integer_create( + const_cast<char*>(GRPC_ARG_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE), + max_tracer_event_memory), + grpc_channel_arg_integer_create( + const_cast<char*>(GRPC_ARG_ENABLE_CHANNELZ), true)}; grpc_channel_args client_args = {GPR_ARRAY_SIZE(client_a), client_a}; channel_ = grpc_insecure_channel_create("fake_target", &client_args, nullptr); @@ -238,8 +238,16 @@ TEST_P(ChannelzChannelTest, BasicChannel) { TEST(ChannelzChannelTest, ChannelzDisabled) { grpc_core::ExecCtx exec_ctx; + // explicitly disable channelz + grpc_arg arg[] = { + grpc_channel_arg_integer_create( + const_cast<char*>(GRPC_ARG_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE), + 0), + grpc_channel_arg_integer_create( + const_cast<char*>(GRPC_ARG_ENABLE_CHANNELZ), false)}; + grpc_channel_args args = {GPR_ARRAY_SIZE(arg), arg}; grpc_channel* channel = - grpc_insecure_channel_create("fake_target", nullptr, nullptr); + grpc_insecure_channel_create("fake_target", &args, nullptr); ChannelNode* channelz_channel = grpc_channel_get_channelz_node(channel); ASSERT_EQ(channelz_channel, nullptr); grpc_channel_destroy(channel); diff --git a/test/core/end2end/tests/channelz.cc b/test/core/end2end/tests/channelz.cc index 41549190e3..a812994435 100644 --- a/test/core/end2end/tests/channelz.cc +++ b/test/core/end2end/tests/channelz.cc @@ -199,9 +199,13 @@ static void run_one_request(grpc_end2end_test_config config, static void test_channelz(grpc_end2end_test_config config) { grpc_end2end_test_fixture f; - grpc_arg arg = grpc_channel_arg_integer_create( - const_cast<char*>(GRPC_ARG_ENABLE_CHANNELZ), true); - grpc_channel_args args = {1, &arg}; + grpc_arg arg[] = { + grpc_channel_arg_integer_create( + const_cast<char*>(GRPC_ARG_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE), + 0), + grpc_channel_arg_integer_create( + const_cast<char*>(GRPC_ARG_ENABLE_CHANNELZ), true)}; + grpc_channel_args args = {GPR_ARRAY_SIZE(arg), arg}; f = begin_test(config, "test_channelz", &args, &args); grpc_core::channelz::ChannelNode* channelz_channel = @@ -267,12 +271,12 @@ static void test_channelz(grpc_end2end_test_config config) { static void test_channelz_with_channel_trace(grpc_end2end_test_config config) { grpc_end2end_test_fixture f; - grpc_arg arg[2]; - arg[0] = grpc_channel_arg_integer_create( - const_cast<char*>(GRPC_ARG_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE), - 1024 * 1024); - arg[1] = grpc_channel_arg_integer_create( - const_cast<char*>(GRPC_ARG_ENABLE_CHANNELZ), true); + grpc_arg arg[] = { + grpc_channel_arg_integer_create( + const_cast<char*>(GRPC_ARG_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE), + 1024 * 1024), + grpc_channel_arg_integer_create( + const_cast<char*>(GRPC_ARG_ENABLE_CHANNELZ), true)}; grpc_channel_args args = {GPR_ARRAY_SIZE(arg), arg}; f = begin_test(config, "test_channelz_with_channel_trace", &args, &args); @@ -307,7 +311,15 @@ static void test_channelz_with_channel_trace(grpc_end2end_test_config config) { static void test_channelz_disabled(grpc_end2end_test_config config) { grpc_end2end_test_fixture f; - f = begin_test(config, "test_channelz_disabled", nullptr, nullptr); + grpc_arg arg[] = { + grpc_channel_arg_integer_create( + const_cast<char*>(GRPC_ARG_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE), + 0), + grpc_channel_arg_integer_create( + const_cast<char*>(GRPC_ARG_ENABLE_CHANNELZ), false)}; + grpc_channel_args args = {GPR_ARRAY_SIZE(arg), arg}; + + f = begin_test(config, "test_channelz_disabled", &args, &args); grpc_core::channelz::ChannelNode* channelz_channel = grpc_channel_get_channelz_node(f.client); GPR_ASSERT(channelz_channel == nullptr); diff --git a/test/cpp/interop/client.cc b/test/cpp/interop/client.cc index 1d7fa73aa8..a4b1a85f85 100644 --- a/test/cpp/interop/client.cc +++ b/test/cpp/interop/client.cc @@ -137,8 +137,7 @@ int main(int argc, char** argv) { &grpc::testing::InteropClient::DoTimeoutOnSleepingServer, &client); actions["empty_stream"] = std::bind(&grpc::testing::InteropClient::DoEmptyStream, &client); - if (FLAGS_use_tls || - FLAGS_custom_credentials_type == "google_default_credentials") { + if (FLAGS_use_tls) { actions["compute_engine_creds"] = std::bind(&grpc::testing::InteropClient::DoComputeEngineCreds, &client, FLAGS_default_service_account, FLAGS_oauth_scope); diff --git a/tools/run_tests/run_interop_tests.py b/tools/run_tests/run_interop_tests.py index 7ec02b707e..1d639edb82 100755 --- a/tools/run_tests/run_interop_tests.py +++ b/tools/run_tests/run_interop_tests.py @@ -1378,8 +1378,7 @@ try: transport_security='tls') jobs.append(tls_test_job) if str(language) in [ - 'c++', - 'go', + 'go' ]: # Add more languages to the list to turn on tests. google_default_creds_test_job = cloud_to_prod_jobspec( language, |