Merge github.com:grpc/grpc into lfe3

10 files changed, 72 insertions, 33 deletions

diff --git a/src/core/ext/filters/client_channel/backup_poller.cc b/src/core/ext/filters/client_channel/backup_poller.cc
index 0236b41374..ed437d255c 100644
--- a/src/core/ext/filters/client_channel/backup_poller.cc
+++ b/src/core/ext/filters/client_channel/backup_poller.cc
@@ -143,9 +143,16 @@ void grpc_client_channel_start_backup_polling(
                     grpc_exec_ctx_now(exec_ctx) + g_poll_interval_ms,
                     &g_poller->run_poller_closure);
   }
+
   gpr_ref(&g_poller->refs);
+  /* Get a reference to g_poller->pollset before releasing g_poller_mu to make
+   * TSAN happy. Otherwise, reading from g_poller (i.e g_poller->pollset) after
+   * releasing the lock and setting g_poller to NULL in g_poller_unref() is
+   * being flagged as a data-race by TSAN */
+  grpc_pollset* pollset = g_poller->pollset;
   gpr_mu_unlock(&g_poller_mu);
-  grpc_pollset_set_add_pollset(exec_ctx, interested_parties, g_poller->pollset);
+
+  grpc_pollset_set_add_pollset(exec_ctx, interested_parties, pollset);
 }
 
 void grpc_client_channel_stop_backup_polling(
diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc
index 272808b20f..bd74e2e4b6 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 == nullptr);
   }
 
+  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);
 
