Merge pull request #16792 from grpc/revert-16791-revert-16695-pass_pollset_set_tsi_handshaker

Revert "Revert "Create and pass pollset_set to ALTS TSI handshaker""

src/core/ext/filters/client_channel/http_connect_handshaker.cc

@@ -351,6 +351,7 @@ static grpc_handshaker* grpc_http_connect_handshaker_create() {
 
 static void handshaker_factory_add_handshakers(
     grpc_handshaker_factory* factory, const grpc_channel_args* args,
+    grpc_pollset_set* interested_parties,
     grpc_handshake_manager* handshake_mgr) {
   grpc_handshake_manager_add(handshake_mgr,
                              grpc_http_connect_handshaker_create());

src/core/ext/transport/chttp2/client/chttp2_connector.cc

@@ -160,7 +160,7 @@ static void on_handshake_done(void* arg, grpc_error* error) {
 static void start_handshake_locked(chttp2_connector* c) {
   c->handshake_mgr = grpc_handshake_manager_create();
   grpc_handshakers_add(HANDSHAKER_CLIENT, c->args.channel_args,
-                       c->handshake_mgr);
+                       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,

src/core/ext/transport/chttp2/server/chttp2_server.cc

@@ -69,6 +69,7 @@ typedef struct {
   grpc_timer timer;
   grpc_closure on_timeout;
   grpc_closure on_receive_settings;
+  grpc_pollset_set* interested_parties;
 } server_connection_state;
 
 static void server_connection_state_unref(
@@ -78,6 +79,9 @@ static void server_connection_state_unref(
       GRPC_CHTTP2_UNREF_TRANSPORT(connection_state->transport,
                                   "receive settings timeout");
     }
+    grpc_pollset_set_del_pollset(connection_state->interested_parties,
+                                 connection_state->accepting_pollset);
+    grpc_pollset_set_destroy(connection_state->interested_parties);
     gpr_free(connection_state);
   }
 }
@@ -192,7 +196,11 @@ static void on_accept(void* arg, grpc_endpoint* tcp,
   connection_state->accepting_pollset = accepting_pollset;
   connection_state->acceptor = acceptor;
   connection_state->handshake_mgr = handshake_mgr;
+  connection_state->interested_parties = grpc_pollset_set_create();
+  grpc_pollset_set_add_pollset(connection_state->interested_parties,
+                               connection_state->accepting_pollset);
   grpc_handshakers_add(HANDSHAKER_SERVER, state->args,
+                       connection_state->interested_parties,
                        connection_state->handshake_mgr);
   const grpc_arg* timeout_arg =
       grpc_channel_args_find(state->args, GRPC_ARG_SERVER_HANDSHAKE_TIMEOUT_MS);

src/core/lib/channel/handshaker_factory.cc

@@ -24,11 +24,12 @@
 
 void grpc_handshaker_factory_add_handshakers(
     grpc_handshaker_factory* handshaker_factory, const grpc_channel_args* args,
+    grpc_pollset_set* interested_parties,
     grpc_handshake_manager* handshake_mgr) {
   if (handshaker_factory != nullptr) {
     GPR_ASSERT(handshaker_factory->vtable != nullptr);
-    handshaker_factory->vtable->add_handshakers(handshaker_factory, args,
-                                                handshake_mgr);
+    handshaker_factory->vtable->add_handshakers(
+        handshaker_factory, args, interested_parties, handshake_mgr);
   }
 }
 

src/core/lib/channel/handshaker_factory.h

@@ -32,6 +32,7 @@ typedef struct grpc_handshaker_factory grpc_handshaker_factory;
 typedef struct {
   void (*add_handshakers)(grpc_handshaker_factory* handshaker_factory,
                           const grpc_channel_args* args,
+                          grpc_pollset_set* interested_parties,
                           grpc_handshake_manager* handshake_mgr);
   void (*destroy)(grpc_handshaker_factory* handshaker_factory);
 } grpc_handshaker_factory_vtable;
@@ -42,6 +43,7 @@ struct grpc_handshaker_factory {
 
 void grpc_handshaker_factory_add_handshakers(
     grpc_handshaker_factory* handshaker_factory, const grpc_channel_args* args,
+    grpc_pollset_set* interested_parties,
     grpc_handshake_manager* handshake_mgr);
 
 void grpc_handshaker_factory_destroy(

src/core/lib/channel/handshaker_registry.cc

@@ -51,9 +51,11 @@ static void grpc_handshaker_factory_list_register(
 
 static void grpc_handshaker_factory_list_add_handshakers(
     grpc_handshaker_factory_list* list, const grpc_channel_args* args,
+    grpc_pollset_set* interested_parties,
     grpc_handshake_manager* handshake_mgr) {
   for (size_t i = 0; i < list->num_factories; ++i) {
-    grpc_handshaker_factory_add_handshakers(list->list[i], args, handshake_mgr);
+    grpc_handshaker_factory_add_handshakers(list->list[i], args,
+                                            interested_parties, handshake_mgr);
   }
 }
 
@@ -91,7 +93,9 @@ void grpc_handshaker_factory_register(bool at_start,
 
 void grpc_handshakers_add(grpc_handshaker_type handshaker_type,
                           const grpc_channel_args* args,
+                          grpc_pollset_set* interested_parties,
                           grpc_handshake_manager* handshake_mgr) {
   grpc_handshaker_factory_list_add_handshakers(
-      &g_handshaker_factory_lists[handshaker_type], args, handshake_mgr);
+      &g_handshaker_factory_lists[handshaker_type], args, interested_parties,
+      handshake_mgr);
 }

src/core/lib/channel/handshaker_registry.h

@@ -43,6 +43,7 @@ void grpc_handshaker_factory_register(bool at_start,
 
 void grpc_handshakers_add(grpc_handshaker_type handshaker_type,
                           const grpc_channel_args* args,
+                          grpc_pollset_set* interested_parties,
                           grpc_handshake_manager* handshake_mgr);
 
 #endif /* GRPC_CORE_LIB_CHANNEL_HANDSHAKER_REGISTRY_H */

src/core/lib/http/httpcli_security_connector.cc

@@ -29,6 +29,7 @@
 #include "src/core/lib/channel/channel_args.h"
 #include "src/core/lib/channel/handshaker_registry.h"
 #include "src/core/lib/gpr/string.h"
+#include "src/core/lib/iomgr/pollset.h"
 #include "src/core/lib/security/transport/security_handshaker.h"
 #include "src/core/lib/slice/slice_internal.h"
 #include "src/core/tsi/ssl_transport_security.h"
@@ -51,6 +52,7 @@ static void httpcli_ssl_destroy(grpc_security_connector* sc) {
 }
 
 static void httpcli_ssl_add_handshakers(grpc_channel_security_connector* sc,
+                                        grpc_pollset_set* interested_parties,
                                         grpc_handshake_manager* handshake_mgr) {
   grpc_httpcli_ssl_channel_security_connector* c =
       reinterpret_cast<grpc_httpcli_ssl_channel_security_connector*>(sc);
@@ -189,7 +191,8 @@ static void ssl_handshake(void* arg, grpc_endpoint* tcp, const char* host,
   grpc_arg channel_arg = grpc_security_connector_to_arg(&sc->base);
   grpc_channel_args args = {1, &channel_arg};
   c->handshake_mgr = grpc_handshake_manager_create();
-  grpc_handshakers_add(HANDSHAKER_CLIENT, &args, c->handshake_mgr);
+  grpc_handshakers_add(HANDSHAKER_CLIENT, &args,
+                       nullptr /* interested_parties */, c->handshake_mgr);
   grpc_handshake_manager_do_handshake(
       c->handshake_mgr, nullptr /* interested_parties */, tcp,
       nullptr /* channel_args */, deadline, nullptr /* acceptor */,

src/core/lib/security/security_connector/alts_security_connector.cc

@@ -64,29 +64,29 @@ static void alts_server_destroy(grpc_security_connector* sc) {
 }
 
 static void alts_channel_add_handshakers(
-    grpc_channel_security_connector* sc,
+    grpc_channel_security_connector* sc, grpc_pollset_set* interested_parties,
     grpc_handshake_manager* handshake_manager) {
   tsi_handshaker* handshaker = nullptr;
   auto c = reinterpret_cast<grpc_alts_channel_security_connector*>(sc);
   grpc_alts_credentials* creds =
       reinterpret_cast<grpc_alts_credentials*>(c->base.channel_creds);
-  GPR_ASSERT(alts_tsi_handshaker_create(creds->options, c->target_name,
-                                        creds->handshaker_service_url, true,
-                                        &handshaker) == TSI_OK);
+  GPR_ASSERT(alts_tsi_handshaker_create(
+                 creds->options, c->target_name, creds->handshaker_service_url,
+                 true, interested_parties, &handshaker) == TSI_OK);
   grpc_handshake_manager_add(handshake_manager, grpc_security_handshaker_create(
                                                     handshaker, &sc->base));
 }
 
 static void alts_server_add_handshakers(
-    grpc_server_security_connector* sc,
+    grpc_server_security_connector* sc, grpc_pollset_set* interested_parties,
     grpc_handshake_manager* handshake_manager) {
   tsi_handshaker* handshaker = nullptr;
   auto c = reinterpret_cast<grpc_alts_server_security_connector*>(sc);
   grpc_alts_server_credentials* creds =
       reinterpret_cast<grpc_alts_server_credentials*>(c->base.server_creds);
-  GPR_ASSERT(alts_tsi_handshaker_create(creds->options, nullptr,
-                                        creds->handshaker_service_url, false,
-                                        &handshaker) == TSI_OK);
+  GPR_ASSERT(alts_tsi_handshaker_create(
+                 creds->options, nullptr, creds->handshaker_service_url, false,
+                 interested_parties, &handshaker) == TSI_OK);
   grpc_handshake_manager_add(handshake_manager, grpc_security_handshaker_create(
                                                     handshaker, &sc->base));
 }

src/core/lib/security/security_connector/local_security_connector.cc

@@ -30,6 +30,7 @@
 
 #include "src/core/ext/filters/client_channel/client_channel.h"
 #include "src/core/lib/channel/channel_args.h"
+#include "src/core/lib/iomgr/pollset.h"
 #include "src/core/lib/security/credentials/local/local_credentials.h"
 #include "src/core/lib/security/transport/security_handshaker.h"
 #include "src/core/tsi/local_transport_security.h"
@@ -68,7 +69,7 @@ static void local_server_destroy(grpc_security_connector* sc) {
 }
 
 static void local_channel_add_handshakers(
-    grpc_channel_security_connector* sc,
+    grpc_channel_security_connector* sc, grpc_pollset_set* interested_parties,
     grpc_handshake_manager* handshake_manager) {
   tsi_handshaker* handshaker = nullptr;
   GPR_ASSERT(local_tsi_handshaker_create(true /* is_client */, &handshaker) ==
@@ -78,7 +79,7 @@ static void local_channel_add_handshakers(
 }
 
 static void local_server_add_handshakers(
-    grpc_server_security_connector* sc,
+    grpc_server_security_connector* sc, grpc_pollset_set* interested_parties,
     grpc_handshake_manager* handshake_manager) {
   tsi_handshaker* handshaker = nullptr;
   GPR_ASSERT(local_tsi_handshaker_create(false /* is_client */, &handshaker) ==

src/core/lib/security/security_connector/security_connector.cc

@@ -120,17 +120,19 @@ const tsi_peer_property* tsi_peer_get_property_by_name(const tsi_peer* peer,
 
 void grpc_channel_security_connector_add_handshakers(
     grpc_channel_security_connector* connector,
+    grpc_pollset_set* interested_parties,
     grpc_handshake_manager* handshake_mgr) {
   if (connector != nullptr) {
-    connector->add_handshakers(connector, handshake_mgr);
+    connector->add_handshakers(connector, interested_parties, handshake_mgr);
   }
 }
 
 void grpc_server_security_connector_add_handshakers(
     grpc_server_security_connector* connector,
+    grpc_pollset_set* interested_parties,
     grpc_handshake_manager* handshake_mgr) {
   if (connector != nullptr) {
-    connector->add_handshakers(connector, handshake_mgr);
+    connector->add_handshakers(connector, interested_parties, handshake_mgr);
   }
 }
 
@@ -519,7 +521,7 @@ static void fake_channel_cancel_check_call_host(
 }
 
 static void fake_channel_add_handshakers(
-    grpc_channel_security_connector* sc,
+    grpc_channel_security_connector* sc, grpc_pollset_set* interested_parties,
     grpc_handshake_manager* handshake_mgr) {
   grpc_handshake_manager_add(
       handshake_mgr,
@@ -528,6 +530,7 @@ static void fake_channel_add_handshakers(
 }
 
 static void fake_server_add_handshakers(grpc_server_security_connector* sc,
+                                        grpc_pollset_set* interested_parties,
                                         grpc_handshake_manager* handshake_mgr) {
   grpc_handshake_manager_add(
       handshake_mgr,
@@ -669,6 +672,7 @@ static void ssl_server_destroy(grpc_security_connector* sc) {
 }
 
 static void ssl_channel_add_handshakers(grpc_channel_security_connector* sc,
+                                        grpc_pollset_set* interested_parties,
                                         grpc_handshake_manager* handshake_mgr) {
   grpc_ssl_channel_security_connector* c =
       reinterpret_cast<grpc_ssl_channel_security_connector*>(sc);
@@ -779,6 +783,7 @@ static bool try_fetch_ssl_server_credentials(
 }
 
 static void ssl_server_add_handshakers(grpc_server_security_connector* sc,
+                                       grpc_pollset_set* interested_parties,
                                        grpc_handshake_manager* handshake_mgr) {
   grpc_ssl_server_security_connector* c =
       reinterpret_cast<grpc_ssl_server_security_connector*>(sc);

src/core/lib/security/security_connector/security_connector.h

@@ -27,6 +27,7 @@
 
 #include "src/core/lib/channel/handshaker.h"
 #include "src/core/lib/iomgr/endpoint.h"
+#include "src/core/lib/iomgr/pollset.h"
 #include "src/core/lib/iomgr/tcp_server.h"
 #include "src/core/tsi/ssl_transport_security.h"
 #include "src/core/tsi/transport_security_interface.h"
@@ -125,6 +126,7 @@ struct grpc_channel_security_connector {
                                  grpc_closure* on_call_host_checked,
                                  grpc_error* error);
   void (*add_handshakers)(grpc_channel_security_connector* sc,
+                          grpc_pollset_set* interested_parties,
                           grpc_handshake_manager* handshake_mgr);
 };
 
@@ -151,6 +153,7 @@ void grpc_channel_security_connector_cancel_check_call_host(
 /* Registers handshakers with \a handshake_mgr. */
 void grpc_channel_security_connector_add_handshakers(
     grpc_channel_security_connector* connector,
+    grpc_pollset_set* interested_parties,
     grpc_handshake_manager* handshake_mgr);
 
 /* --- server_security_connector object. ---
@@ -164,6 +167,7 @@ struct grpc_server_security_connector {
   grpc_security_connector base;
   grpc_server_credentials* server_creds;
   void (*add_handshakers)(grpc_server_security_connector* sc,
+                          grpc_pollset_set* interested_parties,
                           grpc_handshake_manager* handshake_mgr);
 };
 
@@ -172,7 +176,8 @@ int grpc_server_security_connector_cmp(grpc_server_security_connector* sc1,
                                        grpc_server_security_connector* sc2);
 
 void grpc_server_security_connector_add_handshakers(
-    grpc_server_security_connector* sc, grpc_handshake_manager* handshake_mgr);
+    grpc_server_security_connector* sc, grpc_pollset_set* interested_parties,
+    grpc_handshake_manager* handshake_mgr);
 
 /* --- Creation security connectors. --- */
 

src/core/lib/security/transport/security_handshaker.cc

@@ -475,22 +475,24 @@ static grpc_handshaker* fail_handshaker_create() {
 
 static void client_handshaker_factory_add_handshakers(
     grpc_handshaker_factory* handshaker_factory, const grpc_channel_args* args,
+    grpc_pollset_set* interested_parties,
     grpc_handshake_manager* handshake_mgr) {
   grpc_channel_security_connector* security_connector =
       reinterpret_cast<grpc_channel_security_connector*>(
           grpc_security_connector_find_in_args(args));
-  grpc_channel_security_connector_add_handshakers(security_connector,
-                                                  handshake_mgr);
+  grpc_channel_security_connector_add_handshakers(
+      security_connector, interested_parties, handshake_mgr);
 }
 
 static void server_handshaker_factory_add_handshakers(
     grpc_handshaker_factory* hf, const grpc_channel_args* args,
+    grpc_pollset_set* interested_parties,
     grpc_handshake_manager* handshake_mgr) {
   grpc_server_security_connector* security_connector =
       reinterpret_cast<grpc_server_security_connector*>(
           grpc_security_connector_find_in_args(args));
-  grpc_server_security_connector_add_handshakers(security_connector,
-                                                 handshake_mgr);
+  grpc_server_security_connector_add_handshakers(
+      security_connector, interested_parties, handshake_mgr);
 }
 
 static void handshaker_factory_destroy(

src/core/tsi/alts/handshaker/alts_tsi_handshaker.cc

@@ -347,7 +347,8 @@ static void init_shared_resources(const char* handshaker_service_url) {
 
 tsi_result alts_tsi_handshaker_create(
     const grpc_alts_credentials_options* options, const char* target_name,
-    const char* handshaker_service_url, bool is_client, tsi_handshaker** self) {
+    const char* handshaker_service_url, bool is_client,
+    grpc_pollset_set* interested_parties, tsi_handshaker** self) {
   if (handshaker_service_url == nullptr || self == nullptr ||
       options == nullptr || (is_client && target_name == nullptr)) {
     gpr_log(GPR_ERROR, "Invalid arguments to alts_tsi_handshaker_create()");

src/core/tsi/alts/handshaker/alts_tsi_handshaker.h

@@ -23,6 +23,7 @@
 
 #include <grpc/grpc.h>
 
+#include "src/core/lib/iomgr/pollset_set.h"
 #include "src/core/lib/security/credentials/alts/grpc_alts_credentials_options.h"
 #include "src/core/tsi/alts_transport_security.h"
 #include "src/core/tsi/transport_security.h"
@@ -51,6 +52,7 @@ typedef struct alts_tsi_handshaker alts_tsi_handshaker;
  *   "host:port".
  * - is_client: boolean value indicating if the handshaker is used at the client
  *   (is_client = true) or server (is_client = false) side.
+ * - interested_parties: set of pollsets interested in this connection.
  * - self: address of ALTS TSI handshaker instance to be returned from the
  *   method.
  *
@@ -58,7 +60,8 @@ typedef struct alts_tsi_handshaker alts_tsi_handshaker;
  */
 tsi_result alts_tsi_handshaker_create(
     const grpc_alts_credentials_options* options, const char* target_name,
-    const char* handshaker_service_url, bool is_client, tsi_handshaker** self);
+    const char* handshaker_service_url, bool is_client,
+    grpc_pollset_set* interested_parties, tsi_handshaker** self);
 
 /**
  * This method handles handshaker response returned from ALTS handshaker

test/core/handshake/readahead_handshaker_server_ssl.cc

@@ -75,6 +75,7 @@ static grpc_handshaker* readahead_handshaker_create() {
 
 static void readahead_handshaker_factory_add_handshakers(
     grpc_handshaker_factory* hf, const grpc_channel_args* args,
+    grpc_pollset_set* interested_parties,
     grpc_handshake_manager* handshake_mgr) {
   grpc_handshake_manager_add(handshake_mgr, readahead_handshaker_create());
 }

test/core/security/ssl_server_fuzzer.cc

@@ -91,7 +91,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
     struct handshake_state state;
     state.done_callback_called = false;
     grpc_handshake_manager* handshake_mgr = grpc_handshake_manager_create();
-    grpc_server_security_connector_add_handshakers(sc, handshake_mgr);
+    grpc_server_security_connector_add_handshakers(sc, nullptr, handshake_mgr);
     grpc_handshake_manager_do_handshake(
         handshake_mgr, nullptr /* interested_parties */, mock_endpoint,
         nullptr /* channel_args */, deadline, nullptr /* acceptor */,

test/core/tsi/alts/handshaker/alts_tsi_handshaker_test.cc

@@ -421,7 +421,7 @@ static tsi_handshaker* create_test_handshaker(bool used_for_success_test,
       alts_mock_handshaker_client_create(used_for_success_test);
   grpc_alts_credentials_options* options =
       grpc_alts_credentials_client_options_create();
-  alts_tsi_handshaker_create(options, "target_name", "lame", is_client,
+  alts_tsi_handshaker_create(options, "target_name", "lame", is_client, nullptr,
                              &handshaker);
   alts_tsi_handshaker* alts_handshaker =
       reinterpret_cast<alts_tsi_handshaker*>(handshaker);