Hi, Sergey! On Nov 27, Sergey Vojtovich wrote: > Hi Sergei, > > I'm afraid I couldn't find easy fix for this deadlock. The question is if it > is worth to spend time for more complex (and possibly less stable) solution. > Details below. > > Locking profile > --------------- > INSTALL PLUGIN: LOCK_plugin -> LOCK_system_variables_hash > (w) > SHOW VARIABLES: LOCK_system_variables_hash (r) -> LOCK_global_system_variables > change_user(): LOCK_global_system_variables -> LOCK_plugin > > INSTALL PLUGIN > -------------- > Needs LOCK_plugin to register new plugin. > Needs LOCK_system_variables_hash to register plugin variables. > Doesn't seem to need both locks at the same time. > Similar lock order is used in a few places. > > SHOW VARIABLES > -------------- > Needs LOCK_system_variables_hash to iterate system_variable_hash. > Needs LOCK_global_system_variables to read variable value. > Does seem to need both locks at the same time. > This lock order is not used in other places. > > change_user() > ------------- > Needs LOCK_global_system_variables to read > global_system_variable.table_plugin. > Needs LOCK_plugin to my_plugin_lock(table_plugin). > Does seem to need both locks at the same time. > Similar lock order is used in a few places.
Observations: THD::init() starts from locking LOCK_global_system_variables, then invokes plugin_thdvar_init(), that invokes cleanup_variables(), that read-locks LOCK_system_variables_hash. This is directly against SHOW VARIABLES order. Suggestion: swap the order of cleanup_variables() and LOCK_global_system_variables. > "SHOW VARIABLES" looks correct. So I have only two ideas: > - Fix INSTALL PLUGIN so it releases LOCK_plugin before registering variables. > Sounds like the best solution, but there are a few more things to fix, e.g. > UNINTALL PLUGIN. Is it possible? On a quick look I wasn't able to answer that :( > - Store default storage engine as a string, not Sys_var_plugin. That'd mean, that neither global nor local @@default_storage_engine setting locks the plugin. We'll need to do a hash lookup and lock the plugin for every statement that uses the engine. Which may be or may not be a problem. Here's yet another idea: don't protect plugin->ref_cnt and plugin->state with LOCK_plugin. Then intern_plugin_lock and intern_plugin_unlock could be completely lock-free. One would need to increment ref_cnt before or at the same time as checking the state. It *seems* that the rest of the code could support that with very few changes. But it'd need more analysys. Regards, Sergei _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
