From 71a3cde44144d0b10f4a52b8971d63a37507e7fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 7 Mar 2019 20:28:09 +0200 Subject: [PATCH 1/4] MXS-2373: Fix filter serialization The module of a filter was ignored as it wasn't in the list of expected module parameters. --- server/core/filter.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/core/filter.cc b/server/core/filter.cc index 77718419d..14c04884c 100644 --- a/server/core/filter.cc +++ b/server/core/filter.cc @@ -540,12 +540,13 @@ static bool create_filter_config(const SFilterDef& filter, const char* filename) dprintf(file, "[%s]\n", filter->name.c_str()); dprintf(file, "%s=%s\n", CN_TYPE, CN_FILTER); + dprintf(file, "%s=%s\n", CN_MODULE, filter->module.c_str()); const MXS_MODULE* mod = get_module(filter->module.c_str(), NULL); mxb_assert(mod); MXS_MODULE_PARAM no_common_params = {}; - dump_param_list(file, filter->parameters, {CN_TYPE}, &no_common_params, mod->parameters); + dump_param_list(file, filter->parameters, {CN_TYPE, CN_MODULE}, &no_common_params, mod->parameters); close(file); return true; From 7db87784accbaf0e5c17042232165f05b6c55878 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Fri, 8 Mar 2019 07:45:59 +0200 Subject: [PATCH 2/4] Deliver hangups only to valid DCBs If a DCB was closed and a hangup event was sent to it via dcb_hangup_foreach shortly after it was closed, the DCB would still receive it even if it was closed. To prevent this, events must only be delivered to DCBs if they haven't been closed. --- server/core/backend.cc | 4 ++-- server/core/dcb.cc | 3 +-- server/modules/protocol/MySQL/mariadbbackend/mysql_backend.cc | 1 + 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/core/backend.cc b/server/core/backend.cc index d54fdeb9e..348ac5329 100644 --- a/server/core/backend.cc +++ b/server/core/backend.cc @@ -47,7 +47,7 @@ Backend::~Backend() void Backend::close(close_type type) { - mxb_assert(m_dcb->n_close == 0); + mxb_assert(m_dcb && m_dcb->n_close == 0); if (!m_closed) { @@ -180,7 +180,7 @@ void Backend::set_state(backend_state state) bool Backend::connect(MXS_SESSION* session, SessionCommandList* sescmd) { - mxb_assert(!in_use()); + mxb_assert(!in_use() && m_dcb == nullptr); bool rval = false; if ((m_dcb = dcb_connect(m_backend->server, session, m_backend->server->protocol))) diff --git a/server/core/dcb.cc b/server/core/dcb.cc index eb07a7ad7..52d9b248b 100644 --- a/server/core/dcb.cc +++ b/server/core/dcb.cc @@ -2041,8 +2041,7 @@ static void dcb_hangup_foreach_worker(MXB_WORKER* worker, struct server* server) for (DCB* dcb = this_unit.all_dcbs[id]; dcb; dcb = dcb->thread.next) { - if (dcb->state == DCB_STATE_POLLING && dcb->server - && dcb->server == server) + if (dcb->state == DCB_STATE_POLLING && dcb->server && dcb->server == server && dcb->n_close == 0) { dcb->flags |= DCBF_HUNG; dcb->func.hangup(dcb); diff --git a/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.cc b/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.cc index 2236f7da8..9de1843c1 100644 --- a/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.cc +++ b/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.cc @@ -1354,6 +1354,7 @@ static int gw_error_backend_event(DCB* dcb) */ static int gw_backend_hangup(DCB* dcb) { + mxb_assert(dcb->n_close == 0); MXS_SESSION* session = dcb->session; if (dcb->persistentstart) From 5005facfb150e3e17ca42864f28addf5a6e8d0a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Fri, 8 Mar 2019 07:48:34 +0200 Subject: [PATCH 3/4] Always dump the full configuration Always storing runtime configuration changes prevents problems when the change causes another parameter to change. One example of this is transaction_replay that implicitly enables other parameters. --- server/core/config.cc | 36 ++++-------------------------------- 1 file changed, 4 insertions(+), 32 deletions(-) diff --git a/server/core/config.cc b/server/core/config.cc index d3d395a08..adbf12f1a 100644 --- a/server/core/config.cc +++ b/server/core/config.cc @@ -4954,36 +4954,6 @@ MXS_CONFIG_PARAMETER* ParamList::params() } } -void dump_if_changed(const MXS_MODULE_PARAM* params, - int file, - const std::string& key, - const std::string& value) -{ - for (int i = 0; params[i].name; i++) - { - if (params[i].name == key) - { - /** - * This detects only exact matches, not ones that are logically equivalent - * but lexicographically different e.g. 1 and true. This might not - * be a bad thing: it'll distinct user defined values from defaults. - */ - - if (!params[i].default_value || value != params[i].default_value) - { - if (dprintf(file, "%s=%s\n", key.c_str(), value.c_str()) == -1) - { - MXS_ERROR("Failed to serialize service value: %d, %s", - errno, - mxs_strerror(errno)); - } - } - - break; - } - } -} - void dump_param_list(int file, MXS_CONFIG_PARAMETER* list, const std::unordered_set& ignored, @@ -4994,8 +4964,10 @@ void dump_param_list(int file, { if (ignored.count(p->name) == 0 && *p->value) { - dump_if_changed(common_params, file, p->name, p->value); - dump_if_changed(module_params, file, p->name, p->value); + if (dprintf(file, "%s=%s\n", p->name, p->value) == -1) + { + MXS_ERROR("Failed to serialize service value: %d, %s", errno, mxs_strerror(errno)); + } } } } From 247e558ffa37acbcbe8ac1e0c7067d330bd36058 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Fri, 8 Mar 2019 09:51:49 +0200 Subject: [PATCH 4/4] Fix tls-model exposure to other modules The flag was propagated to other modules that depend on it. --- server/core/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/CMakeLists.txt b/server/core/CMakeLists.txt index 25c221525..9a4e6f2f7 100644 --- a/server/core/CMakeLists.txt +++ b/server/core/CMakeLists.txt @@ -83,7 +83,7 @@ endif() # Using initial-exec instead of the default global-dynamic tls-model # reduces the cost of using thread-local variables in dynamic libraries. -target_compile_options(maxscale-common PUBLIC "-ftls-model=initial-exec") +target_compile_options(maxscale-common PRIVATE "-ftls-model=initial-exec") add_dependencies(maxscale-common pcre2 connector-c libmicrohttpd jansson maxbase) set_target_properties(maxscale-common PROPERTIES VERSION "1.0.0")