Hi Sergei, well, I guess there is no alternative way to workaround it. :( Ok to push.
Regards, Sergei On Mon, Dec 09, 2013 at 01:51:51PM +0100, Sergei Golubchik wrote: > Hi. > > I don't see my commit emails on the list, so here you are: > > first patch is the bug fix itself. > second - cleanup of the existing code after the bug fix. > > Regards, > Sergei > > ------------------------------------------------------------ > revno: 3927 > fixes bug: https://mariadb.atlassian.net/browse/MDEV-4403 > committer: Sergei Golubchik <[email protected]> > branch nick: 10.0 > timestamp: Mon 2013-12-09 12:39:13 +0100 > message: > MDEV-4403 Attempting to use cassandra storage engine causes "service > 'my_snprintf_service' interface version mismatch" > > When a DSO is loaded we rewrite service pointers to point to the actual > service structures. > But when a DSO is unloaded, we have to restore their original values, in > case this DSO > wasn't removed from memory on dlclose() and is later loaded again. > added: > mysql-test/suite/plugins/r/cassandra_reinstall.result > mysql-test/suite/plugins/t/cassandra_reinstall.test > modified: > sql/sql_plugin.cc > sql/sql_plugin.h > diff: > === added file 'mysql-test/suite/plugins/r/cassandra_reinstall.result' > --- mysql-test/suite/plugins/r/cassandra_reinstall.result 1970-01-01 > 00:00:00 +0000 > +++ mysql-test/suite/plugins/r/cassandra_reinstall.result 2013-12-09 > 11:39:13 +0000 > @@ -0,0 +1,14 @@ > +install soname 'ha_cassandra'; > +select plugin_name,plugin_status,plugin_library from > information_schema.plugins where plugin_name = 'cassandra'; > +plugin_name plugin_status plugin_library > +CASSANDRA ACTIVE ha_cassandra.so > +uninstall plugin cassandra; > +select plugin_name,plugin_status,plugin_library from > information_schema.plugins where plugin_name = 'cassandra'; > +plugin_name plugin_status plugin_library > +install soname 'ha_cassandra'; > +select plugin_name,plugin_status,plugin_library from > information_schema.plugins where plugin_name = 'cassandra'; > +plugin_name plugin_status plugin_library > +CASSANDRA ACTIVE ha_cassandra.so > +uninstall plugin cassandra; > +select plugin_name,plugin_status,plugin_library from > information_schema.plugins where plugin_name = 'cassandra'; > +plugin_name plugin_status plugin_library > > === added file 'mysql-test/suite/plugins/t/cassandra_reinstall.test' > --- mysql-test/suite/plugins/t/cassandra_reinstall.test 1970-01-01 > 00:00:00 +0000 > +++ mysql-test/suite/plugins/t/cassandra_reinstall.test 2013-12-09 > 11:39:13 +0000 > @@ -0,0 +1,16 @@ > +# > +# MDEV-4403 Attempting to use cassandra storage engine causes "service > 'my_snprintf_service' interface version mismatch" > +# > +if (!$HA_CASSANDRA_SO) { > + skip No Cassandra engine; > +} > + > +install soname 'ha_cassandra'; > +select plugin_name,plugin_status,plugin_library from > information_schema.plugins where plugin_name = 'cassandra'; > +uninstall plugin cassandra; > +select plugin_name,plugin_status,plugin_library from > information_schema.plugins where plugin_name = 'cassandra'; > +install soname 'ha_cassandra'; > +select plugin_name,plugin_status,plugin_library from > information_schema.plugins where plugin_name = 'cassandra'; > +uninstall plugin cassandra; > +select plugin_name,plugin_status,plugin_library from > information_schema.plugins where plugin_name = 'cassandra'; > + > > === modified file 'sql/sql_plugin.cc' > --- sql/sql_plugin.cc 2013-11-13 22:03:48 +0000 > +++ sql/sql_plugin.cc 2013-12-09 11:39:13 +0000 > @@ -309,6 +309,7 @@ static void unlock_variables(THD *thd, s > static void cleanup_variables(THD *thd, struct system_variables *vars); > static void plugin_vars_free_values(sys_var *vars); > static void restore_pluginvar_names(sys_var *first); > +static void restore_ptr_backup(uint n, st_ptr_backup *backup); > static plugin_ref intern_plugin_lock(LEX *lex, plugin_ref plugin); > static void intern_plugin_unlock(LEX *lex, plugin_ref plugin); > static void reap_plugins(void); > @@ -473,9 +474,16 @@ static st_plugin_dl *plugin_dl_insert_or > #endif /* HAVE_DLOPEN */ > > > -static inline void free_plugin_mem(struct st_plugin_dl *p) > +static void free_plugin_mem(struct st_plugin_dl *p) > { > #ifdef HAVE_DLOPEN > + if (p->ptr_backup) > + { > + DBUG_ASSERT(p->nbackups); > + DBUG_ASSERT(p->handle); > + restore_ptr_backup(p->nbackups, p->ptr_backup); > + my_free(p->ptr_backup); > + } > if (p->handle) > dlclose(p->handle); > #endif > @@ -706,6 +714,7 @@ static st_plugin_dl *plugin_dl_add(const > uint plugin_dir_len, dummy_errors, dlpathlen, i; > struct st_plugin_dl *tmp= 0, plugin_dl; > void *sym; > + st_ptr_backup tmp_backup[array_elements(list_of_services)]; > DBUG_ENTER("plugin_dl_add"); > DBUG_PRINT("enter", ("dl->str: '%s', dl->length: %d", > dl->str, (int) dl->length)); > @@ -772,7 +781,8 @@ static st_plugin_dl *plugin_dl_add(const > { > if ((sym= dlsym(plugin_dl.handle, list_of_services[i].name))) > { > - uint ver= (uint)(intptr)*(void**)sym; > + void **ptr= (void **)sym; > + uint ver= (uint)(intptr)*ptr; > if (ver > list_of_services[i].version || > (ver >> 8) < (list_of_services[i].version >> 8)) > { > @@ -783,8 +793,22 @@ static st_plugin_dl *plugin_dl_add(const > report_error(report, ER_CANT_OPEN_LIBRARY, dlpath, ENOEXEC, buf); > goto ret; > } > - *(void**)sym= list_of_services[i].service; > + tmp_backup[plugin_dl.nbackups++].save(ptr); > + *ptr= list_of_services[i].service; > + } > + } > + > + if (plugin_dl.nbackups) > + { > + size_t bytes= plugin_dl.nbackups * sizeof(plugin_dl.ptr_backup[0]); > + plugin_dl.ptr_backup= (st_ptr_backup *)my_malloc(bytes, MYF(0)); > + if (!plugin_dl.ptr_backup) > + { > + restore_ptr_backup(plugin_dl.nbackups, tmp_backup); > + report_error(report, ER_OUTOFMEMORY, bytes); > + goto ret; > } > + memcpy(plugin_dl.ptr_backup, tmp_backup, bytes); > } > > /* Duplicate and convert dll name */ > @@ -4017,3 +4041,38 @@ sys_var *find_plugin_sysvar(st_plugin_in > return 0; > } > > +/* > + On dlclose() we need to restore values of all symbols that we've modified > in > + the DSO. The reason is - the DSO might not actually be unloaded, so on the > + next dlopen() these symbols will have old values, they won't be > + reinitialized. > + > + Perhaps, there can be many reason, why a DSO won't be unloaded. Strictly > + speaking, it's implementation defined whether to unload an unused DSO or to > + keep it in memory. > + > + In particular, this happens for some plugins: In 2009 a new ELF stub was > + introduced, see Ulrich Drepper's email "Unique symbols for C++" > + http://www.redhat.com/archives/posix-c++-wg/2009-August/msg00002.html > + > + DSO that has objects with this stub (STB_GNU_UNIQUE) cannot be unloaded > + (this is mentioned in the email, see the url above). > + > + These "unique" objects are, for example, static variables in templates, > + in inline functions, in classes. So any DSO that uses them can > + only be loaded once. And because Boost has them, any DSO that uses Boost > + almost certainly cannot be unloaded. > + > + To know whether a particular DSO has these objects, one can use > + > + readelf -s /path/to/plugin.so|grep UNIQUE > + > + There's nothing we can do about it, but to reset the DSO to its initial > + state before dlclose(). > +*/ > +static void restore_ptr_backup(uint n, st_ptr_backup *backup) > +{ > + while (n--) > + (backup++)->restore(); > +} > + > > === modified file 'sql/sql_plugin.h' > --- sql/sql_plugin.h 2013-06-03 07:57:34 +0000 > +++ sql/sql_plugin.h 2013-12-09 11:39:13 +0000 > @@ -81,15 +81,24 @@ typedef struct st_mysql_show_var SHOW_VA > > /* A handle for the dynamic library containing a plugin or plugins. */ > > +struct st_ptr_backup { > + void **ptr; > + void *value; > + void save(void **p) { ptr= p; value= *p; } > + void restore() { *ptr= value; } > +}; > + > struct st_plugin_dl > { > LEX_STRING dl; > void *handle; > struct st_maria_plugin *plugins; > + st_ptr_backup *ptr_backup; > + uint nbackups; > + uint ref_count; /* number of plugins loaded from the library */ > int mysqlversion; > int mariaversion; > bool allocated; > - uint ref_count; /* number of plugins loaded from the library */ > }; > > /* A handle of a plugin */ > ------------------------------------------------------------ > revno: 3928 > committer: Sergei Golubchik <[email protected]> > branch nick: 10.0 > timestamp: Mon 2013-12-09 12:39:19 +0100 > message: > remove sys_var specific restore_pluginvar_names() function, > use generic restore_ptr_backup() approach > modified: > sql/sql_plugin.cc > sql/sql_plugin.h > diff: > === modified file 'sql/sql_plugin.cc' > --- sql/sql_plugin.cc 2013-12-09 11:39:13 +0000 > +++ sql/sql_plugin.cc 2013-12-09 11:39:19 +0000 > @@ -257,15 +257,6 @@ class sys_var_pluginvar: public sys_var > public: > struct st_plugin_int *plugin; > struct st_mysql_sys_var *plugin_var; > - /** > - variable name from whatever is hard-coded in the plugin source > - and doesn't have pluginname- prefix is replaced by an allocated name > - with a plugin prefix. When plugin is uninstalled we need to restore the > - pointer to point to the hard-coded value, because plugin may be > - installed/uninstalled many times without reloading the shared object. > - */ > - const char *orig_pluginvar_name; > - > static void *operator new(size_t size, MEM_ROOT *mem_root) > { return (void*) alloc_root(mem_root, size); } > static void operator delete(void *ptr_arg,size_t size) > @@ -278,7 +269,7 @@ class sys_var_pluginvar: public sys_var > (plugin_var_arg->flags & PLUGIN_VAR_READONLY ? READONLY : 0), > 0, -1, NO_ARG, pluginvar_show_type(plugin_var_arg), 0, 0, > VARIABLE_NOT_IN_BINLOG, NULL, NULL, NULL), > - plugin_var(plugin_var_arg), orig_pluginvar_name(plugin_var_arg->name) > + plugin_var(plugin_var_arg) > { plugin_var->name= name_arg; } > sys_var_pluginvar *cast_pluginvar() { return this; } > bool check_update_type(Item_result type); > @@ -308,7 +299,6 @@ static bool register_builtin(struct st_m > static void unlock_variables(THD *thd, struct system_variables *vars); > static void cleanup_variables(THD *thd, struct system_variables *vars); > static void plugin_vars_free_values(sys_var *vars); > -static void restore_pluginvar_names(sys_var *first); > static void restore_ptr_backup(uint n, st_ptr_backup *backup); > static plugin_ref intern_plugin_lock(LEX *lex, plugin_ref plugin); > static void intern_plugin_unlock(LEX *lex, plugin_ref plugin); > @@ -1122,7 +1112,6 @@ static bool plugin_add(MEM_ROOT *tmp_roo > if (!(tmp_plugin_ptr= plugin_insert_or_reuse(&tmp))) > { > mysql_del_sys_var_chain(tmp.system_vars); > - restore_pluginvar_names(tmp.system_vars); > goto err; > } > plugin_array_version++; > @@ -1139,6 +1128,8 @@ static bool plugin_add(MEM_ROOT *tmp_roo > > err: > errs++; > + if (tmp.nbackups) > + restore_ptr_backup(tmp.nbackups, tmp.ptr_backup); > if (name->str) > break; > } > @@ -1217,7 +1208,7 @@ static void plugin_del(struct st_plugin_ > mysql_rwlock_wrlock(&LOCK_system_variables_hash); > mysql_del_sys_var_chain(plugin->system_vars); > mysql_rwlock_unlock(&LOCK_system_variables_hash); > - restore_pluginvar_names(plugin->system_vars); > + restore_ptr_backup(plugin->nbackups, plugin->ptr_backup); > plugin_vars_free_values(plugin->system_vars); > my_hash_delete(&plugin_hash[plugin->plugin->type], (uchar*)plugin); > plugin_dl_del(plugin->plugin_dl); > @@ -2938,16 +2929,6 @@ static st_bookmark *register_var(const c > return result; > } > > -static void restore_pluginvar_names(sys_var *first) > -{ > - for (sys_var *var= first; var; var= var->next) > - { > - sys_var_pluginvar *pv= var->cast_pluginvar(); > - pv->plugin_var->name= pv->orig_pluginvar_name; > - } > -} > - > - > /* > returns a pointer to the memory which holds the thd-local variable or > a pointer to the global variable if thd==null. > @@ -3823,7 +3804,7 @@ static my_option *construct_help_options > to get the correct (not double-prefixed) help text. > We won't need @@sysvars anymore and don't care about their proper names. > */ > - restore_pluginvar_names(p->system_vars); > + restore_ptr_backup(p->nbackups, p->ptr_backup); > > if (construct_options(mem_root, p, opts)) > DBUG_RETURN(NULL); > @@ -3868,6 +3849,7 @@ static int test_plugin_options(MEM_ROOT > sys_var *v __attribute__((unused)); > struct st_bookmark *var; > uint len, count= EXTRA_OPTIONS; > + st_ptr_backup *tmp_backup= 0; > DBUG_ENTER("test_plugin_options"); > DBUG_ASSERT(tmp->plugin && tmp->name.str); > > @@ -3940,59 +3922,85 @@ static int test_plugin_options(MEM_ROOT > plugin_name= tmp->name; > > error= 1; > - for (opt= tmp->plugin->system_vars; opt && *opt; opt++) > + > + if (tmp->plugin->system_vars) > { > - st_mysql_sys_var *o= *opt; > + for (len=0, opt= tmp->plugin->system_vars; *opt; len++, opt++) /* no-op > */; > + tmp_backup= (st_ptr_backup *)my_alloca(len * sizeof(tmp_backup[0])); > + DBUG_ASSERT(tmp->nbackups == 0); > + DBUG_ASSERT(tmp->ptr_backup == 0); > > - /* > - PLUGIN_VAR_STR command-line options without PLUGIN_VAR_MEMALLOC, point > - directly to values in the argv[] array. For plugins started at the > - server startup, argv[] array is allocated with load_defaults(), and > - freed when the server is shut down. But for plugins loaded with > - INSTALL PLUGIN, the memory allocated with load_defaults() is freed with > - freed() at the end of mysql_install_plugin(). Which means we cannot > - allow any pointers into that area. > - Thus, for all plugins loaded after the server was started, > - we copy string values to a plugin's memroot. > - */ > - if (mysqld_server_started && > - ((o->flags & (PLUGIN_VAR_STR | PLUGIN_VAR_NOCMDOPT | > - PLUGIN_VAR_MEMALLOC)) == PLUGIN_VAR_STR)) > + for (opt= tmp->plugin->system_vars; *opt; opt++) > { > - sysvar_str_t* str= (sysvar_str_t *)o; > - if (*str->value) > - *str->value= strdup_root(mem_root, *str->value); > - } > + st_mysql_sys_var *o= *opt; > > - if (o->flags & PLUGIN_VAR_NOSYSVAR) > - continue; > - if ((var= find_bookmark(plugin_name.str, o->name, o->flags))) > - v= new (mem_root) sys_var_pluginvar(&chain, var->key + 1, o); > - else > + /* > + PLUGIN_VAR_STR command-line options without PLUGIN_VAR_MEMALLOC, > point > + directly to values in the argv[] array. For plugins started at the > + server startup, argv[] array is allocated with load_defaults(), and > + freed when the server is shut down. But for plugins loaded with > + INSTALL PLUGIN, the memory allocated with load_defaults() is freed > with > + freed() at the end of mysql_install_plugin(). Which means we cannot > + allow any pointers into that area. > + Thus, for all plugins loaded after the server was started, > + we copy string values to a plugin's memroot. > + */ > + if (mysqld_server_started && > + ((o->flags & (PLUGIN_VAR_STR | PLUGIN_VAR_NOCMDOPT | > + PLUGIN_VAR_MEMALLOC)) == PLUGIN_VAR_STR)) > + { > + sysvar_str_t* str= (sysvar_str_t *)o; > + if (*str->value) > + *str->value= strdup_root(mem_root, *str->value); > + } > + > + if (o->flags & PLUGIN_VAR_NOSYSVAR) > + continue; > + tmp_backup[tmp->nbackups++].save(&o->name); > + if ((var= find_bookmark(plugin_name.str, o->name, o->flags))) > + v= new (mem_root) sys_var_pluginvar(&chain, var->key + 1, o); > + else > + { > + len= plugin_name.length + strlen(o->name) + 2; > + varname= (char*) alloc_root(mem_root, len); > + strxmov(varname, plugin_name.str, "-", o->name, NullS); > + my_casedn_str(&my_charset_latin1, varname); > + convert_dash_to_underscore(varname, len-1); > + v= new (mem_root) sys_var_pluginvar(&chain, varname, o); > + } > + DBUG_ASSERT(v); /* check that an object was actually constructed */ > + } /* end for */ > + > + if (tmp->nbackups) > { > - len= plugin_name.length + strlen(o->name) + 2; > - varname= (char*) alloc_root(mem_root, len); > - strxmov(varname, plugin_name.str, "-", o->name, NullS); > - my_casedn_str(&my_charset_latin1, varname); > - convert_dash_to_underscore(varname, len-1); > - v= new (mem_root) sys_var_pluginvar(&chain, varname, o); > - } > - DBUG_ASSERT(v); /* check that an object was actually constructed */ > - } /* end for */ > - if (chain.first) > - { > - chain.last->next = NULL; > - if (mysql_add_sys_var_chain(chain.first)) > + size_t bytes= tmp->nbackups * sizeof(tmp->ptr_backup[0]); > + tmp->ptr_backup= (st_ptr_backup *)alloc_root(mem_root, bytes); > + if (!tmp->ptr_backup) > + { > + restore_ptr_backup(tmp->nbackups, tmp_backup); > + goto err; > + } > + memcpy(tmp->ptr_backup, tmp_backup, bytes); > + } > + > + if (chain.first) > { > - sql_print_error("Plugin '%s' has conflicting system variables", > - tmp->name.str); > - goto err; > + chain.last->next = NULL; > + if (mysql_add_sys_var_chain(chain.first)) > + { > + sql_print_error("Plugin '%s' has conflicting system variables", > + tmp->name.str); > + goto err; > + } > + tmp->system_vars= chain.first; > } > - tmp->system_vars= chain.first; > } > + > DBUG_RETURN(0); > > err: > + if (tmp_backup) > + my_afree(tmp_backup); > if (opts) > my_cleanup_options(opts); > DBUG_RETURN(error); > > === modified file 'sql/sql_plugin.h' > --- sql/sql_plugin.h 2013-12-09 11:39:13 +0000 > +++ sql/sql_plugin.h 2013-12-09 11:39:19 +0000 > @@ -85,6 +85,7 @@ struct st_ptr_backup { > void **ptr; > void *value; > void save(void **p) { ptr= p; value= *p; } > + void save(const char **p) { save((void**)p); } > void restore() { *ptr= value; } > }; > > @@ -108,6 +109,8 @@ struct st_plugin_int > LEX_STRING name; > struct st_maria_plugin *plugin; > struct st_plugin_dl *plugin_dl; > + st_ptr_backup *ptr_backup; > + uint nbackups; > uint state; > uint ref_count; /* number of threads using the plugin */ > uint locks_total; /* how many times the plugin was locked */ _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
