From b7294a28afab44c2294bf70baa296bdb81b847c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Tue, 18 Jun 2019 07:23:16 +0300 Subject: [PATCH 1/3] MXS-2547: Assert that workers aren't stopped If a worker is stopped, none of the Worker::execute or post_message methods should be called. --- server/core/worker.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/core/worker.cc b/server/core/worker.cc index 728cbca72..6d6990caa 100644 --- a/server/core/worker.cc +++ b/server/core/worker.cc @@ -718,6 +718,8 @@ size_t Worker::execute_concurrently(Task& task) bool Worker::post_message(uint32_t msg_id, intptr_t arg1, intptr_t arg2) { + ss_dassert(state() != Worker::STOPPED); + // NOTE: No logging here, this function must be signal safe. MessageQueue::Message message(msg_id, arg1, arg2); From 69b3879357c35cd21e4952cf109fee0e1ae8664a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Tue, 18 Jun 2019 11:00:58 +0300 Subject: [PATCH 2/3] MXS-2547: Don't post messages to stopped workers If a worker has been stopped, tasks must not be executed on it. To prevent this, the calling code should check whether the worker has been stopped. This does not prevent the case where a message is successfully posted to a worker but the worker is stopped before it processes it. --- server/core/worker.cc | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/server/core/worker.cc b/server/core/worker.cc index 6d6990caa..b92ef49c4 100644 --- a/server/core/worker.cc +++ b/server/core/worker.cc @@ -718,12 +718,17 @@ size_t Worker::execute_concurrently(Task& task) bool Worker::post_message(uint32_t msg_id, intptr_t arg1, intptr_t arg2) { + // NOTE: No logging here, this function must be signal safe. + bool rval = false; ss_dassert(state() != Worker::STOPPED); - // NOTE: No logging here, this function must be signal safe. - MessageQueue::Message message(msg_id, arg1, arg2); + if (state() != Worker::STOPPED) + { + MessageQueue::Message message(msg_id, arg1, arg2); + rval = m_pQueue->post(message); + } - return m_pQueue->post(message); + return rval; } bool mxs_worker_post_message(MXS_WORKER* pWorker, uint32_t msg_id, intptr_t arg1, intptr_t arg2) From 27fb397041c340eb757bd52086271e17bc99f83f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Tue, 18 Jun 2019 11:23:14 +0300 Subject: [PATCH 3/3] MXS-2547: Do shutdown on main worker By stopping the REST API before the workers and moving the shutdown to the same worker that handles REST API requests, we prevent the hang on shutdown. This also makes the signal handler signal-safe. --- server/core/admin.cc | 1 + server/core/gateway.cc | 22 +++++++++++++++------- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/server/core/admin.cc b/server/core/admin.cc index 266226b5c..58396473a 100644 --- a/server/core/admin.cc +++ b/server/core/admin.cc @@ -410,6 +410,7 @@ bool mxs_admin_init() void mxs_admin_shutdown() { MHD_stop_daemon(http_daemon); + MXS_NOTICE("Stopped MaxScale REST API"); } bool mxs_admin_https_enabled() diff --git a/server/core/gateway.cc b/server/core/gateway.cc index 91da2c46d..aaf22e581 100644 --- a/server/core/gateway.cc +++ b/server/core/gateway.cc @@ -2293,9 +2293,6 @@ int main(int argc, char **argv) /*< Stop all the monitors */ monitorStopAll(); - /** Stop administrative interface */ - mxs_admin_shutdown(); - /*< Stop all the monitors */ monitorStopAll(); @@ -2380,6 +2377,19 @@ return_main: return rc; } /*< End of main */ +class ShutdownTask: public Worker::DisposableTask +{ +public: + void execute(mxs::Worker& w) + { + mxs_admin_shutdown(); + service_shutdown(); + Worker::shutdown_all(); + hkshutdown(); + log_flush_shutdown(); + } +}; + /*< * Shutdown MaxScale server */ @@ -2391,10 +2401,8 @@ int maxscale_shutdown() if (n == 0) { - service_shutdown(); - Worker::shutdown_all(); - hkshutdown(); - log_flush_shutdown(); + mxs::Worker* worker = Worker::get(0); + worker->post(std::auto_ptr(new ShutdownTask), mxs::Worker::EXECUTE_QUEUED); } return n + 1;