From fb296824af9f5a0d41f7233620388737f7fdeb28 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 5 Oct 2018 09:49:29 +0200 Subject: [PATCH] Revert "Create and pass pollset_set to ALTS TSI handshaker" --- .../client_channel/http_connect_handshaker.cc | 1 - .../ext/transport/chttp2/client/chttp2_connector.cc | 2 +- .../ext/transport/chttp2/server/chttp2_server.cc | 8 -------- src/core/lib/channel/handshaker_factory.cc | 5 ++--- src/core/lib/channel/handshaker_factory.h | 2 -- src/core/lib/channel/handshaker_registry.cc | 8 ++------ src/core/lib/channel/handshaker_registry.h | 1 - src/core/lib/http/httpcli_security_connector.cc | 3 +-- .../security_connector/alts_security_connector.cc | 12 ++++++------ .../security_connector/security_connector.cc | 7 ------- .../security/security_connector/security_connector.h | 5 ----- .../lib/security/transport/security_handshaker.cc | 10 ---------- src/core/tsi/alts/handshaker/alts_tsi_handshaker.cc | 3 +-- src/core/tsi/alts/handshaker/alts_tsi_handshaker.h | 5 +---- .../handshake/readahead_handshaker_server_ssl.cc | 1 - .../tsi/alts/handshaker/alts_tsi_handshaker_test.cc | 2 +- 16 files changed, 15 insertions(+), 60 deletions(-) diff --git a/src/core/ext/filters/client_channel/http_connect_handshaker.cc b/src/core/ext/filters/client_channel/http_connect_handshaker.cc index bfabc68c661..7ce8da8c00e 100644 --- a/src/core/ext/filters/client_channel/http_connect_handshaker.cc +++ b/src/core/ext/filters/client_channel/http_connect_handshaker.cc @@ -351,7 +351,6 @@ 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()); diff --git a/src/core/ext/transport/chttp2/client/chttp2_connector.cc b/src/core/ext/transport/chttp2/client/chttp2_connector.cc index 5229304fa45..0ac84032fd4 100644 --- a/src/core/ext/transport/chttp2/client/chttp2_connector.cc +++ b/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->args.interested_parties, c->handshake_mgr); + 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, diff --git a/src/core/ext/transport/chttp2/server/chttp2_server.cc b/src/core/ext/transport/chttp2/server/chttp2_server.cc index b95baa91914..3f8a26ae32b 100644 --- a/src/core/ext/transport/chttp2/server/chttp2_server.cc +++ b/src/core/ext/transport/chttp2/server/chttp2_server.cc @@ -67,7 +67,6 @@ 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( @@ -77,9 +76,6 @@ 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); } } @@ -193,11 +189,7 @@ 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); diff --git a/src/core/lib/channel/handshaker_factory.cc b/src/core/lib/channel/handshaker_factory.cc index 8ade8fe4e23..4fd43635b6b 100644 --- a/src/core/lib/channel/handshaker_factory.cc +++ b/src/core/lib/channel/handshaker_factory.cc @@ -24,12 +24,11 @@ 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, interested_parties, handshake_mgr); + handshaker_factory->vtable->add_handshakers(handshaker_factory, args, + handshake_mgr); } } diff --git a/src/core/lib/channel/handshaker_factory.h b/src/core/lib/channel/handshaker_factory.h index e17a6781798..3e45fcf20e6 100644 --- a/src/core/lib/channel/handshaker_factory.h +++ b/src/core/lib/channel/handshaker_factory.h @@ -32,7 +32,6 @@ 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; @@ -43,7 +42,6 @@ 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( diff --git a/src/core/lib/channel/handshaker_registry.cc b/src/core/lib/channel/handshaker_registry.cc index fbafc43e795..eec3e1b352d 100644 --- a/src/core/lib/channel/handshaker_registry.cc +++ b/src/core/lib/channel/handshaker_registry.cc @@ -51,11 +51,9 @@ 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, - interested_parties, handshake_mgr); + grpc_handshaker_factory_add_handshakers(list->list[i], args, handshake_mgr); } } @@ -93,9 +91,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) { grpc_handshaker_factory_list_add_handshakers( - &g_handshaker_factory_lists[handshaker_type], args, interested_parties, - handshake_mgr); + &g_handshaker_factory_lists[handshaker_type], args, handshake_mgr); } diff --git a/src/core/lib/channel/handshaker_registry.h b/src/core/lib/channel/handshaker_registry.h index 3dd4316de67..82ad9c5b9af 100644 --- a/src/core/lib/channel/handshaker_registry.h +++ b/src/core/lib/channel/handshaker_registry.h @@ -43,7 +43,6 @@ 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 */ diff --git a/src/core/lib/http/httpcli_security_connector.cc b/src/core/lib/http/httpcli_security_connector.cc index 98fb7d39372..50078c37a17 100644 --- a/src/core/lib/http/httpcli_security_connector.cc +++ b/src/core/lib/http/httpcli_security_connector.cc @@ -189,8 +189,7 @@ 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, - nullptr /* interested_parties */, c->handshake_mgr); + grpc_handshakers_add(HANDSHAKER_CLIENT, &args, c->handshake_mgr); grpc_handshake_manager_do_handshake( c->handshake_mgr, nullptr /* interested_parties */, tcp, nullptr /* channel_args */, deadline, nullptr /* acceptor */, diff --git a/src/core/lib/security/security_connector/alts_security_connector.cc b/src/core/lib/security/security_connector/alts_security_connector.cc index f157c93171a..35a787871a8 100644 --- a/src/core/lib/security/security_connector/alts_security_connector.cc +++ b/src/core/lib/security/security_connector/alts_security_connector.cc @@ -70,9 +70,9 @@ static void alts_channel_add_handshakers( auto c = reinterpret_cast(sc); grpc_alts_credentials* creds = reinterpret_cast(c->base.channel_creds); - GPR_ASSERT(alts_tsi_handshaker_create( - creds->options, c->target_name, creds->handshaker_service_url, - true, sc->base.interested_parties, &handshaker) == TSI_OK); + GPR_ASSERT(alts_tsi_handshaker_create(creds->options, c->target_name, + creds->handshaker_service_url, true, + &handshaker) == TSI_OK); grpc_handshake_manager_add(handshake_manager, grpc_security_handshaker_create( handshaker, &sc->base)); } @@ -84,9 +84,9 @@ static void alts_server_add_handshakers( auto c = reinterpret_cast(sc); grpc_alts_server_credentials* creds = reinterpret_cast(c->base.server_creds); - GPR_ASSERT(alts_tsi_handshaker_create( - creds->options, nullptr, creds->handshaker_service_url, false, - sc->base.interested_parties, &handshaker) == TSI_OK); + GPR_ASSERT(alts_tsi_handshaker_create(creds->options, nullptr, + creds->handshaker_service_url, false, + &handshaker) == TSI_OK); grpc_handshake_manager_add(handshake_manager, grpc_security_handshaker_create( handshaker, &sc->base)); } diff --git a/src/core/lib/security/security_connector/security_connector.cc b/src/core/lib/security/security_connector/security_connector.cc index f4490582a3f..6246613e7b3 100644 --- a/src/core/lib/security/security_connector/security_connector.cc +++ b/src/core/lib/security/security_connector/security_connector.cc @@ -156,13 +156,6 @@ int grpc_security_connector_cmp(grpc_security_connector* sc, return sc->vtable->cmp(sc, other); } -void grpc_security_connector_set_interested_parties( - grpc_security_connector* sc, grpc_pollset_set* interested_parties) { - if (sc != nullptr) { - sc->interested_parties = interested_parties; - } -} - int grpc_channel_security_connector_cmp(grpc_channel_security_connector* sc1, grpc_channel_security_connector* sc2) { GPR_ASSERT(sc1->channel_creds != nullptr); diff --git a/src/core/lib/security/security_connector/security_connector.h b/src/core/lib/security/security_connector/security_connector.h index f0172594ad2..67a506b5767 100644 --- a/src/core/lib/security/security_connector/security_connector.h +++ b/src/core/lib/security/security_connector/security_connector.h @@ -63,7 +63,6 @@ struct grpc_security_connector { const grpc_security_connector_vtable* vtable; gpr_refcount refcount; const char* url_scheme; - grpc_pollset_set* interested_parties; }; /* Refcounting. */ @@ -107,10 +106,6 @@ grpc_security_connector* grpc_security_connector_from_arg(const grpc_arg* arg); grpc_security_connector* grpc_security_connector_find_in_args( const grpc_channel_args* args); -/* Util to set the interested_parties whose ownership is not transferred. */ -void grpc_security_connector_set_interested_parties( - grpc_security_connector* sc, grpc_pollset_set* interested_parties); - /* --- channel_security_connector object. --- A channel security connector object represents a way to configure the diff --git a/src/core/lib/security/transport/security_handshaker.cc b/src/core/lib/security/transport/security_handshaker.cc index f3bdf573b3d..d76d5826388 100644 --- a/src/core/lib/security/transport/security_handshaker.cc +++ b/src/core/lib/security/transport/security_handshaker.cc @@ -475,30 +475,20 @@ 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_security_connector_find_in_args(args)); - if (security_connector != nullptr) { - grpc_security_connector_set_interested_parties(&security_connector->base, - interested_parties); - } grpc_channel_security_connector_add_handshakers(security_connector, 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_security_connector_find_in_args(args)); - if (security_connector != nullptr) { - grpc_security_connector_set_interested_parties(&security_connector->base, - interested_parties); - } grpc_server_security_connector_add_handshakers(security_connector, handshake_mgr); } diff --git a/src/core/tsi/alts/handshaker/alts_tsi_handshaker.cc b/src/core/tsi/alts/handshaker/alts_tsi_handshaker.cc index dfdd659b877..34608a3de19 100644 --- a/src/core/tsi/alts/handshaker/alts_tsi_handshaker.cc +++ b/src/core/tsi/alts/handshaker/alts_tsi_handshaker.cc @@ -347,8 +347,7 @@ 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, - grpc_pollset_set* interested_parties, tsi_handshaker** self) { + const char* handshaker_service_url, bool is_client, 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()"); diff --git a/src/core/tsi/alts/handshaker/alts_tsi_handshaker.h b/src/core/tsi/alts/handshaker/alts_tsi_handshaker.h index 48ce69b1dac..227b30ce537 100644 --- a/src/core/tsi/alts/handshaker/alts_tsi_handshaker.h +++ b/src/core/tsi/alts/handshaker/alts_tsi_handshaker.h @@ -23,7 +23,6 @@ #include -#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" @@ -52,7 +51,6 @@ 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. * @@ -60,8 +58,7 @@ 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, - grpc_pollset_set* interested_parties, tsi_handshaker** self); + const char* handshaker_service_url, bool is_client, tsi_handshaker** self); /** * This method handles handshaker response returned from ALTS handshaker diff --git a/test/core/handshake/readahead_handshaker_server_ssl.cc b/test/core/handshake/readahead_handshaker_server_ssl.cc index 14d96b5d89c..97e9c20ee4f 100644 --- a/test/core/handshake/readahead_handshaker_server_ssl.cc +++ b/test/core/handshake/readahead_handshaker_server_ssl.cc @@ -75,7 +75,6 @@ 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()); } diff --git a/test/core/tsi/alts/handshaker/alts_tsi_handshaker_test.cc b/test/core/tsi/alts/handshaker/alts_tsi_handshaker_test.cc index e9eb7e175f4..85a58114ba6 100644 --- a/test/core/tsi/alts/handshaker/alts_tsi_handshaker_test.cc +++ b/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, nullptr, + alts_tsi_handshaker_create(options, "target_name", "lame", is_client, &handshaker); alts_tsi_handshaker* alts_handshaker = reinterpret_cast(handshaker);