Fix metadata batch removal ref counting

reviewable/pr8842/r2
Craig Tiller 8 years ago
parent f269eddd57
commit de7b4676e9
  1. 4
      src/core/ext/load_reporting/load_reporting_filter.c
  2. 2
      src/core/lib/channel/compress_filter.c
  3. 21
      src/core/lib/channel/http_client_filter.c
  4. 10
      src/core/lib/channel/http_server_filter.c
  5. 8
      src/core/lib/surface/call.c
  6. 8
      src/core/lib/transport/metadata.c
  7. 2
      src/core/lib/transport/metadata.h
  8. 4
      src/core/lib/transport/metadata_batch.c
  9. 3
      src/core/lib/transport/metadata_batch.h

@ -86,7 +86,7 @@ static void on_initial_md_ready(grpc_exec_ctx *exec_ctx, void *user_data,
GRPC_MDVALUE(calld->recv_initial_metadata->idx.named.lb_token->md)); GRPC_MDVALUE(calld->recv_initial_metadata->idx.named.lb_token->md));
calld->have_initial_md_string = true; calld->have_initial_md_string = true;
grpc_metadata_batch_remove( grpc_metadata_batch_remove(
calld->recv_initial_metadata, exec_ctx, calld->recv_initial_metadata,
calld->recv_initial_metadata->idx.named.lb_token); calld->recv_initial_metadata->idx.named.lb_token);
} }
} else { } else {
@ -197,7 +197,7 @@ static void lr_start_transport_stream_op(grpc_exec_ctx *exec_ctx,
GRPC_MDVALUE(op->send_trailing_metadata->idx.named.lb_cost_bin->md)); GRPC_MDVALUE(op->send_trailing_metadata->idx.named.lb_cost_bin->md));
calld->have_trailing_md_string = true; calld->have_trailing_md_string = true;
grpc_metadata_batch_remove( grpc_metadata_batch_remove(
op->send_trailing_metadata, exec_ctx, op->send_trailing_metadata,
op->send_trailing_metadata->idx.named.lb_cost_bin); op->send_trailing_metadata->idx.named.lb_cost_bin);
} }
} }

@ -134,7 +134,7 @@ static grpc_error *process_send_initial_metadata(
calld->has_compression_algorithm = 1; calld->has_compression_algorithm = 1;
grpc_metadata_batch_remove( grpc_metadata_batch_remove(
initial_metadata, exec_ctx, initial_metadata,
initial_metadata->idx.named.grpc_internal_encoding_request); initial_metadata->idx.named.grpc_internal_encoding_request);
} else { } else {
/* If no algorithm was found in the metadata and we aren't /* If no algorithm was found in the metadata and we aren't

@ -99,7 +99,7 @@ static grpc_error *client_filter_incoming_metadata(grpc_exec_ctx *exec_ctx,
grpc_metadata_batch *b) { grpc_metadata_batch *b) {
if (b->idx.named.status != NULL) { if (b->idx.named.status != NULL) {
if (grpc_mdelem_eq(b->idx.named.status->md, GRPC_MDELEM_STATUS_200)) { if (grpc_mdelem_eq(b->idx.named.status->md, GRPC_MDELEM_STATUS_200)) {
grpc_metadata_batch_remove(b, b->idx.named.status); grpc_metadata_batch_remove(exec_ctx, b, b->idx.named.status);
} else { } else {
char *val = grpc_dump_slice(GRPC_MDVALUE(b->idx.named.status->md), char *val = grpc_dump_slice(GRPC_MDVALUE(b->idx.named.status->md),
GPR_DUMP_ASCII); GPR_DUMP_ASCII);
@ -150,7 +150,7 @@ static grpc_error *client_filter_incoming_metadata(grpc_exec_ctx *exec_ctx,
gpr_free(val); gpr_free(val);
} }
} }
grpc_metadata_batch_remove(b, b->idx.named.content_type); grpc_metadata_batch_remove(exec_ctx, b, b->idx.named.content_type);
} }
return GRPC_ERROR_NONE; return GRPC_ERROR_NONE;
@ -201,10 +201,11 @@ static void send_done(grpc_exec_ctx *exec_ctx, void *elemp, grpc_error *error) {
calld->post_send->cb(exec_ctx, calld->post_send->cb_arg, error); calld->post_send->cb(exec_ctx, calld->post_send->cb_arg, error);
} }
static void remove_if_present(grpc_metadata_batch *batch, static void remove_if_present(grpc_exec_ctx *exec_ctx,
grpc_metadata_batch *batch,
grpc_metadata_batch_callouts_index idx) { grpc_metadata_batch_callouts_index idx) {
if (batch->idx.array[idx] != NULL) { if (batch->idx.array[idx] != NULL) {
grpc_metadata_batch_remove(batch, batch->idx.array[idx]); grpc_metadata_batch_remove(exec_ctx, batch, batch->idx.array[idx]);
} }
} }
@ -303,11 +304,13 @@ static grpc_error *hc_mutate_op(grpc_exec_ctx *exec_ctx,
} }
} }
remove_if_present(op->send_initial_metadata, GRPC_BATCH_METHOD); remove_if_present(exec_ctx, op->send_initial_metadata, GRPC_BATCH_METHOD);
remove_if_present(op->send_initial_metadata, GRPC_BATCH_SCHEME); remove_if_present(exec_ctx, op->send_initial_metadata, GRPC_BATCH_SCHEME);
remove_if_present(op->send_initial_metadata, GRPC_BATCH_TE); remove_if_present(exec_ctx, op->send_initial_metadata, GRPC_BATCH_TE);
remove_if_present(op->send_initial_metadata, GRPC_BATCH_CONTENT_TYPE); remove_if_present(exec_ctx, op->send_initial_metadata,
remove_if_present(op->send_initial_metadata, GRPC_BATCH_USER_AGENT); GRPC_BATCH_CONTENT_TYPE);
remove_if_present(exec_ctx, op->send_initial_metadata,
GRPC_BATCH_USER_AGENT);
/* Send : prefixed headers, which have to be before any application /* Send : prefixed headers, which have to be before any application
layer headers. */ layer headers. */

@ -128,7 +128,7 @@ static grpc_error *server_filter_incoming_metadata(grpc_exec_ctx *exec_ctx,
grpc_attach_md_to_error(GRPC_ERROR_CREATE("Bad header"), grpc_attach_md_to_error(GRPC_ERROR_CREATE("Bad header"),
b->idx.named.method->md)); b->idx.named.method->md));
} }
grpc_metadata_batch_remove(b, b->idx.named.method); grpc_metadata_batch_remove(exec_ctx, b, b->idx.named.method);
} else { } else {
add_error(error_name, &error, add_error(error_name, &error,
grpc_error_set_str(GRPC_ERROR_CREATE("Missing header"), grpc_error_set_str(GRPC_ERROR_CREATE("Missing header"),
@ -141,7 +141,7 @@ static grpc_error *server_filter_incoming_metadata(grpc_exec_ctx *exec_ctx,
grpc_attach_md_to_error(GRPC_ERROR_CREATE("Bad header"), grpc_attach_md_to_error(GRPC_ERROR_CREATE("Bad header"),
b->idx.named.te->md)); b->idx.named.te->md));
} }
grpc_metadata_batch_remove(b, b->idx.named.te); grpc_metadata_batch_remove(exec_ctx, b, b->idx.named.te);
} else { } else {
add_error(error_name, &error, add_error(error_name, &error,
grpc_error_set_str(GRPC_ERROR_CREATE("Missing header"), grpc_error_set_str(GRPC_ERROR_CREATE("Missing header"),
@ -156,7 +156,7 @@ static grpc_error *server_filter_incoming_metadata(grpc_exec_ctx *exec_ctx,
grpc_attach_md_to_error(GRPC_ERROR_CREATE("Bad header"), grpc_attach_md_to_error(GRPC_ERROR_CREATE("Bad header"),
b->idx.named.scheme->md)); b->idx.named.scheme->md));
} }
grpc_metadata_batch_remove(b, b->idx.named.scheme); grpc_metadata_batch_remove(exec_ctx, b, b->idx.named.scheme);
} else { } else {
add_error(error_name, &error, add_error(error_name, &error,
grpc_error_set_str(GRPC_ERROR_CREATE("Missing header"), grpc_error_set_str(GRPC_ERROR_CREATE("Missing header"),
@ -189,7 +189,7 @@ static grpc_error *server_filter_incoming_metadata(grpc_exec_ctx *exec_ctx,
gpr_free(val); gpr_free(val);
} }
} }
grpc_metadata_batch_remove(b, b->idx.named.content_type); grpc_metadata_batch_remove(exec_ctx, b, b->idx.named.content_type);
} }
if (b->idx.named.path == NULL) { if (b->idx.named.path == NULL) {
@ -220,7 +220,7 @@ static grpc_error *server_filter_incoming_metadata(grpc_exec_ctx *exec_ctx,
GRPC_MDVALUE(b->idx.named.grpc_payload_bin->md))); GRPC_MDVALUE(b->idx.named.grpc_payload_bin->md)));
grpc_slice_buffer_stream_init(&calld->read_stream, grpc_slice_buffer_stream_init(&calld->read_stream,
&calld->read_slice_buffer, 0); &calld->read_slice_buffer, 0);
grpc_metadata_batch_remove(b, b->idx.named.grpc_payload_bin); grpc_metadata_batch_remove(exec_ctx, b, b->idx.named.grpc_payload_bin);
} }
return error; return error;

@ -866,7 +866,7 @@ static void recv_common_filter(grpc_exec_ctx *exec_ctx, grpc_call *call,
GPR_TIMER_BEGIN("status", 0); GPR_TIMER_BEGIN("status", 0);
set_status_code(call, STATUS_FROM_WIRE, set_status_code(call, STATUS_FROM_WIRE,
decode_status(b->idx.named.grpc_status->md)); decode_status(b->idx.named.grpc_status->md));
grpc_metadata_batch_remove(b, b->idx.named.grpc_status); grpc_metadata_batch_remove(exec_ctx, b, b->idx.named.grpc_status);
GPR_TIMER_END("status", 0); GPR_TIMER_END("status", 0);
} }
@ -875,7 +875,7 @@ static void recv_common_filter(grpc_exec_ctx *exec_ctx, grpc_call *call,
set_status_details( set_status_details(
exec_ctx, call, STATUS_FROM_WIRE, exec_ctx, call, STATUS_FROM_WIRE,
grpc_slice_ref_internal(GRPC_MDVALUE(b->idx.named.grpc_message->md))); grpc_slice_ref_internal(GRPC_MDVALUE(b->idx.named.grpc_message->md)));
grpc_metadata_batch_remove(b, b->idx.named.grpc_message); grpc_metadata_batch_remove(exec_ctx, b, b->idx.named.grpc_message);
GPR_TIMER_END("status-details", 0); GPR_TIMER_END("status-details", 0);
} }
} }
@ -910,14 +910,14 @@ static void recv_initial_filter(grpc_exec_ctx *exec_ctx, grpc_call *call,
set_incoming_compression_algorithm( set_incoming_compression_algorithm(
call, decode_compression(b->idx.named.grpc_encoding->md)); call, decode_compression(b->idx.named.grpc_encoding->md));
GPR_TIMER_END("incoming_compression_algorithm", 0); GPR_TIMER_END("incoming_compression_algorithm", 0);
grpc_metadata_batch_remove(b, b->idx.named.grpc_encoding); grpc_metadata_batch_remove(exec_ctx, b, b->idx.named.grpc_encoding);
} }
if (b->idx.named.grpc_accept_encoding != NULL) { if (b->idx.named.grpc_accept_encoding != NULL) {
GPR_TIMER_BEGIN("encodings_accepted_by_peer", 0); GPR_TIMER_BEGIN("encodings_accepted_by_peer", 0);
set_encodings_accepted_by_peer(exec_ctx, call, set_encodings_accepted_by_peer(exec_ctx, call,
b->idx.named.grpc_accept_encoding->md); b->idx.named.grpc_accept_encoding->md);
grpc_metadata_batch_remove(b, b->idx.named.grpc_accept_encoding); grpc_metadata_batch_remove(exec_ctx, b, b->idx.named.grpc_accept_encoding);
GPR_TIMER_END("encodings_accepted_by_peer", 0); GPR_TIMER_END("encodings_accepted_by_peer", 0);
} }

@ -260,6 +260,14 @@ grpc_mdelem grpc_mdelem_create(
allocated->key = grpc_slice_ref_internal(key); allocated->key = grpc_slice_ref_internal(key);
allocated->value = grpc_slice_ref_internal(value); allocated->value = grpc_slice_ref_internal(value);
gpr_atm_rel_store(&allocated->refcnt, 1); gpr_atm_rel_store(&allocated->refcnt, 1);
#ifdef GRPC_METADATA_REFCOUNT_DEBUG
char *key_str = grpc_dump_slice(allocated->key, GPR_DUMP_ASCII);
char *value_str = grpc_dump_slice(allocated->value, GPR_DUMP_ASCII);
gpr_log(GPR_DEBUG, "ELM ALLOC:%p:%zu: '%s' = '%s'", (void *)allocated,
gpr_atm_no_barrier_load(&allocated->refcnt), key_str, value_str);
gpr_free(key_str);
gpr_free(value_str);
#endif
return GRPC_MAKE_MDELEM(allocated, GRPC_MDELEM_STORAGE_ALLOCATED); return GRPC_MAKE_MDELEM(allocated, GRPC_MDELEM_STORAGE_ALLOCATED);
} }

@ -148,7 +148,7 @@ void *grpc_mdelem_set_user_data(grpc_mdelem md, void (*destroy_func)(void *),
void *user_data); void *user_data);
/* Reference counting */ /* Reference counting */
//#define GRPC_METADATA_REFCOUNT_DEBUG #define GRPC_METADATA_REFCOUNT_DEBUG
#ifdef GRPC_METADATA_REFCOUNT_DEBUG #ifdef GRPC_METADATA_REFCOUNT_DEBUG
#define GRPC_MDELEM_REF(s) grpc_mdelem_ref((s), __FILE__, __LINE__) #define GRPC_MDELEM_REF(s) grpc_mdelem_ref((s), __FILE__, __LINE__)
#define GRPC_MDELEM_UNREF(exec_ctx, s) \ #define GRPC_MDELEM_UNREF(exec_ctx, s) \

@ -228,11 +228,13 @@ static void unlink_storage(grpc_mdelem_list *list,
assert_valid_list(list); assert_valid_list(list);
} }
void grpc_metadata_batch_remove(grpc_metadata_batch *batch, void grpc_metadata_batch_remove(grpc_exec_ctx *exec_ctx,
grpc_metadata_batch *batch,
grpc_linked_mdelem *storage) { grpc_linked_mdelem *storage) {
assert_valid_callouts(batch); assert_valid_callouts(batch);
maybe_unlink_callout(batch, storage); maybe_unlink_callout(batch, storage);
unlink_storage(&batch->list, storage); unlink_storage(&batch->list, storage);
GRPC_MDELEM_UNREF(exec_ctx, storage->md);
assert_valid_callouts(batch); assert_valid_callouts(batch);
} }

@ -81,7 +81,8 @@ bool grpc_metadata_batch_is_empty(grpc_metadata_batch *batch);
size_t grpc_metadata_batch_size(grpc_metadata_batch *batch); size_t grpc_metadata_batch_size(grpc_metadata_batch *batch);
/** Remove \a storage from the batch, unreffing the mdelem contained */ /** Remove \a storage from the batch, unreffing the mdelem contained */
void grpc_metadata_batch_remove(grpc_metadata_batch *batch, void grpc_metadata_batch_remove(grpc_exec_ctx *exec_ctx,
grpc_metadata_batch *batch,
grpc_linked_mdelem *storage); grpc_linked_mdelem *storage);
/** Substitute a new mdelem for an old value */ /** Substitute a new mdelem for an old value */

Loading…
Cancel
Save