From 0c1d8e2086b055b6e322e00e52223ec5359fca4b Mon Sep 17 00:00:00 2001
From: Sree Kuchibhotla <sreek@google.com>
Date: Mon, 6 Nov 2017 10:25:28 -0800
Subject: [PATCH] Review comment: merge seen_goaway and goaway_error fields on
 transport

---
 .../chttp2/transport/chttp2_transport.cc      | 32 ++++++++++---------
 .../ext/transport/chttp2/transport/internal.h | 10 +++---
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc
index 42664a753e5..2a156f5f7ec 100644
--- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc
+++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc
@@ -205,6 +205,8 @@ static void destruct_transport(grpc_exec_ctx *exec_ctx,
     GPR_ASSERT(t->lists[i].tail == NULL);
   }
 
+  GRPC_ERROR_UNREF(t->goaway_error);
+
   GPR_ASSERT(grpc_chttp2_stream_map_size(&t->stream_map) == 0);
 
   grpc_chttp2_stream_map_destroy(&t->stream_map);
@@ -320,6 +322,7 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
                     keepalive_watchdog_fired_locked, t,
                     grpc_combiner_scheduler(t->combiner));
 
+  t->goaway_error = GRPC_ERROR_NONE;
   grpc_chttp2_goaway_parser_init(&t->goaway_parser);
   grpc_chttp2_hpack_parser_init(exec_ctx, &t->hpack_parser);
 
@@ -1120,8 +1123,16 @@ void grpc_chttp2_add_incoming_goaway(grpc_exec_ctx *exec_ctx,
                                      grpc_slice goaway_text) {
   // GRPC_CHTTP2_IF_TRACING(
   //     gpr_log(GPR_DEBUG, "got goaway [%d]: %s", goaway_error, msg));
-  t->seen_goaway = 1;
-  t->goaway_error = goaway_error;
+
+  // Discard the error from a previous goaway frame (if any)
+  if (t->goaway_error != GRPC_ERROR_NONE) {
+    GRPC_ERROR_UNREF(t->goaway_error);
+  }
+  t->goaway_error = grpc_error_set_str(
+      grpc_error_set_int(
+          GRPC_ERROR_CREATE_FROM_STATIC_STRING("GOAWAY received"),
+          GRPC_ERROR_INT_HTTP2_ERROR, (intptr_t)goaway_error),
+      GRPC_ERROR_STR_RAW_BYTES, goaway_text);
 
   /* When a client receives a GOAWAY with error code ENHANCE_YOUR_CALM and debug
    * data equal to "too_many_pings", it should log the occurrence at a log level
@@ -1142,14 +1153,8 @@ void grpc_chttp2_add_incoming_goaway(grpc_exec_ctx *exec_ctx,
 
   /* lie: use transient failure from the transport to indicate goaway has been
    * received */
-  connectivity_state_set(
-      exec_ctx, t, GRPC_CHANNEL_TRANSIENT_FAILURE,
-      grpc_error_set_str(
-          grpc_error_set_int(
-              GRPC_ERROR_CREATE_FROM_STATIC_STRING("GOAWAY received"),
-              GRPC_ERROR_INT_HTTP2_ERROR, (intptr_t)goaway_error),
-          GRPC_ERROR_STR_RAW_BYTES, goaway_text),
-      "got_goaway");
+  connectivity_state_set(exec_ctx, t, GRPC_CHANNEL_TRANSIENT_FAILURE,
+                         GRPC_ERROR_REF(t->goaway_error), "got_goaway");
 }
 
 static void maybe_start_some_streams(grpc_exec_ctx *exec_ctx,
@@ -2544,11 +2549,8 @@ static void read_action_locked(grpc_exec_ctx *exec_ctx, void *tp,
   if (error != GRPC_ERROR_NONE) {
     /* If a goaway frame was received, this might be the reason why the read
      * failed. Add this info to the error */
-    if (t->seen_goaway) {
-      error = grpc_error_add_child(
-          error, grpc_error_set_int(
-                     GRPC_ERROR_CREATE_FROM_STATIC_STRING("GOAWAY received"),
-                     GRPC_ERROR_INT_HTTP2_ERROR, (intptr_t)t->goaway_error));
+    if (t->goaway_error != GRPC_ERROR_NONE) {
+      error = grpc_error_add_child(error, GRPC_ERROR_REF(t->goaway_error));
     }
 
     close_transport_locked(exec_ctx, t, GRPC_ERROR_REF(error));
diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h
index bd25e51ec06..3a20b653a45 100644
--- a/src/core/ext/transport/chttp2/transport/internal.h
+++ b/src/core/ext/transport/chttp2/transport/internal.h
@@ -306,13 +306,11 @@ struct grpc_chttp2_transport {
    */
   uint32_t write_buffer_size;
 
-  /** have we seen a goaway */
-  bool seen_goaway;
-  /** have we sent a goaway */
+  /** Set to a grpc_error object if a goaway frame is received. By default, set
+   * to GRPC_ERROR_NONE */
+  grpc_error *goaway_error;
+
   grpc_chttp2_sent_goaway_state sent_goaway_state;
-  /** http2 error code received in the GOAWAY frame. Only relevant if
-   * seen_goaway is true */
-  uint32_t goaway_error;
 
   /** are the local settings dirty and need to be sent? */
   bool dirtied_local_settings;