diff --git a/mysql-test/main/plugin_checkpoint.opt b/mysql-test/main/plugin_checkpoint.opt new file mode 100644 index 00000000000..beae84b3862 --- /dev/null +++ b/mysql-test/main/plugin_checkpoint.opt @@ -0,0 +1 @@ +--log-bin diff --git a/mysql-test/main/plugin_checkpoint.result b/mysql-test/main/plugin_checkpoint.result new file mode 100644 index 00000000000..cd4815e251d --- /dev/null +++ b/mysql-test/main/plugin_checkpoint.result @@ -0,0 +1,16 @@ +# +# MDEV-40038 tp_foreach() crashes on a dying transaction participant +# +INSTALL SONAME 'ha_example'; +connect con1,localhost,root; +SET DEBUG_SYNC= 'ha_finalize_handlerton_before_slot_clear SIGNAL deinit_started WAIT_FOR deinit_go'; +UNINSTALL SONAME 'ha_example'; +connection default; +SET DEBUG_SYNC= 'now WAIT_FOR deinit_started'; +RESET MASTER; +SET DEBUG_SYNC= 'now SIGNAL deinit_go'; +connection con1; +disconnect con1; +connection default; +SET DEBUG_SYNC= 'RESET'; +# End of 11.8 tests diff --git a/mysql-test/main/plugin_checkpoint.test b/mysql-test/main/plugin_checkpoint.test new file mode 100644 index 00000000000..dd650979647 --- /dev/null +++ b/mysql-test/main/plugin_checkpoint.test @@ -0,0 +1,28 @@ +--source include/have_debug_sync.inc +--source include/have_log_bin.inc +--source include/have_example_plugin.inc + +--echo # +--echo # MDEV-40038 tp_foreach() crashes on a dying transaction participant +--echo # + +INSTALL SONAME 'ha_example'; + +--connect con1,localhost,root +SET DEBUG_SYNC= 'ha_finalize_handlerton_before_slot_clear SIGNAL deinit_started WAIT_FOR deinit_go'; +--send UNINSTALL SONAME 'ha_example' + +--connection default +SET DEBUG_SYNC= 'now WAIT_FOR deinit_started'; +# The engine is PLUGIN_IS_DYING but hton2plugin[] still points to it. +RESET MASTER; +SET DEBUG_SYNC= 'now SIGNAL deinit_go'; + +--connection con1 +--reap +--disconnect con1 + +--connection default +SET DEBUG_SYNC= 'RESET'; + +--echo # End of 11.8 tests diff --git a/sql/handler.cc b/sql/handler.cc index 38c502c889f..775f7938f2f 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -616,6 +616,7 @@ int ha_finalize_handlerton(void *plugin_) /* Make sure we are not unpluging another plugin */ DBUG_ASSERT(hton2plugin[hton->slot] == plugin); DBUG_ASSERT(hton->slot < MAX_HA); + DEBUG_SYNC(current_thd, "ha_finalize_handlerton_before_slot_clear"); hton2plugin[hton->slot]= NULL; } @@ -880,6 +881,17 @@ int ha_end() */ typedef bool (tp_foreach_func)(THD *thd, transaction_participant *tp, void *arg); +/* + Call func for each READY transaction participant. + + LOCK_plugin is not held while func runs. func calls into the engine, and + the engine can take plugin locks itself or wait for its own threads, which + can deadlock with a thread that holds LOCK_plugin. reap_plugins() and + plugin_deinitialize() also run without LOCK_plugin for the same reason. + Instead, each plugin gets a reference count under LOCK_plugin, which keeps + it from being deinitialized or freed while func runs. The state check and + the reference count happen in one step, in plugin_lock_ready(). +*/ static bool tp_foreach(THD *thd, tp_foreach_func *func, void *arg) { int j=0, err= 0; @@ -888,7 +900,25 @@ static bool tp_foreach(THD *thd, tp_foreach_func *func, void *arg) { if (st_plugin_int *pi= hton2plugin[i]) { - locks[j]= plugin_lock(NULL, plugin_int_to_ref(pi)); + /* + Only PLUGIN_IS_READY engines are visited, as plugin_foreach() does. + The slot stays set in hton2plugin[] until the end of + ha_finalize_handlerton(), so an engine that reap_plugins() is + deinitializing (PLUGIN_IS_DYING) is still found here. The lock then + fails and the engine is skipped, as is an engine that is not yet + READY. A lock failure for a READY engine is out of memory (debug + builds only). It must not skip a live participant, so it is an error. + */ + bool failed; + if (!(locks[j]= plugin_lock_ready(NULL, plugin_int_to_ref(pi), &failed))) + { + if (failed) + { + err= 1; + break; + } + continue; + } if ((err= func(thd, plugin_hton(locks[j++]), arg))) break; } diff --git a/sql/sql_plugin.cc b/sql/sql_plugin.cc index a5f133fb979..94119427b6e 100644 --- a/sql/sql_plugin.cc +++ b/sql/sql_plugin.cc @@ -1064,6 +1064,34 @@ plugin_ref plugin_lock(THD *thd, plugin_ref ptr) } +/** + Lock a plugin only if it is PLUGIN_IS_READY. + + Like plugin_lock(), but returns NULL for a plugin that is still being + installed, is being deinitialized, or is already freed. The caller must + unlock a non-NULL result with plugin_unlock(). + + @param[out] failed Set when the plugin is READY but locking failed + (out of memory in debug builds). The state is read + under LOCK_plugin, so it matches the lock result. + A NULL result with *failed false means the plugin + is not READY. +*/ +plugin_ref plugin_lock_ready(THD *thd, plugin_ref ptr, bool *failed) +{ + LEX *lex= thd ? thd->lex : 0; + plugin_ref rc; + DBUG_ENTER("plugin_lock_ready"); + + mysql_mutex_lock(&LOCK_plugin); + plugin_ref_to_int(ptr)->locks_total++; + rc= intern_plugin_lock(lex, ptr, PLUGIN_IS_READY); + *failed= !rc && (plugin_ref_to_int(ptr)->state == PLUGIN_IS_READY); + mysql_mutex_unlock(&LOCK_plugin); + DBUG_RETURN(rc); +} + + /* Notes on lifetime: diff --git a/sql/sql_plugin.h b/sql/sql_plugin.h index f1650c52deb..d5f1ed92da7 100644 --- a/sql/sql_plugin.h +++ b/sql/sql_plugin.h @@ -170,6 +170,7 @@ extern bool plugin_is_ready(const LEX_CSTRING *name, int type); #define my_plugin_lock_by_name(A,B,C) plugin_lock_by_name(A,B,C) #define my_plugin_lock(A,B) plugin_lock(A,B) extern plugin_ref plugin_lock(THD *thd, plugin_ref ptr); +extern plugin_ref plugin_lock_ready(THD *thd, plugin_ref ptr, bool *failed); extern plugin_ref plugin_lock_by_name(THD *thd, const LEX_CSTRING *name, int type); extern void plugin_unlock(THD *thd, plugin_ref plugin);