diff --git a/src/core/ext/filters/http/client_authority_filter.cc b/src/core/ext/filters/http/client_authority_filter.cc index 1f57ab5ce62..63b9150aec3 100644 --- a/src/core/ext/filters/http/client_authority_filter.cc +++ b/src/core/ext/filters/http/client_authority_filter.cc @@ -59,8 +59,9 @@ 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(chand->default_authority))); + grpc_mdelem_from_slices( + GRPC_MDSTR_AUTHORITY, + grpc_slice_ref_internal(chand->default_authority))); if (error != GRPC_ERROR_NONE) { grpc_transport_stream_op_batch_finish_with_failure(batch, error, calld->call_combiner); @@ -110,7 +111,7 @@ grpc_error* init_channel_elem(grpc_channel_element* elem, /* Destructor for channel data */ void destroy_channel_elem(grpc_channel_element* elem) { channel_data* chand = static_cast(elem->channel_data); - grpc_slice_unref(chand->default_authority); + grpc_slice_unref_internal(chand->default_authority); } } // namespace diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.cc b/src/core/ext/transport/cronet/transport/cronet_transport.cc index 0b88ff7afed..420c2d13e1d 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.cc +++ b/src/core/ext/transport/cronet/transport/cronet_transport.cc @@ -736,7 +736,7 @@ static void convert_metadata_to_cronet_headers( if (grpc_is_binary_header(GRPC_MDKEY(mdelem))) { grpc_slice wire_value = grpc_chttp2_base64_encode(GRPC_MDVALUE(mdelem)); value = grpc_slice_to_c_string(wire_value); - grpc_slice_unref(wire_value); + grpc_slice_unref_internal(wire_value); } else { value = grpc_slice_to_c_string(GRPC_MDVALUE(mdelem)); } diff --git a/src/core/lib/iomgr/tcp_client_posix.cc b/src/core/lib/iomgr/tcp_client_posix.cc index 6144d389f7c..900c0565753 100644 --- a/src/core/lib/iomgr/tcp_client_posix.cc +++ b/src/core/lib/iomgr/tcp_client_posix.cc @@ -45,6 +45,7 @@ #include "src/core/lib/iomgr/tcp_posix.h" #include "src/core/lib/iomgr/timer.h" #include "src/core/lib/iomgr/unix_sockets_posix.h" +#include "src/core/lib/slice/slice_internal.h" extern grpc_core::TraceFlag grpc_tcp_trace; @@ -233,7 +234,7 @@ finish: error = grpc_error_set_str(error, GRPC_ERROR_STR_TARGET_ADDRESS, addr_str_slice /* takes ownership */); } else { - grpc_slice_unref(addr_str_slice); + grpc_slice_unref_internal(addr_str_slice); } if (done) { // This is safe even outside the lock, because "done", the sentinel, is diff --git a/src/core/lib/security/security_connector/alts_security_connector.cc b/src/core/lib/security/security_connector/alts_security_connector.cc index 5ff7d7938bf..35a787871a8 100644 --- a/src/core/lib/security/security_connector/alts_security_connector.cc +++ b/src/core/lib/security/security_connector/alts_security_connector.cc @@ -30,6 +30,7 @@ #include "src/core/lib/security/credentials/alts/alts_credentials.h" #include "src/core/lib/security/transport/security_handshaker.h" +#include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/transport/transport.h" #include "src/core/tsi/alts/handshaker/alts_tsi_handshaker.h" @@ -133,7 +134,7 @@ grpc_security_status grpc_alts_auth_context_from_tsi_peer( rpc_versions_prop->value.data, rpc_versions_prop->value.length); bool decode_result = grpc_gcp_rpc_protocol_versions_decode(slice, &peer_versions); - grpc_slice_unref(slice); + grpc_slice_unref_internal(slice); if (!decode_result) { gpr_log(GPR_ERROR, "Invalid peer rpc protocol versions."); return GRPC_SECURITY_ERROR; diff --git a/src/core/lib/transport/byte_stream.cc b/src/core/lib/transport/byte_stream.cc index cb15a71a912..16b85ca0db0 100644 --- a/src/core/lib/transport/byte_stream.cc +++ b/src/core/lib/transport/byte_stream.cc @@ -45,7 +45,7 @@ SliceBufferByteStream::SliceBufferByteStream(grpc_slice_buffer* slice_buffer, SliceBufferByteStream::~SliceBufferByteStream() {} void SliceBufferByteStream::Orphan() { - grpc_slice_buffer_destroy(&backing_buffer_); + grpc_slice_buffer_destroy_internal(&backing_buffer_); GRPC_ERROR_UNREF(shutdown_error_); // Note: We do not actually delete the object here, since // SliceBufferByteStream is usually allocated as part of a larger diff --git a/tools/run_tests/sanity/core_banned_functions.py b/tools/run_tests/sanity/core_banned_functions.py index 989990542e3..1d3f2eba8a6 100755 --- a/tools/run_tests/sanity/core_banned_functions.py +++ b/tools/run_tests/sanity/core_banned_functions.py @@ -23,35 +23,39 @@ os.chdir(os.path.join(os.path.dirname(sys.argv[0]), '../../..')) # map of banned function signature to whitelist BANNED_EXCEPT = { - 'grpc_resource_quota_ref(': ['src/core/lib/iomgr/resource_quota.c'], - 'grpc_resource_quota_unref(': ['src/core/lib/iomgr/resource_quota.c'], - 'grpc_slice_buffer_destroy(': ['src/core/lib/slice/slice_buffer.c'], - 'grpc_slice_buffer_reset_and_unref(': ['src/core/lib/slice/slice_buffer.c'], - 'grpc_slice_ref(': ['src/core/lib/slice/slice.c'], - 'grpc_slice_unref(': ['src/core/lib/slice/slice.c'], - 'grpc_error_create(': ['src/core/lib/iomgr/error.c'], - 'grpc_error_ref(': ['src/core/lib/iomgr/error.c'], - 'grpc_error_unref(': ['src/core/lib/iomgr/error.c'], - 'grpc_os_error(': ['src/core/lib/iomgr/error.c'], - 'grpc_wsa_error(': ['src/core/lib/iomgr/error.c'], - 'grpc_log_if_error(': ['src/core/lib/iomgr/error.c'], - 'grpc_slice_malloc(': ['src/core/lib/slice/slice.c'], - 'grpc_closure_create(': ['src/core/lib/iomgr/closure.c'], - 'grpc_closure_init(': ['src/core/lib/iomgr/closure.c'], - 'grpc_closure_sched(': ['src/core/lib/iomgr/closure.c'], - 'grpc_closure_run(': ['src/core/lib/iomgr/closure.c'], - 'grpc_closure_list_sched(': ['src/core/lib/iomgr/closure.c'], + 'grpc_resource_quota_ref(': ['src/core/lib/iomgr/resource_quota.cc'], + 'grpc_resource_quota_unref(': ['src/core/lib/iomgr/resource_quota.cc'], + 'grpc_slice_buffer_destroy(': ['src/core/lib/slice/slice_buffer.cc'], + 'grpc_slice_buffer_reset_and_unref(': + ['src/core/lib/slice/slice_buffer.cc'], + 'grpc_slice_ref(': ['src/core/lib/slice/slice.cc'], + 'grpc_slice_unref(': ['src/core/lib/slice/slice.cc'], + 'grpc_error_create(': ['src/core/lib/iomgr/error.cc'], + 'grpc_error_ref(': ['src/core/lib/iomgr/error.cc'], + 'grpc_error_unref(': ['src/core/lib/iomgr/error.cc'], + 'grpc_os_error(': ['src/core/lib/iomgr/error.cc'], + 'grpc_wsa_error(': ['src/core/lib/iomgr/error.cc'], + 'grpc_log_if_error(': ['src/core/lib/iomgr/error.cc'], + 'grpc_slice_malloc(': ['src/core/lib/slice/slice.cc'], + 'grpc_closure_create(': ['src/core/lib/iomgr/closure.cc'], + 'grpc_closure_init(': ['src/core/lib/iomgr/closure.cc'], + 'grpc_closure_sched(': ['src/core/lib/iomgr/closure.cc'], + 'grpc_closure_run(': ['src/core/lib/iomgr/closure.cc'], + 'grpc_closure_list_sched(': ['src/core/lib/iomgr/closure.cc'], 'gpr_getenv_silent(': [ - 'src/core/lib/gpr/log.c', 'src/core/lib/gpr/env_linux.c', - 'src/core/lib/gpr/env_posix.c', 'src/core/lib/gpr/env_windows.c' + 'src/core/lib/gpr/log.cc', 'src/core/lib/gpr/env_linux.cc', + 'src/core/lib/gpr/env_posix.cc', 'src/core/lib/gpr/env_windows.cc' ], } errors = 0 +num_files = 0 for root, dirs, files in os.walk('src/core'): + if root.startswith('src/core/tsi'): continue for filename in files: + num_files += 1 path = os.path.join(root, filename) - if os.path.splitext(path)[1] != '.c': continue + if os.path.splitext(path)[1] != '.cc': continue with open(path) as f: text = f.read() for banned, exceptions in BANNED_EXCEPT.items(): @@ -61,3 +65,8 @@ for root, dirs, files in os.walk('src/core'): errors += 1 assert errors == 0 +# This check comes about from this issue: +# https://github.com/grpc/grpc/issues/15381 +# Basically, a change rendered this script useless and we did not realize it. +# This dumb check ensures that this type of issue doesn't occur again. +assert num_files > 300 # we definitely have more than 300 files