From 7cdf759c86a3b80bf215fcc6baab87b930425170 Mon Sep 17 00:00:00 2001 From: Vladislav Vaintroub Date: Sat, 27 Jan 2018 17:46:31 +0000 Subject: MDEV-14485 Server hangs on startup in THD::init Solve 3 way deadlock between plugin_initialiaze(), THD::init() and mysql_sys_var_char(). The deadlock exists because of the lock order inversion between LOCK_global_system_variables mutex and LOCK_system_variables_hash read-write lock- In this case, it is enough to change LOCK_system_variables_hash to prefer reads to fix the deadlock, i.e change it to mysql_prlock_t --- sql/mysqld.cc | 6 +++--- sql/mysqld.h | 2 +- sql/set_var.cc | 8 ++++---- sql/sql_plugin.cc | 22 +++++++++++----------- sql/sql_show.cc | 4 ++-- 5 files changed, 21 insertions(+), 21 deletions(-) (limited to 'sql') diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 9de442ebe77..24021f327b8 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -778,7 +778,7 @@ mysql_mutex_t LOCK_prepared_stmt_count; mysql_mutex_t LOCK_des_key_file; #endif mysql_rwlock_t LOCK_grant, LOCK_sys_init_connect, LOCK_sys_init_slave; -mysql_rwlock_t LOCK_system_variables_hash; +mysql_prlock_t LOCK_system_variables_hash; mysql_cond_t COND_thread_count, COND_start_thread; pthread_t signal_thread; pthread_attr_t connection_attrib; @@ -2354,7 +2354,7 @@ static void clean_up_mutexes() mysql_rwlock_destroy(&LOCK_sys_init_connect); mysql_rwlock_destroy(&LOCK_sys_init_slave); mysql_mutex_destroy(&LOCK_global_system_variables); - mysql_rwlock_destroy(&LOCK_system_variables_hash); + mysql_prlock_destroy(&LOCK_system_variables_hash); mysql_mutex_destroy(&LOCK_short_uuid_generator); mysql_mutex_destroy(&LOCK_prepared_stmt_count); mysql_mutex_destroy(&LOCK_error_messages); @@ -4680,7 +4680,7 @@ static int init_thread_environment() &LOCK_global_system_variables, MY_MUTEX_INIT_FAST); mysql_mutex_record_order(&LOCK_active_mi, &LOCK_global_system_variables); mysql_mutex_record_order(&LOCK_status, &LOCK_thread_count); - mysql_rwlock_init(key_rwlock_LOCK_system_variables_hash, + mysql_prlock_init(key_rwlock_LOCK_system_variables_hash, &LOCK_system_variables_hash); mysql_mutex_init(key_LOCK_prepared_stmt_count, &LOCK_prepared_stmt_count, MY_MUTEX_INIT_FAST); diff --git a/sql/mysqld.h b/sql/mysqld.h index 912fe3696ae..310bc1dd342 100644 --- a/sql/mysqld.h +++ b/sql/mysqld.h @@ -577,7 +577,7 @@ extern mysql_mutex_t LOCK_des_key_file; extern mysql_mutex_t LOCK_server_started; extern mysql_cond_t COND_server_started; extern mysql_rwlock_t LOCK_grant, LOCK_sys_init_connect, LOCK_sys_init_slave; -extern mysql_rwlock_t LOCK_system_variables_hash; +extern mysql_prlock_t LOCK_system_variables_hash; extern mysql_cond_t COND_thread_count, COND_start_thread; extern mysql_cond_t COND_manager; extern mysql_cond_t COND_slave_background; diff --git a/sql/set_var.cc b/sql/set_var.cc index 07395e3e708..e96e636e3d3 100644 --- a/sql/set_var.cc +++ b/sql/set_var.cc @@ -595,10 +595,10 @@ int mysql_del_sys_var_chain(sys_var *first) { int result= 0; - mysql_rwlock_wrlock(&LOCK_system_variables_hash); + mysql_prlock_wrlock(&LOCK_system_variables_hash); for (sys_var *var= first; var; var= var->next) result|= my_hash_delete(&system_variable_hash, (uchar*) var); - mysql_rwlock_unlock(&LOCK_system_variables_hash); + mysql_prlock_unlock(&LOCK_system_variables_hash); return result; } @@ -1067,7 +1067,7 @@ int fill_sysvars(THD *thd, TABLE_LIST *tables, COND *cond) cond= make_cond_for_info_schema(thd, cond, tables); thd->count_cuted_fields= CHECK_FIELD_WARN; - mysql_rwlock_rdlock(&LOCK_system_variables_hash); + mysql_prlock_rdlock(&LOCK_system_variables_hash); for (uint i= 0; i < system_variable_hash.records; i++) { @@ -1229,7 +1229,7 @@ int fill_sysvars(THD *thd, TABLE_LIST *tables, COND *cond) } res= 0; end: - mysql_rwlock_unlock(&LOCK_system_variables_hash); + mysql_prlock_unlock(&LOCK_system_variables_hash); thd->count_cuted_fields= save_count_cuted_fields; return res; } diff --git a/sql/sql_plugin.cc b/sql/sql_plugin.cc index 2f0a5eaf9c9..24f4495d37f 100644 --- a/sql/sql_plugin.cc +++ b/sql/sql_plugin.cc @@ -1394,10 +1394,10 @@ static int plugin_initialize(MEM_ROOT *tmp_root, struct st_plugin_int *plugin, mysql_mutex_unlock(&LOCK_plugin); - mysql_rwlock_wrlock(&LOCK_system_variables_hash); + mysql_prlock_wrlock(&LOCK_system_variables_hash); if (test_plugin_options(tmp_root, plugin, argc, argv)) state= PLUGIN_IS_DISABLED; - mysql_rwlock_unlock(&LOCK_system_variables_hash); + mysql_prlock_unlock(&LOCK_system_variables_hash); if (options_only || state == PLUGIN_IS_DISABLED) { @@ -2797,11 +2797,11 @@ sys_var *find_sys_var_ex(THD *thd, const char *str, size_t length, if (!locked) mysql_mutex_lock(&LOCK_plugin); - mysql_rwlock_rdlock(&LOCK_system_variables_hash); + mysql_prlock_rdlock(&LOCK_system_variables_hash); if ((var= intern_find_sys_var(str, length)) && (pi= var->cast_pluginvar())) { - mysql_rwlock_unlock(&LOCK_system_variables_hash); + mysql_prlock_unlock(&LOCK_system_variables_hash); LEX *lex= thd ? thd->lex : 0; if (!(plugin= intern_plugin_lock(lex, plugin_int_to_ref(pi->plugin)))) var= NULL; /* failed to lock it, it must be uninstalling */ @@ -2814,7 +2814,7 @@ sys_var *find_sys_var_ex(THD *thd, const char *str, size_t length, } } else - mysql_rwlock_unlock(&LOCK_system_variables_hash); + mysql_prlock_unlock(&LOCK_system_variables_hash); if (!locked) mysql_mutex_unlock(&LOCK_plugin); @@ -3043,9 +3043,9 @@ static uchar *intern_sys_var_ptr(THD* thd, int offset, bool global_lock) if (!thd->variables.dynamic_variables_ptr || (uint)offset > thd->variables.dynamic_variables_head) { - mysql_rwlock_rdlock(&LOCK_system_variables_hash); + mysql_prlock_rdlock(&LOCK_system_variables_hash); sync_dynamic_session_variables(thd, global_lock); - mysql_rwlock_unlock(&LOCK_system_variables_hash); + mysql_prlock_unlock(&LOCK_system_variables_hash); } DBUG_RETURN((uchar*)thd->variables.dynamic_variables_ptr + offset); } @@ -3160,7 +3160,7 @@ static void cleanup_variables(struct system_variables *vars) st_bookmark *v; uint idx; - mysql_rwlock_rdlock(&LOCK_system_variables_hash); + mysql_prlock_rdlock(&LOCK_system_variables_hash); for (idx= 0; idx < bookmark_hash.records; idx++) { v= (st_bookmark*) my_hash_element(&bookmark_hash, idx); @@ -3179,7 +3179,7 @@ static void cleanup_variables(struct system_variables *vars) *ptr= NULL; } } - mysql_rwlock_unlock(&LOCK_system_variables_hash); + mysql_prlock_unlock(&LOCK_system_variables_hash); DBUG_ASSERT(vars->table_plugin == NULL); DBUG_ASSERT(vars->tmp_table_plugin == NULL); @@ -4234,10 +4234,10 @@ int thd_key_create(MYSQL_THD_KEY_T *key) PLUGIN_VAR_NOSYSVAR | PLUGIN_VAR_NOCMDOPT; char namebuf[256]; snprintf(namebuf, sizeof(namebuf), "%u", thd_key_no++); - mysql_rwlock_wrlock(&LOCK_system_variables_hash); + mysql_prlock_wrlock(&LOCK_system_variables_hash); // non-letters in the name as an extra safety st_bookmark *bookmark= register_var("\a\v\a\t\a\r", namebuf, flags); - mysql_rwlock_unlock(&LOCK_system_variables_hash); + mysql_prlock_unlock(&LOCK_system_variables_hash); if (bookmark) { *key= bookmark->offset; diff --git a/sql/sql_show.cc b/sql/sql_show.cc index 03107aa4574..60796df35d4 100644 --- a/sql/sql_show.cc +++ b/sql/sql_show.cc @@ -7399,7 +7399,7 @@ int fill_variables(THD *thd, TABLE_LIST *tables, COND *cond) COND *partial_cond= make_cond_for_info_schema(thd, cond, tables); - mysql_rwlock_rdlock(&LOCK_system_variables_hash); + mysql_prlock_rdlock(&LOCK_system_variables_hash); /* Avoid recursive LOCK_system_variables_hash acquisition in @@ -7414,7 +7414,7 @@ int fill_variables(THD *thd, TABLE_LIST *tables, COND *cond) res= show_status_array(thd, wild, enumerate_sys_vars(thd, sorted_vars, scope), scope, NULL, "", tables->table, upper_case_names, partial_cond); - mysql_rwlock_unlock(&LOCK_system_variables_hash); + mysql_prlock_unlock(&LOCK_system_variables_hash); DBUG_RETURN(res); } -- cgit v1.2.1 From 446b3d356218bff06efe4b3f5df89595cdfe8284 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Lindstr=C3=B6m?= Date: Tue, 30 Jan 2018 17:40:40 +0200 Subject: MDEV-14875: galera_new_cluster crashes mysqld when existing server contains databases Fortify wsrep_hton so that wsrep calls are not done to NULL-pointers. --- sql/wsrep_hton.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'sql') diff --git a/sql/wsrep_hton.cc b/sql/wsrep_hton.cc index 0a85a947294..e7c7b377e08 100644 --- a/sql/wsrep_hton.cc +++ b/sql/wsrep_hton.cc @@ -120,7 +120,7 @@ void wsrep_post_commit(THD* thd, bool all) case LOCAL_COMMIT: { DBUG_ASSERT(thd->wsrep_trx_meta.gtid.seqno != WSREP_SEQNO_UNDEFINED); - if (wsrep->post_commit(wsrep, &thd->wsrep_ws_handle)) + if (wsrep && wsrep->post_commit(wsrep, &thd->wsrep_ws_handle)) { DBUG_PRINT("wsrep", ("set committed fail")); WSREP_WARN("set committed fail: %llu %d", @@ -252,7 +252,7 @@ static int wsrep_rollback(handlerton *hton, THD *thd, bool all) if ((all || !thd_test_options(thd, OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)) && (thd->variables.wsrep_on && thd->wsrep_conflict_state != MUST_REPLAY)) { - if (wsrep->post_rollback(wsrep, &thd->wsrep_ws_handle)) + if (wsrep && wsrep->post_rollback(wsrep, &thd->wsrep_ws_handle)) { DBUG_PRINT("wsrep", ("setting rollback fail")); WSREP_ERROR("settting rollback fail: thd: %llu, schema: %s, SQL: %s", @@ -294,7 +294,7 @@ int wsrep_commit(handlerton *hton, THD *thd, bool all) possible changes to clean state. */ if (WSREP_PROVIDER_EXISTS) { - if (wsrep->post_rollback(wsrep, &thd->wsrep_ws_handle)) + if (wsrep && wsrep->post_rollback(wsrep, &thd->wsrep_ws_handle)) { DBUG_PRINT("wsrep", ("setting rollback fail")); WSREP_ERROR("settting rollback fail: thd: %llu, schema: %s, SQL: %s", @@ -471,7 +471,7 @@ wsrep_run_wsrep_commit(THD *thd, bool all) } else if (!rcode) { - if (WSREP_OK == rcode) + if (WSREP_OK == rcode && wsrep) rcode = wsrep->pre_commit(wsrep, (wsrep_conn_id_t)thd->thread_id, &thd->wsrep_ws_handle, -- cgit v1.2.1