diff options
| -rw-r--r-- | sql/mysqld.cc | 6 | ||||
| -rw-r--r-- | sql/set_var.cc | 58 | ||||
| -rw-r--r-- | sql/set_var.h | 5 | ||||
| -rw-r--r-- | sql/sys_vars.cc | 5 | ||||
| -rw-r--r-- | sql/sys_vars.ic | 22 |
5 files changed, 71 insertions, 25 deletions
diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 09c9c5c681e..0b8aaf1de78 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -4932,10 +4932,10 @@ init_gtid_pos_auto_engines(void) then it will certainly not be needed. */ if (gtid_pos_auto_engines) - plugins= resolve_engine_list(gtid_pos_auto_engines, - strlen(gtid_pos_auto_engines), false); + plugins= resolve_engine_list(NULL, gtid_pos_auto_engines, + strlen(gtid_pos_auto_engines), false, false); else - plugins= resolve_engine_list("", 0, false); + plugins= resolve_engine_list(NULL, "", 0, false, false); if (!plugins) return 1; mysql_mutex_lock(&LOCK_global_system_variables); diff --git a/sql/set_var.cc b/sql/set_var.cc index 78904f75661..acc744808cb 100644 --- a/sql/set_var.cc +++ b/sql/set_var.cc @@ -1301,17 +1301,18 @@ engine_list_next_item(const char **pos, const char *end_pos, static bool -resolve_engine_list_item(plugin_ref *list, uint32 *idx, +resolve_engine_list_item(THD *thd, plugin_ref *list, uint32 *idx, const char *pos, const char *pos_end, - bool error_on_unknown_engine) + bool error_on_unknown_engine, bool temp_copy) { LEX_STRING item_str; plugin_ref ref; uint32_t i; + THD *thd_or_null = (temp_copy ? thd : NULL); item_str.str= const_cast<char*>(pos); item_str.length= pos_end-pos; - ref= ha_resolve_by_name(NULL, &item_str, false); + ref= ha_resolve_by_name(thd_or_null, &item_str, false); if (!ref) { if (error_on_unknown_engine) @@ -1327,7 +1328,8 @@ resolve_engine_list_item(plugin_ref *list, uint32 *idx, { if (plugin_hton(list[i]) == plugin_hton(ref)) { - plugin_unlock(NULL, ref); + if (!temp_copy) + plugin_unlock(NULL, ref); return false; } } @@ -1341,10 +1343,16 @@ resolve_engine_list_item(plugin_ref *list, uint32 *idx, Helper for class Sys_var_pluginlist. Resolve a comma-separated list of storage engine names to a null-terminated array of plugin_ref. + + If TEMP_COPY is true, a THD must be given as well. In this case, the + allocated memory and locked plugins are registered in the THD and will + be freed / unlocked automatically. If TEMP_COPY is true, THD can be + passed as NULL, and resources must be freed explicitly later with + free_engine_list(). */ plugin_ref * -resolve_engine_list(const char *str_arg, size_t str_arg_len, - bool error_on_unknown_engine) +resolve_engine_list(THD *thd, const char *str_arg, size_t str_arg_len, + bool error_on_unknown_engine, bool temp_copy) { uint32 count, idx; const char *pos, *item_start, *item_end; @@ -1360,7 +1368,10 @@ resolve_engine_list(const char *str_arg, size_t str_arg_len, ++count; } - res= (plugin_ref *)my_malloc((count+1)*sizeof(*res), MYF(MY_ZEROFILL|MY_WME)); + if (temp_copy) + res= (plugin_ref *)thd->calloc((count+1)*sizeof(*res)); + else + res= (plugin_ref *)my_malloc((count+1)*sizeof(*res), MYF(MY_ZEROFILL|MY_WME)); if (!res) { my_error(ER_OUTOFMEMORY, MYF(0), (int)((count+1)*sizeof(*res))); @@ -1376,15 +1387,16 @@ resolve_engine_list(const char *str_arg, size_t str_arg_len, DBUG_ASSERT(idx < count); if (idx >= count) break; - if (resolve_engine_list_item(res, &idx, item_start, item_end, - error_on_unknown_engine)) + if (resolve_engine_list_item(thd, res, &idx, item_start, item_end, + error_on_unknown_engine, temp_copy)) goto err; } return res; err: - free_engine_list(res); + if (!temp_copy) + free_engine_list(res); return NULL; } @@ -1418,6 +1430,32 @@ copy_engine_list(plugin_ref *list) } for (i= 0; i < count; ++i) p[i]= my_plugin_lock(NULL, list[i]); + p[i] = NULL; + return p; +} + + +/* + Create a temporary copy of an engine list. The memory will be freed + (and the plugins unlocked) automatically, on the passed THD. +*/ +plugin_ref * +temp_copy_engine_list(THD *thd, plugin_ref *list) +{ + plugin_ref *p; + uint32 count, i; + + for (p= list, count= 0; *p; ++p, ++count) + ; + p= (plugin_ref *)thd->alloc((count+1)*sizeof(*p)); + if (!p) + { + my_error(ER_OUTOFMEMORY, MYF(0), (int)((count+1)*sizeof(*p))); + return NULL; + } + for (i= 0; i < count; ++i) + p[i]= my_plugin_lock(thd, list[i]); + p[i] = NULL; return p; } diff --git a/sql/set_var.h b/sql/set_var.h index cdd0f7da1ba..66a3eeae42d 100644 --- a/sql/set_var.h +++ b/sql/set_var.h @@ -423,10 +423,11 @@ int sys_var_init(); uint sys_var_elements(); int sys_var_add_options(DYNAMIC_ARRAY *long_options, int parse_flags); void sys_var_end(void); -plugin_ref *resolve_engine_list(const char *str_arg, size_t str_arg_len, - bool error_on_unknown_engine); +plugin_ref *resolve_engine_list(THD *thd, const char *str_arg, size_t str_arg_len, + bool error_on_unknown_engine, bool temp_copy); void free_engine_list(plugin_ref *list); plugin_ref *copy_engine_list(plugin_ref *list); +plugin_ref *temp_copy_engine_list(THD *thd, plugin_ref *list); char *pretty_print_engine_list(THD *thd, plugin_ref *list); #endif diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc index 8f0a081acc7..84b88f84bbf 100644 --- a/sql/sys_vars.cc +++ b/sql/sys_vars.cc @@ -3541,11 +3541,6 @@ check_gtid_pos_auto_engines(sys_var *self, THD *thd, set_var *var) if (running) err= true; } - if (err && var->save_result.plugins) - { - free_engine_list(var->save_result.plugins); - var->save_result.plugins= NULL; - } return err; } diff --git a/sql/sys_vars.ic b/sql/sys_vars.ic index c5fe8c0af2c..5227384f203 100644 --- a/sql/sys_vars.ic +++ b/sql/sys_vars.ic @@ -1546,6 +1546,17 @@ public: These variables don't support command-line equivalents, any such command-line options should be added manually to my_long_options in mysqld.cc + + Note on lifetimes of resources allocated: We allocate a zero-terminated array + of plugin_ref*, and lock the contained plugins. The list in the global + variable must be freed (with free_engine_list()). However, the way Sys_var + works, there is no place to explicitly free other lists, like the one + returned from get_default(). + + Therefore, the code needs to work with temporary lists, which are + registered in the THD to be automatically freed (and plugins similarly + automatically unlocked). This is why do_check() allocates a temporary + list, from which do_update() then makes a permanent copy. */ class Sys_var_pluginlist: public sys_var { @@ -1575,9 +1586,9 @@ public: plugin_ref *plugins; if (!(res=var->value->val_str(&str))) - plugins= resolve_engine_list("", 0, true); + plugins= resolve_engine_list(thd, "", 0, true, true); else - plugins= resolve_engine_list(res->ptr(), res->length(), true); + plugins= resolve_engine_list(thd, res->ptr(), res->length(), true, true); if (!plugins) return true; var->save_result.plugins= plugins; @@ -1586,7 +1597,7 @@ public: void do_update(plugin_ref **valptr, plugin_ref* newval) { plugin_ref *oldval= *valptr; - *valptr= newval; + *valptr= copy_engine_list(newval); free_engine_list(oldval); } bool session_update(THD *thd, set_var *var) @@ -1604,14 +1615,15 @@ public: void session_save_default(THD *thd, set_var *var) { plugin_ref* plugins= global_var(plugin_ref *); - var->save_result.plugins= plugins ? copy_engine_list(plugins) : 0; + var->save_result.plugins= plugins ? temp_copy_engine_list(thd, plugins) : 0; } plugin_ref *get_default(THD *thd) { char *default_value= *reinterpret_cast<char**>(option.def_value); if (!default_value) return 0; - return resolve_engine_list(default_value, strlen(default_value), false); + return resolve_engine_list(thd, default_value, strlen(default_value), + false, true); } void global_save_default(THD *thd, set_var *var) |