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

3 files changed, 17 insertions, 10 deletions

diff --git a/include/grpc++/impl/codegen/call.h b/include/grpc++/impl/codegen/call.h
index aea1a6acec..d081b7d9c5 100644
--- a/include/grpc++/impl/codegen/call.h
+++ b/include/grpc++/impl/codegen/call.h
@@ -281,10 +281,9 @@ class CallOpRecvMessage {
     if (message_ == nullptr) return;
     if (recv_buf_) {
       if (*status) {
-        got_message = true;
-        *status = SerializationTraits<R>::Deserialize(recv_buf_, message_,
-                                                      max_message_size)
-                      .ok();
+        got_message = *status = SerializationTraits<R>::Deserialize(
+                                    recv_buf_, message_, max_message_size)
+                                    .ok();
       } else {
         got_message = false;
         g_core_codegen_interface->grpc_byte_buffer_destroy(recv_buf_);
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? */