From bb295b5cbea8c983839bbe441a03bfd53fee55bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 29 Nov 2018 16:02:33 +0200 Subject: [PATCH] MXS-2196: Remove listeners from services All access to the listeners of a service are done via the listener functions. --- include/maxscale/service.hh | 11 +- server/core/config.cc | 32 ++--- server/core/config_runtime.cc | 16 +-- server/core/internal/service.hh | 22 +--- server/core/service.cc | 123 ++++-------------- server/core/test/test_service.cc | 16 +-- server/modules/filter/tee/teesession.cc | 5 +- .../modules/protocol/MySQL/mariadb_client.cc | 2 +- server/modules/routing/binlogrouter/blr.cc | 5 - .../routing/binlogrouter/blr_master.cc | 4 - 10 files changed, 67 insertions(+), 169 deletions(-) diff --git a/include/maxscale/service.hh b/include/maxscale/service.hh index 788565a66..0d1c7e3ff 100644 --- a/include/maxscale/service.hh +++ b/include/maxscale/service.hh @@ -92,13 +92,10 @@ typedef struct server_ref_t class SERVICE { public: - const char* name; /**< The service name */ - int state; /**< The service state */ - int client_count; /**< Number of connected clients */ - int max_connections; /**< Maximum client connections */ - SERV_LISTENER* ports; /**< Linked list of ports and - * protocols - * that this service will listen on */ + const char* name; /**< The service name */ + int state; /**< The service state */ + int client_count; /**< Number of connected clients */ + int max_connections; /**< Maximum client connections */ const char* routerModule; /**< Name of router module to use */ struct mxs_router_object* router; /**< The router we are using */ struct mxs_router* router_instance; /**< The router instance for this diff --git a/server/core/config.cc b/server/core/config.cc index fa858b3d4..d92f9ea37 100644 --- a/server/core/config.cc +++ b/server/core/config.cc @@ -3881,25 +3881,25 @@ int create_new_listener(CONFIG_CONTEXT* obj) if (socket) { - serviceCreateListener(service, - obj->object, - protocol, - socket, - 0, - authenticator, - authenticator_options, - ssl_info); + listener_alloc(service, + obj->object, + protocol, + socket, + 0, + authenticator, + authenticator_options, + ssl_info); } else if (port) { - serviceCreateListener(service, - obj->object, - protocol, - address, - atoi(port), - authenticator, - authenticator_options, - ssl_info); + listener_alloc(service, + obj->object, + protocol, + address, + atoi(port), + authenticator, + authenticator_options, + ssl_info); } } diff --git a/server/core/config_runtime.cc b/server/core/config_runtime.cc index d3271be76..39485d266 100644 --- a/server/core/config_runtime.cc +++ b/server/core/config_runtime.cc @@ -1015,14 +1015,14 @@ bool runtime_create_listener(Service* service, else { const char* print_addr = addr ? addr : "::"; - SERV_LISTENER* listener = serviceCreateListener(service, - name, - proto, - addr, - u_port, - auth, - auth_opt, - ssl); + SListener listener = listener_alloc(service, + name, + proto, + addr, + u_port, + auth, + auth_opt, + ssl); if (listener && listener_serialize(listener)) { diff --git a/server/core/internal/service.hh b/server/core/internal/service.hh index 31912c189..cf1c0cfc5 100644 --- a/server/core/internal/service.hh +++ b/server/core/internal/service.hh @@ -208,18 +208,6 @@ int service_launch_all(void); */ bool service_thread_init(); -/** - * Creating and adding new components to services - */ -SERV_LISTENER* serviceCreateListener(Service* service, - const char* name, - const char* protocol, - const char* address, - unsigned short port, - const char* authenticator, - const char* options, - SSL_LISTENER* ssl); - /** * @brief Remove a listener from use * @@ -328,7 +316,7 @@ bool serviceHasBackend(Service* service, SERVER* server); * @param port Listener to start * @return True if listener was started */ -bool serviceLaunchListener(Service* service, SERV_LISTENER* port); +bool serviceLaunchListener(Service* service, const SListener& port); /** * @brief Find listener with specified properties. @@ -343,10 +331,10 @@ bool serviceLaunchListener(Service* service, SERV_LISTENER* port); * * @return True if service has the listener */ -SERV_LISTENER* service_find_listener(Service* service, - const char* socket, - const char* address, - unsigned short port); +SListener service_find_listener(Service* service, + const char* socket, + const char* address, + unsigned short port); /** * @brief Check if a service has a listener diff --git a/server/core/service.cc b/server/core/service.cc index 0af3a314f..911cbb0b0 100644 --- a/server/core/service.cc +++ b/server/core/service.cc @@ -181,7 +181,6 @@ Service::Service(const std::string& service_name, stats.n_sessions = 0; state = SERVICE_STATE_ALLOC; active = true; - ports = NULL; dbref = NULL; n_dbref = 0; snprintf(user, sizeof(user), "%s", m_user.c_str()); @@ -206,7 +205,7 @@ Service::Service(const std::string& service_name, } else { - retain_last_statements = -1; // Indicates that it has not been set. + retain_last_statements = -1; // Indicates that it has not been set. } /** @@ -237,11 +236,10 @@ Service::~Service() { mxs_rworker_delete_data(m_wkey); - while (auto tmp = ports) + for (const auto& tmp : listener_find_by_service(this)) { - ports = ports->next; mxb_assert(!tmp->active || maxscale_teardown_in_progress()); - listener_free(tmp); + listener_free(listener_find(tmp->name)); } if (router && router_instance && router->destroyInstance) @@ -321,23 +319,6 @@ bool service_isvalid(Service* service) service) != this_unit.services.end(); } -static inline void close_port(SERV_LISTENER* port) -{ - if (port->service->state == SERVICE_STATE_ALLOC) - { - /** The service failed when it was being allocated */ - port->service->state = SERVICE_STATE_FAILED; - } - - if (port->listener) - { - dcb_close(port->listener); - port->listener = NULL; - } - - listener_set_active(port, false); -} - /** * Start an individual port/protocol pair * @@ -345,7 +326,7 @@ static inline void close_port(SERV_LISTENER* port) * @param port The port to start * @return The number of listeners started */ -static int serviceStartPort(Service* service, SERV_LISTENER* port) +static int serviceStartPort(Service* service, const SListener& port) { const size_t ANY_IPV4_ADDRESS_LEN = 7; // strlen("0:0:0:0"); @@ -359,17 +340,15 @@ static int serviceStartPort(Service* service, SERV_LISTENER* port) { /* Should never happen, this guarantees it can't */ MXS_ERROR("Attempt to start port with null or incomplete service"); - close_port(port); mxb_assert(false); return 0; } - port->listener = dcb_alloc(DCB_ROLE_SERVICE_LISTENER, port); + port->listener = dcb_alloc(DCB_ROLE_SERVICE_LISTENER, port.get()); if (port->listener == NULL) { MXS_ERROR("Failed to create listener for service %s.", service->name); - close_port(port); return 0; } @@ -380,7 +359,6 @@ static int serviceStartPort(Service* service, SERV_LISTENER* port) MXS_ERROR("Unable to load protocol module %s. Listener for service %s not started.", port->protocol.c_str(), service->name); - close_port(port); return 0; } @@ -404,7 +382,6 @@ static int serviceStartPort(Service* service, SERV_LISTENER* port) MXS_ERROR("Failed to load authenticator module '%s' for listener '%s'", authenticator_name, port->name.c_str()); - close_port(port); return 0; } @@ -433,14 +410,13 @@ static int serviceStartPort(Service* service, SERV_LISTENER* port) /** Load the authentication users before before starting the listener */ if (port->listener->authfunc.loadusers) { - switch (port->listener->authfunc.loadusers(port)) + switch (port->listener->authfunc.loadusers(port.get())) { case MXS_AUTH_LOADUSERS_FATAL: MXS_ERROR("[%s] Fatal error when loading users for listener '%s', " "service is not started.", service->name, port->name.c_str()); - close_port(port); return 0; case MXS_AUTH_LOADUSERS_ERROR: @@ -467,13 +443,11 @@ static int serviceStartPort(Service* service, SERV_LISTENER* port) else { MXS_ERROR("[%s] Failed to create listener session.", service->name); - close_port(port); } } else { MXS_ERROR("[%s] Failed to listen on %s", service->name, config_bind); - close_port(port); } return listeners; @@ -489,15 +463,19 @@ static int serviceStartPort(Service* service, SERV_LISTENER* port) */ int serviceStartAllPorts(Service* service) { - SERV_LISTENER* port = service->ports; int listeners = 0; + auto my_listeners = listener_find_by_service(service); - if (port) + if (!my_listeners.empty()) { - while (!maxscale_is_shutting_down() && port) + for (const auto& listener : my_listeners) { - listeners += serviceStartPort(service, port); - port = port->next; + if (maxscale_is_shutting_down()) + { + break; + } + + listeners += serviceStartPort(service, listener); } if (service->state == SERVICE_STATE_FAILED) @@ -569,7 +547,7 @@ int serviceInitialize(Service* service) return listeners; } -bool serviceLaunchListener(Service* service, SERV_LISTENER* port) +bool serviceLaunchListener(Service* service, const SListener& port) { mxb_assert(service->state != SERVICE_STATE_FAILED); bool rval = true; @@ -674,25 +652,6 @@ bool serviceStart(SERVICE* service) return listeners > 0; } -/** - * Add a listener to a service - * - * @param service Service where listener is added - * @param proto Listener to add - */ -static void service_add_listener(SERVICE* service, SERV_LISTENER* proto) -{ - do - { - /** Read the current value of the list's head. This will be our expected - * value for the following compare-and-swap operation. */ - proto->next = (SERV_LISTENER*)atomic_load_ptr((void**)&service->ports); - } - /** Compare the current value to our expected value and if they match, replace - * the current value with our new value. */ - while (!atomic_cas_ptr((void**)&service->ports, (void**)&proto->next, proto)); -} - bool service_remove_listener(Service* service, const char* target) { bool rval = false; @@ -707,48 +666,10 @@ bool service_remove_listener(Service* service, const char* target) return rval; } -/** - * Create a listener for the service - * - * @param service The service - * @param protocol The name of the protocol module - * @param address The address to listen with - * @param port The port to listen on - * @param authenticator Name of the authenticator to be used - * @param ssl SSL configuration - * - * @return Created listener or NULL on error - */ -SERV_LISTENER* serviceCreateListener(Service* service, - const char* name, - const char* protocol, - const char* address, - unsigned short port, - const char* authenticator, - const char* options, - SSL_LISTENER* ssl) -{ - SERV_LISTENER* proto = listener_alloc(service, - name, - protocol, - address, - port, - authenticator, - options, - ssl); - - if (proto) - { - service_add_listener(service, proto); - } - - return proto; -} - -SERV_LISTENER* service_find_listener(Service* service, - const char* socket, - const char* address, - unsigned short port) +SListener service_find_listener(Service* service, + const char* socket, + const char* address, + unsigned short port) { for (const auto& listener : listener_find_by_service(service)) { @@ -1893,7 +1814,7 @@ static json_t* service_all_listeners_json_data(const SERVICE* service) for (const auto& listener : listener_find_by_service(service)) { - json_array_append_new(arr, listener_to_json(listener.get())); + json_array_append_new(arr, listener_to_json(listener)); } return arr; @@ -1905,7 +1826,7 @@ static json_t* service_listener_json_data(const SERVICE* service, const char* na if (listener && listener->service == service) { - return listener_to_json(listener.get()); + return listener_to_json(listener); } return NULL; diff --git a/server/core/test/test_service.cc b/server/core/test/test_service.cc index 886b9a9a3..e71cdd00e 100644 --- a/server/core/test/test_service.cc +++ b/server/core/test/test_service.cc @@ -74,14 +74,14 @@ static int test1() mxb_assert_message(0 != service_isvalid(service), "Service must be valid after creation"); mxb_assert_message(0 == strcmp("MyService", service->name), "Service must have given name"); fprintf(stderr, "\t..done\nAdding protocol testprotocol."); - mxb_assert_message(serviceCreateListener(service, - "TestProtocol", - "mariadbclient", - "localhost", - 9876, - "MySQLAuth", - NULL, - NULL), + mxb_assert_message(listener_alloc(service, + "TestProtocol", + "mariadbclient", + "localhost", + 9876, + "MySQLAuth", + NULL, + NULL), "Add Protocol should succeed"); mxb_assert_message(0 != serviceHasListener(service, "TestProtocol", "mariadbclient", "localhost", 9876), "Service should have new protocol as requested"); diff --git a/server/modules/filter/tee/teesession.cc b/server/modules/filter/tee/teesession.cc index 6cf6afae9..86b4b3a02 100644 --- a/server/modules/filter/tee/teesession.cc +++ b/server/modules/filter/tee/teesession.cc @@ -60,9 +60,10 @@ TeeSession* TeeSession::create(Tee* my_instance, MXS_SESSION* session) (MySQLProtocol*)session->client_dcb->protocol, my_instance->get_service())) == NULL) { + const char* extra = !listener_find_by_service(my_instance->get_service()).empty() ? "" : + ": Service has no network listeners"; MXS_ERROR("Failed to create local client connection to '%s'%s", - my_instance->get_service()->name, - my_instance->get_service()->ports ? "" : ": Service has no network listeners"); + my_instance->get_service()->name, extra); return NULL; } } diff --git a/server/modules/protocol/MySQL/mariadb_client.cc b/server/modules/protocol/MySQL/mariadb_client.cc index 7fc4fb815..5f46832a9 100644 --- a/server/modules/protocol/MySQL/mariadb_client.cc +++ b/server/modules/protocol/MySQL/mariadb_client.cc @@ -272,7 +272,7 @@ LocalClient* LocalClient::create(MYSQL_session* session, MySQLProtocol* proto, S if (listener->port > 0) { /** Pick the first network listener */ - rval = create(session, proto, "127.0.0.1", service->ports->port); + rval = create(session, proto, "127.0.0.1", listener->port); break; } } diff --git a/server/modules/routing/binlogrouter/blr.cc b/server/modules/routing/binlogrouter/blr.cc index 849b6beee..9b963a74b 100644 --- a/server/modules/routing/binlogrouter/blr.cc +++ b/server/modules/routing/binlogrouter/blr.cc @@ -1131,11 +1131,6 @@ static MXS_ROUTER* createInstance(SERVICE* service, MXS_CONFIG_PARAMETER* params */ static void free_instance(ROUTER_INSTANCE* instance) { - for (SERV_LISTENER* port = instance->service->ports; port; port = port->next) - { - users_free(port->users); - } - MXS_FREE(instance->uuid); MXS_FREE(instance->user); MXS_FREE(instance->password); diff --git a/server/modules/routing/binlogrouter/blr_master.cc b/server/modules/routing/binlogrouter/blr_master.cc index 2b6da77e8..67942ad1a 100644 --- a/server/modules/routing/binlogrouter/blr_master.cc +++ b/server/modules/routing/binlogrouter/blr_master.cc @@ -689,10 +689,6 @@ static GWBUF* blr_make_registration(ROUTER_INSTANCE* router) // Set empty password *data++ = 0; // Slave password length // Add port - if (router->service->ports) - { - port = router->service->ports->port; - } encode_value(&data[0], port, 16); // Slave master port, 2 bytes encode_value(&data[2], 0, 32); // Replication rank, 4 bytes encode_value(&data[6], router->masterid, 32); // Master server-id, 4 bytes