From 8b653133a759aa7f3a227cdf6449f07c7c3248f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 23 Aug 2018 23:22:27 +0300 Subject: [PATCH] Add shutdown detection The maxscale_is_shutting_down function is used to detect when MaxScale should stop. This fixes a race condition in the code where the workers has not yet been initialized but a termination signal has been received. It also replaces the misuse of the service_should_stop variable with a proper function. --- include/maxscale/maxscale.h | 10 ++++++++++ include/maxscale/service.h | 3 --- server/core/gateway.cc | 17 +++++++++++++++++ server/core/misc.cc | 12 ++++++++---- server/core/service.cc | 11 ++--------- .../modules/authenticator/MySQLAuth/dbusers.cc | 3 ++- server/modules/routing/avrorouter/avro_file.cc | 3 ++- server/modules/routing/avrorouter/avro_main.cc | 3 ++- 8 files changed, 43 insertions(+), 19 deletions(-) diff --git a/include/maxscale/maxscale.h b/include/maxscale/maxscale.h index b498f2e77..4134db0c1 100644 --- a/include/maxscale/maxscale.h +++ b/include/maxscale/maxscale.h @@ -43,4 +43,14 @@ time_t maxscale_started(void); */ int maxscale_uptime(void); +/** + * Is MaxScale shutting down + * + * This function can be used to detect whether the shutdown has been initiated. It does not tell + * whether the shutdown has been completed so thread-safety is still important. + * + * @return True if MaxScale is shutting down + */ +bool maxscale_is_shutting_down(); + MXS_END_DECLS diff --git a/include/maxscale/service.h b/include/maxscale/service.h index e7a011948..304c70a3b 100644 --- a/include/maxscale/service.h +++ b/include/maxscale/service.h @@ -146,9 +146,6 @@ typedef enum count_spec_t #define SERVICE_STATE_FAILED 3 /**< The service failed to start */ #define SERVICE_STATE_STOPPED 4 /**< The service has been stopped */ -// Set to 1 when services should stop -extern volatile sig_atomic_t service_should_stop; - /** * Find a service * diff --git a/server/core/gateway.cc b/server/core/gateway.cc index e156fd79d..be97d8b33 100644 --- a/server/core/gateway.cc +++ b/server/core/gateway.cc @@ -2037,6 +2037,13 @@ int main(int argc, char **argv) goto return_main; } + // Before we start the workers we need to check if a shutdown signal has been received + if (maxscale_is_shutting_down()) + { + rc = MAXSCALE_SHUTDOWN; + goto return_main; + } + if (!RoutingWorker::init()) { MXS_ERROR("Failed to initialize routing workers."); @@ -2044,6 +2051,16 @@ int main(int argc, char **argv) goto return_main; } + /** + * If a shutdown signal was received while we were initializing the workers, we need to exit. + * After this point, the shutdown will be driven by the workers. + */ + if (maxscale_is_shutting_down()) + { + rc = MAXSCALE_SHUTDOWN; + goto return_main; + } + if (!config_load(cnf_file_path)) { const char* fprerr = diff --git a/server/core/misc.cc b/server/core/misc.cc index 97c284de0..2037f2c07 100644 --- a/server/core/misc.cc +++ b/server/core/misc.cc @@ -36,15 +36,19 @@ int maxscale_uptime() return time(0) - started; } +static sig_atomic_t n_shutdowns = 0; + +bool maxscale_is_shutting_down() +{ + return n_shutdowns != 0; +} + int maxscale_shutdown() { - static int n_shutdowns = 0; - - int n = atomic_add(&n_shutdowns, 1); + int n = n_shutdowns++; if (n == 0) { - service_shutdown(); mxs::RoutingWorker::shutdown_all(); } diff --git a/server/core/service.cc b/server/core/service.cc index 064921d84..a51b78da6 100644 --- a/server/core/service.cc +++ b/server/core/service.cc @@ -481,7 +481,7 @@ int serviceStartAllPorts(Service* service) if (port) { - while (!service_should_stop && port) + while (!maxscale_is_shutting_down() && port) { listeners += serviceStartPort(service, port); port = port->next; @@ -635,7 +635,7 @@ int service_launch_all() error = true; } - if (service_should_stop) + if (maxscale_is_shutting_down()) { break; } @@ -1520,13 +1520,6 @@ const char* serviceGetWeightingParameter(SERVICE *svc) return service->weightby; } -volatile sig_atomic_t service_should_stop = 0; - -void service_shutdown() -{ - service_should_stop = 1; -} - void service_destroy_instances(void) { // The global list is modified by service_free so we need a copy of it diff --git a/server/modules/authenticator/MySQLAuth/dbusers.cc b/server/modules/authenticator/MySQLAuth/dbusers.cc index 18d75a575..92fb6bc12 100644 --- a/server/modules/authenticator/MySQLAuth/dbusers.cc +++ b/server/modules/authenticator/MySQLAuth/dbusers.cc @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -923,7 +924,7 @@ static int get_users(SERV_LISTENER *listener, bool skip_local) int total_users = -1; bool no_active_servers = true; - for (server = service->dbref; !service_should_stop && server; server = server->next) + for (server = service->dbref; !maxscale_is_shutting_down() && server; server = server->next) { if (!SERVER_REF_IS_ACTIVE(server) || !server_is_active(server->server) || (skip_local && server_is_mxs_service(server->server))) diff --git a/server/modules/routing/avrorouter/avro_file.cc b/server/modules/routing/avrorouter/avro_file.cc index 4fb2effba..cc0e35095 100644 --- a/server/modules/routing/avrorouter/avro_file.cc +++ b/server/modules/routing/avrorouter/avro_file.cc @@ -34,6 +34,7 @@ #include #include +#include #include #include @@ -493,7 +494,7 @@ avro_binlog_end_t avro_read_all_events(Avro *router) std::string next_binlog; bool rotate_seen = false; - while (!service_should_stop) + while (!maxscale_is_shutting_down()) { avro_binlog_end_t rc; REP_HEADER hdr; diff --git a/server/modules/routing/avrorouter/avro_main.cc b/server/modules/routing/avrorouter/avro_main.cc index 1c09af94d..658193ad6 100644 --- a/server/modules/routing/avrorouter/avro_main.cc +++ b/server/modules/routing/avrorouter/avro_main.cc @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -328,7 +329,7 @@ static bool conversion_task_ctl(Avro *inst, bool start) { bool rval = false; - if (!service_should_stop) + if (!maxscale_is_shutting_down()) { Worker* worker = static_cast(mxs_rworker_get(MXS_RWORKER_MAIN)); std::unique_ptr task(new (std::nothrow) ConversionCtlTask(inst, start));