Merge branch 'master' into config-isolation

3 files changed, 20 insertions, 11 deletions

diff --git a/src/core/ext/transport/chttp2/client/chttp2_connector.cc b/src/core/ext/transport/chttp2/client/chttp2_connector.cc
index 5229304fa4..60a32022f5 100644
--- a/src/core/ext/transport/chttp2/client/chttp2_connector.cc
+++ b/src/core/ext/transport/chttp2/client/chttp2_connector.cc
@@ -163,9 +163,8 @@ static void start_handshake_locked(chttp2_connector* c) {
                        c->args.interested_parties, c->handshake_mgr);
   grpc_endpoint_add_to_pollset_set(c->endpoint, c->args.interested_parties);
   grpc_handshake_manager_do_handshake(
-      c->handshake_mgr, c->args.interested_parties, c->endpoint,
-      c->args.channel_args, c->args.deadline, nullptr /* acceptor */,
-      on_handshake_done, c);
+      c->handshake_mgr, c->endpoint, c->args.channel_args, c->args.deadline,
+      nullptr /* acceptor */, on_handshake_done, c);
   c->endpoint = nullptr;  // Endpoint handed off to handshake manager.
 }
 
@@ -213,9 +212,17 @@ static void chttp2_connector_connect(grpc_connector* con,
   GRPC_CLOSURE_INIT(&c->connected, connected, c, grpc_schedule_on_exec_ctx);
   GPR_ASSERT(!c->connecting);
   c->connecting = true;
-  grpc_tcp_client_connect(&c->connected, &c->endpoint, args->interested_parties,
-                          args->channel_args, &addr, args->deadline);
+  grpc_closure* closure = &c->connected;
+  grpc_endpoint** ep = &c->endpoint;
   gpr_mu_unlock(&c->mu);
+  // In some implementations, the closure can be flushed before
+  // grpc_tcp_client_connect and since the closure requires access to c->mu,
+  // this can result in a deadlock. Refer
+  // https://github.com/grpc/grpc/issues/16427
+  // grpc_tcp_client_connect would fill c->endpoint with proper contents and we
+  // make sure that we would still exist at that point by taking a ref.
+  grpc_tcp_client_connect(closure, ep, args->interested_parties,
+                          args->channel_args, &addr, args->deadline);
 }
 
 static const grpc_connector_vtable chttp2_connector_vtable = {
diff --git a/src/core/ext/transport/chttp2/server/chttp2_server.cc b/src/core/ext/transport/chttp2/server/chttp2_server.cc
index 6ed88dfb5e..07b304b320 100644
--- a/src/core/ext/transport/chttp2/server/chttp2_server.cc
+++ b/src/core/ext/transport/chttp2/server/chttp2_server.cc
@@ -208,10 +208,10 @@ static void on_accept(void* arg, grpc_endpoint* tcp,
       grpc_core::ExecCtx::Get()->Now() +
       grpc_channel_arg_get_integer(timeout_arg,
                                    {120 * GPR_MS_PER_SEC, 1, INT_MAX});
-  grpc_handshake_manager_do_handshake(
-      connection_state->handshake_mgr, nullptr /* interested_parties */, tcp,
-      state->args, connection_state->deadline, acceptor, on_handshake_done,
-      connection_state);
+  grpc_handshake_manager_do_handshake(connection_state->handshake_mgr, tcp,
+                                      state->args, connection_state->deadline,
+                                      acceptor, on_handshake_done,
+                                      connection_state);
 }
 
 /* Server callback: start listening on our ports */
diff --git a/src/core/ext/transport/chttp2/transport/parsing.cc b/src/core/ext/transport/chttp2/transport/parsing.cc
index f532b084c9..1ff96d3cd3 100644
--- a/src/core/ext/transport/chttp2/transport/parsing.cc
+++ b/src/core/ext/transport/chttp2/transport/parsing.cc
@@ -368,6 +368,7 @@ static grpc_error* init_data_frame_parser(grpc_chttp2_transport* t) {
         &s->data_parser, t->incoming_frame_flags, s->id, s);
   }
 error_handler:
+  intptr_t unused;
   if (err == GRPC_ERROR_NONE) {
     t->incoming_stream = s;
     /* t->parser = grpc_chttp2_data_parser_parse;*/
@@ -375,7 +376,7 @@ error_handler:
     t->parser_data = &s->data_parser;
     t->ping_state.last_ping_sent_time = GRPC_MILLIS_INF_PAST;
     return GRPC_ERROR_NONE;
-  } else if (grpc_error_get_int(err, GRPC_ERROR_INT_STREAM_ID, nullptr)) {
+  } else if (grpc_error_get_int(err, GRPC_ERROR_INT_STREAM_ID, &unused)) {
     /* handle stream errors by closing the stream */
     if (s != nullptr) {
       grpc_chttp2_mark_stream_closed(t, s, true, false, err);
@@ -756,9 +757,10 @@ static grpc_error* parse_frame_slice(grpc_chttp2_transport* t, grpc_slice slice,
                                      int is_last) {
   grpc_chttp2_stream* s = t->incoming_stream;
   grpc_error* err = t->parser(t->parser_data, t, s, slice, is_last);
+  intptr_t unused;
   if (GPR_LIKELY(err == GRPC_ERROR_NONE)) {
     return err;
-  } else if (grpc_error_get_int(err, GRPC_ERROR_INT_STREAM_ID, nullptr)) {
+  } else if (grpc_error_get_int(err, GRPC_ERROR_INT_STREAM_ID, &unused)) {
     if (grpc_http_trace.enabled()) {
       const char* msg = grpc_error_string(err);
       gpr_log(GPR_ERROR, "%s", msg);