From 3ee5d9a8ea519b80f34a64d0e1de7b1df1bcaa22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Wed, 26 Sep 2018 16:05:39 +0300 Subject: [PATCH] MXS-2067: Move server lock into Server class The lock for the server is now only visible to the MaxScale core. Changing the type to std::mutex also allows the use of RAII lock guards. --- include/maxscale/server.h | 11 +++++----- server/core/internal/server.hh | 7 ++++++ server/core/server.cc | 40 +++++++++++++++++----------------- 3 files changed, 32 insertions(+), 26 deletions(-) diff --git a/include/maxscale/server.h b/include/maxscale/server.h index c1a209b3d..074b206d6 100644 --- a/include/maxscale/server.h +++ b/include/maxscale/server.h @@ -134,12 +134,11 @@ typedef struct server SERVER_PARAM* parameters; /**< Additional custom parameters which may affect routing * decisions. */ // Base variables - SPINLOCK lock; /**< Access lock. Required when modifying server status or settings. */ - bool is_active; /**< Server is active and has not been "destroyed" */ - void* auth_instance;/**< Authenticator instance data */ - SSL_LISTENER* server_ssl; /**< SSL data */ - DCB** persistent; /**< List of unused persistent connections to the server */ - uint8_t charset; /**< Server character set. Read from backend and sent to client. */ + bool is_active; /**< Server is active and has not been "destroyed" */ + void* auth_instance; /**< Authenticator instance data */ + SSL_LISTENER* server_ssl; /**< SSL data */ + DCB** persistent; /**< List of unused persistent connections to the server */ + uint8_t charset; /**< Server character set. Read from backend and sent to client. */ // Statistics and events SERVER_STATS stats; /**< The server statistics, e.g. number of connections */ int persistmax; /**< Maximum pool size actually achieved since startup */ diff --git a/server/core/internal/server.hh b/server/core/internal/server.hh index 9ab603af4..2d57028c1 100644 --- a/server/core/internal/server.hh +++ b/server/core/internal/server.hh @@ -16,6 +16,10 @@ * Internal header for the server type */ +#include + +#include + #include #include #include @@ -48,6 +52,9 @@ public: response_time->add(ave, num_samples); } + // TODO: Do all access to Server via methods + mutable std::mutex lock; + private: // nantti, TODO. Decide whether to expose some of this in config, or if the values // can be calculated at runtime. The "500" or sample_max affects how often a diff --git a/server/core/server.cc b/server/core/server.cc index 02d7fdc91..d91732c22 100644 --- a/server/core/server.cc +++ b/server/core/server.cc @@ -37,7 +37,6 @@ #include #include #include -#include #include #include #include @@ -163,7 +162,6 @@ SERVER* server_alloc(const char* name, MXS_CONFIG_PARAMETER* params) server->persistmaxtime = config_get_integer(params, CN_PERSISTMAXTIME); server->proxy_protocol = config_get_bool(params, CN_PROXY_PROTOCOL); server->parameters = NULL; - spinlock_init(&server->lock); server->is_active = true; server->auth_instance = auth_instance; server->server_ssl = ssl; @@ -875,10 +873,11 @@ static SERVER_PARAM* allocate_parameter(const char* name, const char* value) return param; } -bool server_remove_parameter(SERVER* server, const char* name) +bool server_remove_parameter(SERVER* srv, const char* name) { + Server* server = static_cast(srv); bool rval = false; - spinlock_acquire(&server->lock); + std::lock_guard guard(server->lock); for (SERVER_PARAM* p = server->parameters; p; p = p->next) { @@ -890,17 +889,17 @@ bool server_remove_parameter(SERVER* server, const char* name) } } - spinlock_release(&server->lock); return rval; } -void server_set_parameter(SERVER* server, const char* name, const char* value) +void server_set_parameter(SERVER* srv, const char* name, const char* value) { + Server* server = static_cast(srv); SERVER_PARAM* param = allocate_parameter(name, value); if (param) { - spinlock_acquire(&server->lock); + std::lock_guard guard(server->lock); // Insert new value param->next = server->parameters; @@ -915,8 +914,6 @@ void server_set_parameter(SERVER* server, const char* name, const char* value) break; } } - - spinlock_release(&server->lock); } } @@ -971,12 +968,11 @@ static size_t server_get_parameter_nolock(const SERVER* server, const char* name * * @return Length of the parameter value or 0 if parameter was not found */ -size_t server_get_parameter(const SERVER* server, const char* name, char* out, size_t size) +size_t server_get_parameter(const SERVER* srv, const char* name, char* out, size_t size) { - spinlock_acquire(&server->lock); - size_t len = server_get_parameter_nolock(server, name, out, size); - spinlock_release(&server->lock); - return len; + const Server* server = static_cast(srv); + std::lock_guard guard(server->lock); + return server_get_parameter_nolock(server, name, out, size); } /** @@ -1275,14 +1271,16 @@ bool server_serialize(const SERVER* server) * @param server The server to update * @param bit The bit to set for the server */ -bool mxs::server_set_status(SERVER* server, int bit, string* errmsg_out) +bool mxs::server_set_status(SERVER* srv, int bit, string* errmsg_out) { + Server* server = static_cast(srv); bool written = false; /* First check if the server is monitored. This isn't done under a lock * but the race condition cannot cause significant harm. Monitors are never * freed so the pointer stays valid. */ MXS_MONITOR* mon = monitor_server_in_use(server); - spinlock_acquire(&server->lock); + std::lock_guard guard(server->lock); + if (mon && mon->state == MONITOR_STATE_RUNNING) { /* This server is monitored, in which case modifying any other status bit than Maintenance is @@ -1314,7 +1312,7 @@ bool mxs::server_set_status(SERVER* server, int bit, string* errmsg_out) server_set_status_nolock(server, bit); written = true; } - spinlock_release(&server->lock); + return written; } /** @@ -1325,11 +1323,13 @@ bool mxs::server_set_status(SERVER* server, int bit, string* errmsg_out) * @param server The server to update * @param bit The bit to clear for the server */ -bool mxs::server_clear_status(SERVER* server, int bit, string* errmsg_out) +bool mxs::server_clear_status(SERVER* srv, int bit, string* errmsg_out) { + Server* server = static_cast(srv); bool written = false; MXS_MONITOR* mon = monitor_server_in_use(server); - spinlock_acquire(&server->lock); + std::lock_guard guard(server->lock); + if (mon && mon->state == MONITOR_STATE_RUNNING) { // See server_set_status(). @@ -1359,7 +1359,7 @@ bool mxs::server_clear_status(SERVER* server, int bit, string* errmsg_out) server_clear_status_nolock(server, bit); written = true; } - spinlock_release(&server->lock); + return written; }