Commit 07a0173f authored by Sergei Golubchik's avatar Sergei Golubchik

perfschema: LOCK_all_status_vars not LOCK_status

to iterate over all status variables one should use
LOCK_all_status_vars not LOCK_status

this fixes sporadic mutex lock inversion in plugins.password_reuse_check:
* acl_cache->lock is taken over complex operations that might increment
  status counters (under LOCK_status).
* acl_cache->lock is needed to get the values of Acl% status variables
  when iterating over status variables
parent fd0b47f9
...@@ -762,7 +762,7 @@ bool PFS_status_variable_cache::filter_show_var(const SHOW_VAR *show_var, bool s ...@@ -762,7 +762,7 @@ bool PFS_status_variable_cache::filter_show_var(const SHOW_VAR *show_var, bool s
/** /**
Build an array of SHOW_VARs from the global status array. Expand nested Build an array of SHOW_VARs from the global status array. Expand nested
subarrays, filter unwanted variables. subarrays, filter unwanted variables.
NOTE: Must be done inside of LOCK_status to guard against plugin load/unload. NOTE: Must be done under LOCK_all_status_vars
*/ */
bool PFS_status_variable_cache::init_show_var_array(enum_var_type scope, bool strict) bool PFS_status_variable_cache::init_show_var_array(enum_var_type scope, bool strict)
{ {
...@@ -881,14 +881,12 @@ char * PFS_status_variable_cache::make_show_var_name(const char* prefix, const c ...@@ -881,14 +881,12 @@ char * PFS_status_variable_cache::make_show_var_name(const char* prefix, const c
*/ */
bool PFS_status_variable_cache::do_initialize_session(void) bool PFS_status_variable_cache::do_initialize_session(void)
{ {
/* Acquire LOCK_status to guard against plugin load/unload. */ /* Acquire LOCK_all_status_vars to guard against plugin load/unload. */
//if (m_current_thd->fill_status_recursion_level++ == 0) mysql_rwlock_rdlock(&LOCK_all_status_vars);
mysql_mutex_lock(&LOCK_status);
bool ret= init_show_var_array(OPT_SESSION, true); bool ret= init_show_var_array(OPT_SESSION, true);
//if (m_current_thd->fill_status_recursion_level-- == 1) mysql_rwlock_unlock(&LOCK_all_status_vars);
mysql_mutex_unlock(&LOCK_status);
return ret; return ret;
} }
...@@ -917,13 +915,12 @@ int PFS_status_variable_cache::do_materialize_global(void) ...@@ -917,13 +915,12 @@ int PFS_status_variable_cache::do_materialize_global(void)
m_materialized= false; m_materialized= false;
DEBUG_SYNC(m_current_thd, "before_materialize_global_status_array"); DEBUG_SYNC(m_current_thd, "before_materialize_global_status_array");
/* Acquire LOCK_status to guard against plugin load/unload. */ /* Acquire LOCK_all_status_vars to guard against plugin load/unload. */
//if (m_current_thd->fill_status_recursion_level++ == 0) mysql_rwlock_rdlock(&LOCK_all_status_vars);
mysql_mutex_lock(&LOCK_status);
/* /*
Build array of SHOW_VARs from global status array. Do this within Build array of SHOW_VARs from global status array. Do this under
LOCK_status to ensure that the array remains unchanged during LOCK_all_status_vars to ensure that the array remains unchanged during
materialization. materialization.
*/ */
if (!m_external_init) if (!m_external_init)
...@@ -946,8 +943,7 @@ int PFS_status_variable_cache::do_materialize_global(void) ...@@ -946,8 +943,7 @@ int PFS_status_variable_cache::do_materialize_global(void)
*/ */
manifest(m_current_thd, m_show_var_array.front(), &status_totals, "", false, true); manifest(m_current_thd, m_show_var_array.front(), &status_totals, "", false, true);
//if (m_current_thd->fill_status_recursion_level-- == 1) mysql_rwlock_unlock(&LOCK_all_status_vars);
mysql_mutex_unlock(&LOCK_status);
m_materialized= true; m_materialized= true;
DEBUG_SYNC(m_current_thd, "after_materialize_global_status_array"); DEBUG_SYNC(m_current_thd, "after_materialize_global_status_array");
...@@ -967,13 +963,11 @@ int PFS_status_variable_cache::do_materialize_all(THD* unsafe_thd) ...@@ -967,13 +963,11 @@ int PFS_status_variable_cache::do_materialize_all(THD* unsafe_thd)
m_materialized= false; m_materialized= false;
m_cache.clear(); m_cache.clear();
/* Avoid recursive acquisition of LOCK_status. */ mysql_rwlock_rdlock(&LOCK_all_status_vars);
//if (m_current_thd->fill_status_recursion_level++ == 0)
mysql_mutex_lock(&LOCK_status);
/* /*
Build array of SHOW_VARs from global status array. Do this within Build array of SHOW_VARs from global status array. Do this under
LOCK_status to ensure that the array remains unchanged while this LOCK_all_status_vars to ensure that the array remains unchanged while this
thread is materialized. thread is materialized.
*/ */
if (!m_external_init) if (!m_external_init)
...@@ -996,8 +990,7 @@ int PFS_status_variable_cache::do_materialize_all(THD* unsafe_thd) ...@@ -996,8 +990,7 @@ int PFS_status_variable_cache::do_materialize_all(THD* unsafe_thd)
ret= 0; ret= 0;
} }
//if (m_current_thd->fill_status_recursion_level-- == 1) mysql_rwlock_unlock(&LOCK_all_status_vars);
mysql_mutex_unlock(&LOCK_status);
return ret; return ret;
} }
...@@ -1013,13 +1006,11 @@ int PFS_status_variable_cache::do_materialize_session(THD* unsafe_thd) ...@@ -1013,13 +1006,11 @@ int PFS_status_variable_cache::do_materialize_session(THD* unsafe_thd)
m_materialized= false; m_materialized= false;
m_cache.clear(); m_cache.clear();
/* Avoid recursive acquisition of LOCK_status. */ mysql_rwlock_rdlock(&LOCK_all_status_vars);
//if (m_current_thd->fill_status_recursion_level++ == 0)
mysql_mutex_lock(&LOCK_status);
/* /*
Build array of SHOW_VARs from global status array. Do this within Build array of SHOW_VARs from global status array. Do this under
LOCK_status to ensure that the array remains unchanged while this LOCK_all_status_vars to ensure that the array remains unchanged while this
thread is materialized. thread is materialized.
*/ */
if (!m_external_init) if (!m_external_init)
...@@ -1042,8 +1033,7 @@ int PFS_status_variable_cache::do_materialize_session(THD* unsafe_thd) ...@@ -1042,8 +1033,7 @@ int PFS_status_variable_cache::do_materialize_session(THD* unsafe_thd)
ret= 0; ret= 0;
} }
//if (m_current_thd->fill_status_recursion_level-- == 1) mysql_rwlock_unlock(&LOCK_all_status_vars);
mysql_mutex_unlock(&LOCK_status);
return ret; return ret;
} }
...@@ -1060,9 +1050,8 @@ int PFS_status_variable_cache::do_materialize_session(PFS_thread *pfs_thread) ...@@ -1060,9 +1050,8 @@ int PFS_status_variable_cache::do_materialize_session(PFS_thread *pfs_thread)
m_materialized= false; m_materialized= false;
m_cache.clear(); m_cache.clear();
/* Acquire LOCK_status to guard against plugin load/unload. */ /* Acquire LOCK_all_status_vars to guard against plugin load/unload. */
//if (m_current_thd->fill_status_recursion_level++ == 0) mysql_rwlock_rdlock(&LOCK_all_status_vars);
mysql_mutex_lock(&LOCK_status);
/* The SHOW_VAR array must be initialized externally. */ /* The SHOW_VAR array must be initialized externally. */
assert(m_initialized); assert(m_initialized);
...@@ -1084,8 +1073,7 @@ int PFS_status_variable_cache::do_materialize_session(PFS_thread *pfs_thread) ...@@ -1084,8 +1073,7 @@ int PFS_status_variable_cache::do_materialize_session(PFS_thread *pfs_thread)
ret= 0; ret= 0;
} }
//if (m_current_thd->fill_status_recursion_level-- == 1) mysql_rwlock_unlock(&LOCK_all_status_vars);
mysql_mutex_unlock(&LOCK_status);
return ret; return ret;
} }
...@@ -1104,9 +1092,8 @@ int PFS_status_variable_cache::do_materialize_client(PFS_client *pfs_client) ...@@ -1104,9 +1092,8 @@ int PFS_status_variable_cache::do_materialize_client(PFS_client *pfs_client)
m_materialized= false; m_materialized= false;
m_cache.clear(); m_cache.clear();
/* Acquire LOCK_status to guard against plugin load/unload. */ /* Acquire LOCK_all_status_vars to guard against plugin load/unload. */
//if (m_current_thd->fill_status_recursion_level++ == 0) mysql_rwlock_rdlock(&LOCK_all_status_vars);
mysql_mutex_lock(&LOCK_status);
/* The SHOW_VAR array must be initialized externally. */ /* The SHOW_VAR array must be initialized externally. */
assert(m_initialized); assert(m_initialized);
...@@ -1123,8 +1110,7 @@ int PFS_status_variable_cache::do_materialize_client(PFS_client *pfs_client) ...@@ -1123,8 +1110,7 @@ int PFS_status_variable_cache::do_materialize_client(PFS_client *pfs_client)
*/ */
manifest(m_current_thd, m_show_var_array.front(), &status_totals, "", false, true); manifest(m_current_thd, m_show_var_array.front(), &status_totals, "", false, true);
//if (m_current_thd->fill_status_recursion_level-- == 1) mysql_rwlock_unlock(&LOCK_all_status_vars);
mysql_mutex_unlock(&LOCK_status);
m_materialized= true; m_materialized= true;
return 0; return 0;
...@@ -1217,7 +1203,7 @@ Status_variable::Status_variable(const SHOW_VAR *show_var, STATUS_VAR *status_va ...@@ -1217,7 +1203,7 @@ Status_variable::Status_variable(const SHOW_VAR *show_var, STATUS_VAR *status_va
/** /**
Resolve status value, convert to string. Resolve status value, convert to string.
show_var->value is an offset into status_vars. show_var->value is an offset into status_vars.
NOTE: Assumes LOCK_status is held. NOTE: Assumes LOCK_all_status_vars is held.
*/ */
void Status_variable::init(const SHOW_VAR *show_var, STATUS_VAR *status_vars, enum_var_type query_scope) void Status_variable::init(const SHOW_VAR *show_var, STATUS_VAR *status_vars, enum_var_type query_scope)
{ {
...@@ -1283,7 +1269,7 @@ void sum_account_status(PFS_client *pfs_account, STATUS_VAR *status_totals) ...@@ -1283,7 +1269,7 @@ void sum_account_status(PFS_client *pfs_account, STATUS_VAR *status_totals)
/** /**
Reset aggregated status counter stats for account, user and host. Reset aggregated status counter stats for account, user and host.
NOTE: Assumes LOCK_status is held. NOTE: Assumes LOCK_all_status_vars is held.
*/ */
void reset_pfs_status_stats() void reset_pfs_status_stats()
{ {
......
...@@ -1356,8 +1356,7 @@ PFS_connection_status_visitor::~PFS_connection_status_visitor() = default; ...@@ -1356,8 +1356,7 @@ PFS_connection_status_visitor::~PFS_connection_status_visitor() = default;
/** Aggregate from global status. */ /** Aggregate from global status. */
void PFS_connection_status_visitor::visit_global() void PFS_connection_status_visitor::visit_global()
{ {
/* NOTE: Requires lock on LOCK_status. */ /* NOTE: Requires lock on LOCK_all_status_vars. */
mysql_mutex_assert_owner(&LOCK_status);
add_to_status(m_status_vars, &global_status_var); add_to_status(m_status_vars, &global_status_var);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment