From 4ee1a627230c8564dfdab5247e8703e0eb10d5c8 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 29 Apr 2016 16:47:27 -0700 Subject: Stress test fixes - properly fail a Read() on a stream if we fail to parse a protobuf - fix an ordering problem with the chttp2 transport global lock, whereby a sequence of two operations could be swapped - this resulted in slices being returned to the upper layers in the wrong order, corrupting data --- .../ext/transport/chttp2/transport/chttp2_transport.c | 17 ++++++++++++----- src/core/ext/transport/chttp2/transport/internal.h | 3 ++- 2 files changed, 14 insertions(+), 6 deletions(-) (limited to 'src/core/ext/transport') diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c index fcf2abfe66..8c8593748d 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c @@ -629,9 +629,10 @@ static void finish_global_actions(grpc_exec_ctx *exec_ctx, check_read_ops(exec_ctx, &t->global); gpr_mu_lock(&t->executor.mu); - if (t->executor.pending_actions != NULL) { - hdr = t->executor.pending_actions; - t->executor.pending_actions = NULL; + if (t->executor.pending_actions_head != NULL) { + hdr = t->executor.pending_actions_head; + t->executor.pending_actions_head = t->executor.pending_actions_tail = + NULL; gpr_mu_unlock(&t->executor.mu); while (hdr != NULL) { hdr->action(exec_ctx, t, hdr->stream, hdr->arg); @@ -686,8 +687,14 @@ void grpc_chttp2_run_with_global_lock(grpc_exec_ctx *exec_ctx, gpr_free(hdr); continue; } - hdr->next = t->executor.pending_actions; - t->executor.pending_actions = hdr; + hdr->next = NULL; + if (t->executor.pending_actions_head != NULL) { + t->executor.pending_actions_tail = + t->executor.pending_actions_tail->next = hdr; + } else { + t->executor.pending_actions_tail = t->executor.pending_actions_head = + hdr; + } REF_TRANSPORT(t, "pending_action"); gpr_mu_unlock(&t->executor.mu); } diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h index 7a8084641d..a269338b49 100644 --- a/src/core/ext/transport/chttp2/transport/internal.h +++ b/src/core/ext/transport/chttp2/transport/internal.h @@ -321,7 +321,8 @@ struct grpc_chttp2_transport { /** is a thread currently parsing */ bool parsing_active; - grpc_chttp2_executor_action_header *pending_actions; + grpc_chttp2_executor_action_header *pending_actions_head; + grpc_chttp2_executor_action_header *pending_actions_tail; } executor; /** is the transport destroying itself? */ -- cgit v1.2.3