Re: [Maria-developers] (MDEV-4403) Attempting to use cassandra storage engine causes "service 'my_snprintf_service' interface version mismatch"

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 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 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 */