@@ -1123,7 +1126,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;
+
+  // 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
@@ -1144,14 +1156,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,
@@ -2078,7 +2084,6 @@ void grpc_chttp2_fake_status(grpc_exec_ctx* exec_ctx, grpc_chttp2_transport* t,
   grpc_status_code status;
   grpc_slice slice;
   grpc_error_get_status(exec_ctx, error, s->deadline, &status, &slice, nullptr);
-
   if (status != GRPC_STATUS_OK) {
     s->seen_error = true;
   }
@@ -2546,6 +2551,12 @@ static void read_action_locked(grpc_exec_ctx* exec_ctx, void* tp,
         "Transport closed", &t->closed_with_error, 1);
   }
   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->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));
     t->endpoint_reading = 0;
   } else if (t->closed_with_error == GRPC_ERROR_NONE) {
diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h
index a5a0a804a2..60cc280c43 100644
--- a/src/core/ext/transport/chttp2/transport/internal.h
+++ b/src/core/ext/transport/chttp2/transport/internal.h
@@ -306,9 +306,10 @@ 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;
 
   /** are the local settings dirty and need to be sent? */
@@ -376,11 +377,6 @@ struct grpc_chttp2_transport {
                         grpc_chttp2_transport* t, grpc_chttp2_stream* s,
                         grpc_slice slice, int is_last);
 
-  /* goaway data */
-  grpc_status_code goaway_error;
-  uint32_t goaway_last_stream_index;
-  grpc_slice goaway_text;
-
   grpc_chttp2_write_cb* write_cb_pool;
 
   /* bdp estimator */
diff --git a/src/core/ext/transport/chttp2/transport/parsing.cc b/src/core/ext/transport/chttp2/transport/parsing.cc
index 98dd27d738..5f07dec8e8 100644
--- a/src/core/ext/transport/chttp2/transport/parsing.cc
+++ b/src/core/ext/transport/chttp2/transport/parsing.cc
@@ -590,7 +590,11 @@ static grpc_error* init_header_frame_parser(grpc_exec_ctx* exec_ctx,
         GRPC_CHTTP2_IF_TRACING(gpr_log(
             GPR_ERROR, "ignoring new grpc_chttp2_stream creation on client"));
       }
-      return init_skip_frame_parser(exec_ctx, t, 1);
+      grpc_error* err = init_skip_frame_parser(exec_ctx, t, 1);
+      if (t->incoming_frame_flags & GRPC_CHTTP2_FLAG_HAS_PRIORITY) {
+        grpc_chttp2_hpack_parser_set_has_priority(&t->hpack_parser);
+      }
+      return err;
     } else if (t->last_new_stream_id >= t->incoming_stream_id) {
       GRPC_CHTTP2_IF_TRACING(gpr_log(
           GPR_ERROR,
diff --git a/src/core/lib/iomgr/ev_epoll1_linux.cc b/src/core/lib/iomgr/ev_epoll1_linux.cc
index 75f94524c8..918bc6f933 100644
--- a/src/core/lib/iomgr/ev_epoll1_linux.cc
+++ b/src/core/lib/iomgr/ev_epoll1_linux.cc
@@ -18,6 +18,8 @@
 
 #include "src/core/lib/iomgr/port.h"
 
+#include <grpc/support/log.h>
+
 /* This polling engine is only relevant on linux kernels supporting epoll() */
 #ifdef GRPC_LINUX_EPOLL
 #include "src/core/lib/iomgr/ev_epoll1_linux.h"
@@ -34,7 +36,6 @@
 
 #include <grpc/support/alloc.h>
 #include <grpc/support/cpu.h>
-#include <grpc/support/log.h>
 #include <grpc/support/string_util.h>
 #include <grpc/support/tls.h>
 #include <grpc/support/useful.h>
@@ -1230,6 +1231,7 @@ static const grpc_event_engine_vtable vtable = {
  * support is available */
 const grpc_event_engine_vtable* grpc_init_epoll1_linux(bool explicit_request) {
   if (!grpc_has_wakeup_fd()) {
+    gpr_log(GPR_ERROR, "Skipping epoll1 because of no wakeup fd.");
     return nullptr;
   }
 
@@ -1254,6 +1256,8 @@ const grpc_event_engine_vtable* grpc_init_epoll1_linux(bool explicit_request) {
 /* If GRPC_LINUX_EPOLL is not defined, it means epoll is not available. Return
  * NULL */
 const grpc_event_engine_vtable* grpc_init_epoll1_linux(bool explicit_request) {
+  gpr_log(GPR_ERROR,
+          "Skipping epoll1 becuase GRPC_LINUX_EPOLL is not defined.");
   return NULL;
 }
 #endif /* defined(GRPC_POSIX_SOCKET) */
diff --git a/src/core/lib/iomgr/ev_epollex_linux.cc b/src/core/lib/iomgr/ev_epollex_linux.cc
index 0fe74b2f3f..bfd2ac4326 100644
--- a/src/core/lib/iomgr/ev_epollex_linux.cc
+++ b/src/core/lib/iomgr/ev_epollex_linux.cc
@@ -18,6 +18,8 @@
 
 #include "src/core/lib/iomgr/port.h"
 
+#include <grpc/support/log.h>
+
 /* This polling engine is only relevant on linux kernels supporting epoll() */
 #ifdef GRPC_LINUX_EPOLL
 
@@ -34,7 +36,6 @@
 #include <unistd.h>
 
 #include <grpc/support/alloc.h>
-#include <grpc/support/log.h>
 #include <grpc/support/string_util.h>
 #include <grpc/support/tls.h>
 #include <grpc/support/useful.h>
@@ -1451,10 +1452,12 @@ const grpc_event_engine_vtable* grpc_init_epollex_linux(
   }
 
   if (!grpc_has_wakeup_fd()) {
+    gpr_log(GPR_ERROR, "Skipping epollex because of no wakeup fd.");
     return nullptr;
   }
 
   if (!grpc_is_epollexclusive_available()) {
+    gpr_log(GPR_INFO, "Skipping epollex because it is not supported.");
     return nullptr;
   }
 
@@ -1480,6 +1483,8 @@ const grpc_event_engine_vtable* grpc_init_epollex_linux(
  * NULL */
 const grpc_event_engine_vtable* grpc_init_epollex_linux(
     bool explicitly_requested) {
+  gpr_log(GPR_ERROR,
+          "Skipping epollex becuase GRPC_LINUX_EPOLL is not defined.");
   return NULL;
 }
 #endif /* defined(GRPC_POSIX_SOCKET) */
diff --git a/src/core/lib/iomgr/ev_epollsig_linux.cc b/src/core/lib/iomgr/ev_epollsig_linux.cc
index 8cdd06d659..5b4a7ba19c 100644
--- a/src/core/lib/iomgr/ev_epollsig_linux.cc
+++ b/src/core/lib/iomgr/ev_epollsig_linux.cc
@@ -19,6 +19,7 @@
 #include "src/core/lib/iomgr/port.h"
 
 #include <grpc/grpc_posix.h>
+#include <grpc/support/log.h>
 
 /* This polling engine is only relevant on linux kernels supporting epoll() */
 #ifdef GRPC_LINUX_EPOLL
@@ -37,7 +38,6 @@
 #include <unistd.h>
 
 #include <grpc/support/alloc.h>
-#include <grpc/support/log.h>
 #include <grpc/support/string_util.h>
 #include <grpc/support/tls.h>
 #include <grpc/support/useful.h>
@@ -1711,14 +1711,17 @@ const grpc_event_engine_vtable* grpc_init_epollsig_linux(
     bool explicit_request) {
   /* If use of signals is disabled, we cannot use epoll engine*/
   if (is_grpc_wakeup_signal_initialized && grpc_wakeup_signal < 0) {
+    gpr_log(GPR_ERROR, "Skipping epollsig because use of signals is disabled.");
     return nullptr;
   }
 
   if (!grpc_has_wakeup_fd()) {
+    gpr_log(GPR_ERROR, "Skipping epollsig because of no wakeup fd.");
     return nullptr;
   }
 
   if (!is_epoll_available()) {
+    gpr_log(GPR_ERROR, "Skipping epollsig because epoll is unavailable.");
     return nullptr;
   }
 
@@ -1726,6 +1729,8 @@ const grpc_event_engine_vtable* grpc_init_epollsig_linux(
     if (explicit_request) {
       grpc_use_signal(SIGRTMIN + 6);
     } else {
+      gpr_log(GPR_ERROR,
+              "Skipping epollsig because uninitialized wakeup signal.");
       return nullptr;
     }
   }
@@ -1751,6 +1756,8 @@ const grpc_event_engine_vtable* grpc_init_epollsig_linux(
  * NULL */
 const grpc_event_engine_vtable* grpc_init_epollsig_linux(
     bool explicit_request) {
+  gpr_log(GPR_ERROR,
+          "Skipping epollsig becuase GRPC_LINUX_EPOLL is not defined.");
   return NULL;
 }
 #endif /* defined(GRPC_POSIX_SOCKET) */
diff --git a/src/core/lib/iomgr/ev_poll_posix.cc b/src/core/lib/iomgr/ev_poll_posix.cc
index 75dea800fd..f8b9629462 100644
--- a/src/core/lib/iomgr/ev_poll_posix.cc
+++ b/src/core/lib/iomgr/ev_poll_posix.cc
@@ -1712,6 +1712,7 @@ static const grpc_event_engine_vtable vtable = {
 
 const grpc_event_engine_vtable* grpc_init_poll_posix(bool explicit_request) {
   if (!grpc_has_wakeup_fd()) {
+    gpr_log(GPR_ERROR, "Skipping poll because of no wakeup fd.");
     return nullptr;
   }
   if (!GRPC_LOG_IF_ERROR("pollset_global_init", pollset_global_init())) {
diff --git a/src/core/lib/iomgr/ev_posix.cc b/src/core/lib/iomgr/ev_posix.cc
index d73cd9f45b..076d2e6bab 100644
--- a/src/core/lib/iomgr/ev_posix.cc
+++ b/src/core/lib/iomgr/ev_posix.cc
@@ -172,12 +172,12 @@ void grpc_event_engine_init(void) {
     gpr_free(strings[i]);
   }
   gpr_free(strings);
-  gpr_free(s);
 
   if (g_event_engine == nullptr) {
-    gpr_log(GPR_ERROR, "No event engine could be initialized");
+    gpr_log(GPR_ERROR, "No event engine could be initialized from %s", s);
     abort();
   }
+  gpr_free(s);
 }
 
 void grpc_event_engine_shutdown(void) {
diff --git a/src/core/lib/iomgr/pollset_uv.cc b/src/core/lib/iomgr/pollset_uv.cc
index 6b9c53c01c..1d54942c1d 100644
--- a/src/core/lib/iomgr/pollset_uv.cc
+++ b/src/core/lib/iomgr/pollset_uv.cc
@@ -40,7 +40,7 @@ grpc_tracer_flag grpc_trace_fd_refcount =
 #endif
 
 struct grpc_pollset {
-  uv_timer_t timer;
+  uv_timer_t* timer;
   int shutting_down;
 };
 
@@ -78,12 +78,16 @@ void grpc_pollset_global_shutdown(void) {
 
 static void timer_run_cb(uv_timer_t* timer) {}
 
-static void timer_close_cb(uv_handle_t* handle) { handle->data = (void*)1; }
+static void timer_close_cb(uv_handle_t* handle) {
+  handle->data = (void*)1;
+  gpr_free(handle);
+}
 
 void grpc_pollset_init(grpc_pollset* pollset, gpr_mu** mu) {
   GRPC_UV_ASSERT_SAME_THREAD();
   *mu = &grpc_polling_mu;
-  uv_timer_init(uv_default_loop(), &pollset->timer);
+  pollset->timer = (uv_timer_t*)gpr_malloc(sizeof(uv_timer_t));
+  uv_timer_init(uv_default_loop(), pollset->timer);
   pollset->shutting_down = 0;
 }
 
@@ -104,11 +108,11 @@ void grpc_pollset_shutdown(grpc_exec_ctx* exec_ctx, grpc_pollset* pollset,
 
 void grpc_pollset_destroy(grpc_exec_ctx* exec_ctx, grpc_pollset* pollset) {
   GRPC_UV_ASSERT_SAME_THREAD();
-  uv_close((uv_handle_t*)&pollset->timer, timer_close_cb);
+  uv_close((uv_handle_t*)pollset->timer, timer_close_cb);
   // timer.data is a boolean indicating that the timer has finished closing
-  pollset->timer.data = (void*)0;
+  pollset->timer->data = (void*)0;
   if (grpc_pollset_work_run_loop) {
-    while (!pollset->timer.data) {
+    while (!pollset->timer->data) {
       uv_run(uv_default_loop(), UV_RUN_NOWAIT);
     }
   }
@@ -130,11 +134,11 @@ grpc_error* grpc_pollset_work(grpc_exec_ctx* exec_ctx, grpc_pollset* pollset,
     /* We special-case timeout=0 so that we don't bother with the timer when
        the loop won't block anyway */
     if (timeout > 0) {
-      uv_timer_start(&pollset->timer, timer_run_cb, timeout, 0);
+      uv_timer_start(pollset->timer, timer_run_cb, timeout, 0);
       /* Run until there is some I/O activity or the timer triggers. It doesn't
          matter which happens */
       uv_run(uv_default_loop(), UV_RUN_ONCE);
-      uv_timer_stop(&pollset->timer);
+      uv_timer_stop(pollset->timer);
     } else {
       uv_run(uv_default_loop(), UV_RUN_NOWAIT);
     }