diff --git a/include/maxscale/mysql_utils.h b/include/maxscale/mysql_utils.h index c24d98704..082136c83 100644 --- a/include/maxscale/mysql_utils.h +++ b/include/maxscale/mysql_utils.h @@ -134,12 +134,13 @@ mxs_mysql_name_kind_t mxs_mysql_name_to_pcre(char* pcre, mxs_pcre_quote_approach_t approach); /** - * Set the server information + * Get server information from connector, store it to server object. This does not query + * the server as the data has been read while connecting. * - * @param mysql A MySQL handle to the server. - * @param server The server whose version information should be updated. + * @param mysql MySQL handle from which information is read. + * @param server Server object to write. */ -void mxs_mysql_set_server_version(MYSQL* mysql, SERVER* server); +void mxs_mysql_update_server_version(MYSQL* mysql, SERVER* server); /** * Enable/disable the logging of all SQL statements MaxScale sends to diff --git a/server/core/mysql_utils.cc b/server/core/mysql_utils.cc index d009fb946..c82b8d8fd 100644 --- a/server/core/mysql_utils.cc +++ b/server/core/mysql_utils.cc @@ -407,25 +407,13 @@ mxs_mysql_name_kind_t mxs_mysql_name_to_pcre(char* pcre, return rv; } -void mxs_mysql_set_server_version(MYSQL* mysql, SERVER* server) +void mxs_mysql_update_server_version(MYSQL* mysql, SERVER* server) { + // This function should only be called for a live connection. const char* version_string = mysql_get_server_info(mysql); - - if (version_string) - { - unsigned long version = mysql_get_server_version(mysql); - - server_set_version(server, version_string, version); - - if (strcasestr(version_string, "mariadb") != NULL) - { - server->server_type = SERVER_TYPE_MARIADB; - } - else - { - server->server_type = SERVER_TYPE_MYSQL; - } - } + unsigned long version_num = mysql_get_server_version(mysql); + mxb_assert(version_string != NULL && version_num != 0); + server_set_version(server, version_string, version_num); } void mxs_mysql_set_log_statements(bool enable) diff --git a/server/core/server.cc b/server/core/server.cc index 8ddede3ce..aef0b9f2e 100644 --- a/server/core/server.cc +++ b/server/core/server.cc @@ -1092,13 +1092,11 @@ uint64_t server_map_status(const char* str) */ void server_set_version_string(SERVER* server, const char* version_string) { - // There is a race here. The string may be accessed, while we are - // updating it. Thus we take some precautions to ensure that the - // string cannot be completely garbled at any point. - + // Possible data race. The string may be accessed while we are updating it. + // Take some precautions to ensure that the string cannot be completely garbled at any point. + // Strictly speaking, this is not fool-proof as writes may not appear in order to the reader. size_t old_len = strlen(server->version_string); size_t new_len = strlen(version_string); - if (new_len >= MAX_SERVER_VERSION_LEN) { new_len = MAX_SERVER_VERSION_LEN - 1; @@ -1111,10 +1109,9 @@ void server_set_version_string(SERVER* server, const char* version_string) memset(server->version_string + new_len, 0, old_len - new_len); } + // No null-byte needs to be set. The array starts out as all zeros and the above memset adds + // the necessary null, should the new string be shorter than the old. strncpy(server->version_string, version_string, new_len); - // No null-byte needs to be set. The array starts out as all zeros - // and the above memset adds the necessary null, should the new string - // be shorter than the old. } /** @@ -1128,8 +1125,9 @@ void server_set_version_string(SERVER* server, const char* version_string) void server_set_version(SERVER* server, const char* version_string, uint64_t version) { server_set_version_string(server, version_string); - atomic_store_uint64(&server->version, version); + bool is_mariadb = (strcasestr(version_string, "mariadb") != NULL); + server->server_type = is_mariadb ? SERVER_TYPE_MARIADB : SERVER_TYPE_MYSQL; } uint64_t server_get_version(const SERVER* server) diff --git a/server/modules/authenticator/MySQLAuth/dbusers.cc b/server/modules/authenticator/MySQLAuth/dbusers.cc index 868eb651a..b2da46c71 100644 --- a/server/modules/authenticator/MySQLAuth/dbusers.cc +++ b/server/modules/authenticator/MySQLAuth/dbusers.cc @@ -687,7 +687,7 @@ static bool check_server_permissions(SERVICE* service, if (server->version_string[0] == 0) { - mxs_mysql_set_server_version(mysql, server); + mxs_mysql_update_server_version(mysql, server); } const char* format = "SELECT user, host, %s, Select_priv FROM mysql.user limit 1"; @@ -1010,7 +1010,7 @@ int get_users_from_server(MYSQL* con, SERVER_REF* server_ref, SERVICE* service, { if (server_ref->server->version_string[0] == 0) { - mxs_mysql_set_server_version(con, server_ref->server); + mxs_mysql_update_server_version(con, server_ref->server); } char* query = get_users_query(server_ref->server->version_string, diff --git a/server/modules/monitor/galeramon/galeramon.cc b/server/modules/monitor/galeramon/galeramon.cc index 550ce351c..77b6bb738 100644 --- a/server/modules/monitor/galeramon/galeramon.cc +++ b/server/modules/monitor/galeramon/galeramon.cc @@ -132,7 +132,7 @@ void GaleraMonitor::update_server_status(MXS_MONITORED_SERVER* monitored_server) char* server_string; /* get server version string */ - mxs_mysql_set_server_version(monitored_server->con, monitored_server->server); + mxs_mysql_update_server_version(monitored_server->con, monitored_server->server); server_string = monitored_server->server->version_string; /* Check if the the Galera FSM shows this node is joined to the cluster */ diff --git a/server/modules/monitor/mariadbmon/mariadbserver.cc b/server/modules/monitor/mariadbmon/mariadbserver.cc index 4deb47da5..a96c35e1e 100644 --- a/server/modules/monitor/mariadbmon/mariadbserver.cc +++ b/server/modules/monitor/mariadbmon/mariadbserver.cc @@ -881,7 +881,7 @@ void MariaDBServer::update_server_version() /* Get server version string, also get/set numeric representation. This function does not query the * server, since the data was obtained when connecting. */ - mxs_mysql_set_server_version(conn, srv); + mxs_mysql_update_server_version(conn, srv); // Check whether this server is a MaxScale Binlog Server. MYSQL_RES* result; diff --git a/server/modules/monitor/mmmon/mmmon.cc b/server/modules/monitor/mmmon/mmmon.cc index 5b88290cc..80afdaf20 100644 --- a/server/modules/monitor/mmmon/mmmon.cc +++ b/server/modules/monitor/mmmon/mmmon.cc @@ -90,7 +90,7 @@ void MMMonitor::update_server_status(MXS_MONITORED_SERVER* monitored_server) server_version = mysql_get_server_version(monitored_server->con); /* get server version string */ - mxs_mysql_set_server_version(monitored_server->con, monitored_server->server); + mxs_mysql_update_server_version(monitored_server->con, monitored_server->server); server_string = monitored_server->server->version_string; /* get server_id form current node */ diff --git a/server/modules/monitor/ndbclustermon/ndbclustermon.cc b/server/modules/monitor/ndbclustermon/ndbclustermon.cc index 824341348..a534ba25e 100644 --- a/server/modules/monitor/ndbclustermon/ndbclustermon.cc +++ b/server/modules/monitor/ndbclustermon/ndbclustermon.cc @@ -51,7 +51,7 @@ void NDBCMonitor::update_server_status(MXS_MONITORED_SERVER* monitored_server) char* server_string; /* get server version string */ - mxs_mysql_set_server_version(monitored_server->con, monitored_server->server); + mxs_mysql_update_server_version(monitored_server->con, monitored_server->server); server_string = monitored_server->server->version_string; /* Check if the the SQL node is able to contact one or more data nodes */