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.
This commit is contained in:
Markus Mäkelä
2018-09-26 16:05:39 +03:00
parent c0bc611f03
commit 3ee5d9a8ea
3 changed files with 32 additions and 26 deletions

View File

@ -134,12 +134,11 @@ typedef struct server
SERVER_PARAM* parameters; /**< Additional custom parameters which may affect routing SERVER_PARAM* parameters; /**< Additional custom parameters which may affect routing
* decisions. */ * decisions. */
// Base variables // Base variables
SPINLOCK lock; /**< Access lock. Required when modifying server status or settings. */ bool is_active; /**< Server is active and has not been "destroyed" */
bool is_active; /**< Server is active and has not been "destroyed" */ void* auth_instance; /**< Authenticator instance data */
void* auth_instance;/**< Authenticator instance data */ SSL_LISTENER* server_ssl; /**< SSL data */
SSL_LISTENER* server_ssl; /**< SSL data */ DCB** persistent; /**< List of unused persistent connections to the server */
DCB** persistent; /**< List of unused persistent connections to the server */ uint8_t charset; /**< Server character set. Read from backend and sent to client. */
uint8_t charset; /**< Server character set. Read from backend and sent to client. */
// Statistics and events // Statistics and events
SERVER_STATS stats; /**< The server statistics, e.g. number of connections */ SERVER_STATS stats; /**< The server statistics, e.g. number of connections */
int persistmax; /**< Maximum pool size actually achieved since startup */ int persistmax; /**< Maximum pool size actually achieved since startup */

View File

@ -16,6 +16,10 @@
* Internal header for the server type * Internal header for the server type
*/ */
#include <maxbase/ccdefs.hh>
#include <mutex>
#include <maxbase/average.hh> #include <maxbase/average.hh>
#include <maxscale/server.h> #include <maxscale/server.h>
#include <maxscale/resultset.hh> #include <maxscale/resultset.hh>
@ -48,6 +52,9 @@ public:
response_time->add(ave, num_samples); response_time->add(ave, num_samples);
} }
// TODO: Do all access to Server via methods
mutable std::mutex lock;
private: private:
// nantti, TODO. Decide whether to expose some of this in config, or if the values // 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 // can be calculated at runtime. The "500" or sample_max affects how often a

View File

@ -37,7 +37,6 @@
#include <maxscale/config.h> #include <maxscale/config.h>
#include <maxscale/service.h> #include <maxscale/service.h>
#include <maxscale/session.h> #include <maxscale/session.h>
#include <maxscale/spinlock.h>
#include <maxscale/dcb.h> #include <maxscale/dcb.h>
#include <maxscale/poll.h> #include <maxscale/poll.h>
#include <maxscale/log.h> #include <maxscale/log.h>
@ -163,7 +162,6 @@ SERVER* server_alloc(const char* name, MXS_CONFIG_PARAMETER* params)
server->persistmaxtime = config_get_integer(params, CN_PERSISTMAXTIME); server->persistmaxtime = config_get_integer(params, CN_PERSISTMAXTIME);
server->proxy_protocol = config_get_bool(params, CN_PROXY_PROTOCOL); server->proxy_protocol = config_get_bool(params, CN_PROXY_PROTOCOL);
server->parameters = NULL; server->parameters = NULL;
spinlock_init(&server->lock);
server->is_active = true; server->is_active = true;
server->auth_instance = auth_instance; server->auth_instance = auth_instance;
server->server_ssl = ssl; server->server_ssl = ssl;
@ -875,10 +873,11 @@ static SERVER_PARAM* allocate_parameter(const char* name, const char* value)
return param; return param;
} }
bool server_remove_parameter(SERVER* server, const char* name) bool server_remove_parameter(SERVER* srv, const char* name)
{ {
Server* server = static_cast<Server*>(srv);
bool rval = false; bool rval = false;
spinlock_acquire(&server->lock); std::lock_guard<std::mutex> guard(server->lock);
for (SERVER_PARAM* p = server->parameters; p; p = p->next) 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; 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<Server*>(srv);
SERVER_PARAM* param = allocate_parameter(name, value); SERVER_PARAM* param = allocate_parameter(name, value);
if (param) if (param)
{ {
spinlock_acquire(&server->lock); std::lock_guard<std::mutex> guard(server->lock);
// Insert new value // Insert new value
param->next = server->parameters; param->next = server->parameters;
@ -915,8 +914,6 @@ void server_set_parameter(SERVER* server, const char* name, const char* value)
break; 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 * @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); const Server* server = static_cast<const Server*>(srv);
size_t len = server_get_parameter_nolock(server, name, out, size); std::lock_guard<std::mutex> guard(server->lock);
spinlock_release(&server->lock); return server_get_parameter_nolock(server, name, out, size);
return len;
} }
/** /**
@ -1275,14 +1271,16 @@ bool server_serialize(const SERVER* server)
* @param server The server to update * @param server The server to update
* @param bit The bit to set for the server * @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<Server*>(srv);
bool written = false; bool written = false;
/* First check if the server is monitored. This isn't done under a lock /* 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 * but the race condition cannot cause significant harm. Monitors are never
* freed so the pointer stays valid. */ * freed so the pointer stays valid. */
MXS_MONITOR* mon = monitor_server_in_use(server); MXS_MONITOR* mon = monitor_server_in_use(server);
spinlock_acquire(&server->lock); std::lock_guard<std::mutex> guard(server->lock);
if (mon && mon->state == MONITOR_STATE_RUNNING) if (mon && mon->state == MONITOR_STATE_RUNNING)
{ {
/* This server is monitored, in which case modifying any other status bit than Maintenance is /* 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); server_set_status_nolock(server, bit);
written = true; written = true;
} }
spinlock_release(&server->lock);
return written; 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 server The server to update
* @param bit The bit to clear for the server * @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<Server*>(srv);
bool written = false; bool written = false;
MXS_MONITOR* mon = monitor_server_in_use(server); MXS_MONITOR* mon = monitor_server_in_use(server);
spinlock_acquire(&server->lock); std::lock_guard<std::mutex> guard(server->lock);
if (mon && mon->state == MONITOR_STATE_RUNNING) if (mon && mon->state == MONITOR_STATE_RUNNING)
{ {
// See server_set_status(). // 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); server_clear_status_nolock(server, bit);
written = true; written = true;
} }
spinlock_release(&server->lock);
return written; return written;
} }