From 40d443de1b7cb6380e23d4fe97027f2a7541b1f8 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Tue, 23 Oct 2018 15:58:39 -0700 Subject: Fix deadlock issue in connector --- src/core/ext/transport/chttp2/client/chttp2_connector.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'src/core/ext/transport/chttp2/client/chttp2_connector.cc') diff --git a/src/core/ext/transport/chttp2/client/chttp2_connector.cc b/src/core/ext/transport/chttp2/client/chttp2_connector.cc index 5229304fa4..cfce2280fc 100644 --- a/src/core/ext/transport/chttp2/client/chttp2_connector.cc +++ b/src/core/ext/transport/chttp2/client/chttp2_connector.cc @@ -213,9 +213,11 @@ 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); + grpc_tcp_client_connect(closure, ep, args->interested_parties, + args->channel_args, &addr, args->deadline); } static const grpc_connector_vtable chttp2_connector_vtable = { -- cgit v1.2.3 From cad9be088920b6e02a91d21a78f0e98d8c8d9f20 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Thu, 25 Oct 2018 11:55:14 -0700 Subject: Add comments about the fix --- src/core/ext/transport/chttp2/client/chttp2_connector.cc | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'src/core/ext/transport/chttp2/client/chttp2_connector.cc') diff --git a/src/core/ext/transport/chttp2/client/chttp2_connector.cc b/src/core/ext/transport/chttp2/client/chttp2_connector.cc index cfce2280fc..aca9894e82 100644 --- a/src/core/ext/transport/chttp2/client/chttp2_connector.cc +++ b/src/core/ext/transport/chttp2/client/chttp2_connector.cc @@ -216,6 +216,12 @@ static void chttp2_connector_connect(grpc_connector* con, 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); } -- cgit v1.2.3