Move channel connection pollset to be across all attempts

Slightly increases the amount of polling we have to do, but lets us move
interest registration under the channel lock, simplifying our code (and
fixing a bug)

2 files changed, 20 insertions, 16 deletions

diff --git a/src/core/channel/client_channel.c b/src/core/channel/client_channel.c
index 31b1fc3bde..726196e996 100644
--- a/src/core/channel/client_channel.c
+++ b/src/core/channel/client_channel.c
@@ -102,10 +102,17 @@ struct call_data {
 static int prepare_activate(grpc_call_element *elem,
                             grpc_child_channel *on_child) {
   call_data *calld = elem->call_data;
+  channel_data *chand = elem->channel_data;
   if (calld->state == CALL_CANCELLED) return 0;
 
   /* no more access to calld->s.waiting allowed */
   GPR_ASSERT(calld->state == CALL_WAITING);
+
+  if (calld->s.waiting_op.bind_pollset) {
+    grpc_transport_setup_del_interested_party(chand->transport_setup,
+                                              calld->s.waiting_op.bind_pollset);
+  }
+
   calld->state = CALL_ACTIVE;
 
   /* create a child call */
@@ -199,6 +206,7 @@ static void cc_start_transport_op(grpc_call_element *elem,
         handle_op_after_cancellation(elem, op);
       } else {
         calld->state = CALL_WAITING;
+        calld->s.waiting_op.bind_pollset = NULL;
         if (chand->active_child) {
           /* channel is connected - use the connected stack */
           if (prepare_activate(elem, chand->active_child)) {
@@ -230,14 +238,14 @@ static void cc_start_transport_op(grpc_call_element *elem,
           }
           calld->s.waiting_op = *op;
           chand->waiting_children[chand->waiting_child_count++] = calld;
+          grpc_transport_setup_add_interested_party(chand->transport_setup,
+                                                    op->bind_pollset);
           gpr_mu_unlock(&chand->mu);
 
           /* finally initiate transport setup if needed */
           if (initiate_transport_setup) {
             grpc_transport_setup_initiate(chand->transport_setup);
           }
-          grpc_transport_setup_add_interested_party(chand->transport_setup,
-                                                    op->bind_pollset);
         }
       }
       break;
diff --git a/src/core/channel/client_setup.c b/src/core/channel/client_setup.c
index 8a318eaa86..16b5e89a3e 100644
--- a/src/core/channel/client_setup.c
+++ b/src/core/channel/client_setup.c
@@ -56,12 +56,12 @@ struct grpc_client_setup {
   gpr_cv cv;
   grpc_client_setup_request *active_request;
   int refs;
+  grpc_pollset_set interested_parties;
 };
 
 struct grpc_client_setup_request {
   /* pointer back to the setup object */
   grpc_client_setup *setup;
-  grpc_pollset_set interested_parties;
   gpr_timespec deadline;
 };
 
@@ -71,7 +71,7 @@ gpr_timespec grpc_client_setup_request_deadline(grpc_client_setup_request *r) {
 
 grpc_pollset_set *grpc_client_setup_get_interested_parties(
     grpc_client_setup_request *r) {
-  return &r->interested_parties;
+  return &r->setup->interested_parties;
 }
 
 static void destroy_setup(grpc_client_setup *s) {
@@ -79,11 +79,11 @@ static void destroy_setup(grpc_client_setup *s) {
   gpr_cv_destroy(&s->cv);
   s->done(s->user_data);
   grpc_channel_args_destroy(s->args);
+  grpc_pollset_set_destroy(&s->interested_parties);
   gpr_free(s);
 }
 
 static void destroy_request(grpc_client_setup_request *r) {
-  grpc_pollset_set_destroy(&r->interested_parties);
   gpr_free(r);
 }
 
@@ -94,7 +94,6 @@ static void setup_initiate(grpc_transport_setup *sp) {
   int in_alarm = 0;
 
   r->setup = s;
-  grpc_pollset_set_init(&r->interested_parties);
   /* TODO(klempner): Actually set a deadline */
   r->deadline = gpr_time_add(gpr_now(), gpr_time_from_seconds(60));
 
@@ -125,12 +124,10 @@ static void setup_add_interested_party(grpc_transport_setup *sp,
   grpc_client_setup *s = (grpc_client_setup *)sp;
 
   gpr_mu_lock(&s->mu);
-  if (!s->active_request) {
-    gpr_mu_unlock(&s->mu);
-    return;
-  }
 
-  grpc_pollset_set_add_pollset(&s->active_request->interested_parties, pollset);
+  gpr_log(GPR_DEBUG, "addip: %p %p", sp, pollset);
+
+  grpc_pollset_set_add_pollset(&s->interested_parties, pollset);
 
   gpr_mu_unlock(&s->mu);
 }
@@ -140,12 +137,10 @@ static void setup_del_interested_party(grpc_transport_setup *sp,
   grpc_client_setup *s = (grpc_client_setup *)sp;
 
   gpr_mu_lock(&s->mu);
-  if (!s->active_request) {
-    gpr_mu_unlock(&s->mu);
-    return;
-  }
 
-  grpc_pollset_set_del_pollset(&s->active_request->interested_parties, pollset);
+  gpr_log(GPR_DEBUG, "delip: %p %p", sp, pollset);
+
+  grpc_pollset_set_del_pollset(&s->interested_parties, pollset);
 
   gpr_mu_unlock(&s->mu);
 }
@@ -225,6 +220,7 @@ void grpc_client_setup_create_and_attach(
   s->in_alarm = 0;
   s->in_cb = 0;
   s->cancelled = 0;
+  grpc_pollset_set_init(&s->interested_parties);
 
   grpc_client_channel_set_transport_setup(newly_minted_channel, &s->base);
 }