WL#2936

  "Server variables for plugins"
  Post review fixes.

client/mysql.cc

@@ -743,7 +743,7 @@ static struct my_option my_long_options[] =
    "Number of seconds before connection timeout.",
    (gptr*) &opt_connect_timeout,
    (gptr*) &opt_connect_timeout, 0, GET_ULONG, REQUIRED_ARG, 0, 0, 3600*12, 0,
-   0, 1},
+   0, 0},
   {"max_allowed_packet", OPT_MAX_ALLOWED_PACKET,
    "Max packet length to send to, or receive from server",
    (gptr*) &opt_max_allowed_packet, (gptr*) &opt_max_allowed_packet, 0, GET_ULONG,

include/my_getopt.h

@@ -54,7 +54,7 @@ struct my_option
   longlong   max_value;                 /* Max allowed value */
   longlong   sub_size;                  /* Subtract this from given value */
   long       block_size;                /* Value should be a mult. of this */
-  long       app_type;                  /* To be used by an application */
+  void       *app_type;                 /* To be used by an application */
 };
 
 typedef my_bool (* my_get_one_option) (int, const struct my_option *, char * );

include/my_global.h

@@ -458,6 +458,14 @@ C_MODE_END
 */
 #include <assert.h>
 
+/* an assert that works at compile-time. only for constant expression */
+#define compile_time_assert(X)                                  \
+  do                                                            \
+  {                                                             \
+    char compile_time_assert[(X) ? 1 : -1]                      \
+                             __attribute__ ((unused));          \
+  } while(0)
+
 /* Go around some bugs in different OS and compilers */
 #if defined (HPUX11) && defined(_LARGEFILE_SOURCE)
 #define _LARGEFILE64_SOURCE

include/mysql/plugin.h

@@ -122,9 +122,43 @@ typedef int (*mysql_show_var_func)(MYSQL_THD, struct st_mysql_show_var*, char *)
 struct st_mysql_sys_var;
 struct st_mysql_value;
 
+/*
+  SYNOPSIS
+    (*mysql_var_check_func)()
+      thd               thread handle
+      var               dynamic variable being altered
+      save              pointer to temporary storage
+      value             user provided value
+  RETURN
+    0   user provided value is OK and the update func may be called.
+    any other value indicates error.
+  
+  This function should parse the user provided value and store in the
+  provided temporary storage any data as required by the update func.
+  There is sufficient space in the temporary storage to store a double.
+  Note that the update func may not be called if any other error occurs
+  so any memory allocated should be thread-local so that it may be freed
+  automatically at the end of the statement.
+*/
+
 typedef int (*mysql_var_check_func)(MYSQL_THD thd,
                                     struct st_mysql_sys_var *var,
                                     void *save, struct st_mysql_value *value);
+
+/*
+  SYNOPSIS
+    (*mysql_var_update_func)()
+      thd               thread handle
+      var               dynamic variable being altered
+      var_ptr           pointer to dynamic variable
+      save              pointer to temporary storage
+   RETURN
+     NONE
+   
+   This function should use the validated value stored in the temporary store
+   and persist it in the provided pointer to the dynamic variable.
+   For example, strings may require memory to be allocated.
+*/
 typedef void (*mysql_var_update_func)(MYSQL_THD thd,
                                       struct st_mysql_sys_var *var,
                                       void *var_ptr, void *save);
@@ -582,6 +616,10 @@ struct st_mysql_information_schema
   st_mysql_value struct for reading values from mysqld.
   Used by server variables framework to parse user-provided values.
   Will be used for arguments when implementing UDFs.
+
+  Note that val_str() returns a string in temporary memory
+  that will be freed at the end of statement. Copy the string
+  if you need it to persist.
 */
 
 #define MYSQL_VALUE_TYPE_STRING 0

mysys/array.c

@@ -57,9 +57,10 @@ my_bool init_dynamic_array2(DYNAMIC_ARRAY *array, uint element_size,
   }
 
   if (!init_alloc)
+  {
     init_alloc=alloc_increment;
-  else
     init_buffer= 0;
+  }
   array->elements=0;
   array->max_element=init_alloc;
   array->alloc_increment=alloc_increment;

mysys/typelib.c

@@ -20,6 +20,8 @@
 #include <m_ctype.h>
 
 
+static const char field_separator=',';
+
 /*
   Search after a string in a list of strings. Endspace in x is not compared.
 
@@ -31,6 +33,7 @@
 			If & 1 accept only whole names
 			If & 2 don't expand if half field
 			If & 4 allow #number# as type
+			If & 8 use ',' as string terminator
 
   NOTES
     If part, uniq field is found and full_name == 0 then x is expanded
@@ -60,16 +63,18 @@ int find_type(my_string x, TYPELIB *typelib, uint full_name)
   for (pos=0 ; (j=typelib->type_names[pos]) ; pos++)
   {
     for (i=x ; 
-    	*i && my_toupper(&my_charset_latin1,*i) == 
+    	*i && (!(full_name & 8) || *i != field_separator) &&
+        my_toupper(&my_charset_latin1,*i) == 
     		my_toupper(&my_charset_latin1,*j) ; i++, j++) ;
     if (! *j)
     {
       while (*i == ' ')
 	i++;					/* skip_end_space */
-      if (! *i)
+      if (! *i || ((full_name & 8) && *i == field_separator))
 	DBUG_RETURN(pos+1);
     }
-    if (! *i && (!*j || !(full_name & 1)))
+    if ((!*i && (!(full_name & 8) || *i != field_separator)) && 
+        (!*j || !(full_name & 1)))
     {
       find++;
       findpos=pos;
@@ -120,8 +125,6 @@ const char *get_type(TYPELIB *typelib, uint nr)
 }
 
 
-static const char field_separator=',';
-
 /*
   Create an integer value to represent the supplied comma-seperated
   string where each string in the TYPELIB denotes a bit position.
@@ -157,9 +160,7 @@ my_ulonglong find_typeset(my_string x, TYPELIB *lib, int *err)
     (*err)++;
     i= x;
     while (*x && *x != field_separator) x++;
-    if (*x)
-      *x++= 0;
-    if ((find= find_type(i, lib, 2) - 1) < 0)
+    if ((find= find_type(i, lib, 2 | 8) - 1) < 0)
       DBUG_RETURN(0);
     result|= (ULL(1) << find);
   }

sql/set_var.cc

@@ -2838,7 +2838,6 @@ int set_var_init()
 
 error:
   fprintf(stderr, "failed to initialize system variables");
-  pthread_mutex_unlock(&LOCK_global_system_variables);
   DBUG_RETURN(1);
 }
 

sql/sql_class.cc

@@ -167,18 +167,25 @@ Open_tables_state::Open_tables_state(ulong version_arg)
   reset_open_tables_state();
 }
 
+/*
+  The following functions form part of the C plugin API
+*/
+
+extern "C"
 int thd_in_lock_tables(const THD *thd)
 {
   return test(thd->in_lock_tables);
 }
 
 
+extern "C"
 int thd_tablespace_op(const THD *thd)
 {
   return test(thd->tablespace_op);
 }
 
 
+extern "C"
 const char *thd_proc_info(THD *thd, const char *info)
 {
   const char *old_info= thd->proc_info;
@@ -186,16 +193,19 @@ const char *thd_proc_info(THD *thd, const char *info)
   return old_info;
 }
 
+extern "C"
 void **thd_ha_data(const THD *thd, const struct handlerton *hton)
 {
   return (void **) thd->ha_data + hton->slot;
 }
 
+extern "C"
 long long thd_test_options(const THD *thd, long long test_options)
 {
   return thd->options & test_options;
 }
 
+extern "C"
 int thd_sql_command(const THD *thd)
 {
   return (int) thd->lex->sql_command;
@@ -216,6 +226,7 @@ int thd_sql_command(const THD *thd)
   RETURN VALUES
     pointer to string
 */
+extern "C"
 char *thd_security_context(THD *thd, char *buffer, int length,
                            int max_query_len)
 {
@@ -268,6 +279,7 @@ char *thd_security_context(THD *thd, char *buffer, int length,
   return thd->strmake(str.ptr(), str.length());
 }
 
+
 /*
   Pass nominal parameters to Statement constructor only to ensure that
   the destructor works OK in case of error. The main_mem_root will be

sql/sql_lex.cc

@@ -1745,6 +1745,9 @@ st_lex::st_lex()
   :result(0), yacc_yyss(0), yacc_yyvs(0),
    sql_command(SQLCOM_END)
 {
+  /* Check that plugins_static_buffer is declared immediately after plugins */
+  compile_time_assert((&plugins + 1) == (DYNAMIC_ARRAY*)plugins_static_buffer);
+
   my_init_dynamic_array2(&plugins, sizeof(plugin_ref),
                          plugins_static_buffer,
                          INITIAL_LEX_PLUGIN_LIST_SIZE, 

sql/sql_plugin.cc

@@ -161,9 +161,8 @@ class sys_var_pluginvar: public sys_var
   { TRASH(ptr_arg, size); }
 
   sys_var_pluginvar(const char *name_arg,
-                    struct st_plugin_int *plugin_arg,
                     struct st_mysql_sys_var *plugin_var_arg)
-    :sys_var(name_arg), plugin(plugin_arg), plugin_var(plugin_var_arg) {}
+    :sys_var(name_arg), plugin_var(plugin_var_arg) {}
   sys_var_pluginvar *cast_pluginvar() { return this; }
   bool is_readonly() const { return plugin_var->flags & PLUGIN_VAR_READONLY; }
   bool check_type(enum_var_type type)
@@ -631,11 +630,6 @@ static plugin_ref intern_plugin_lock(LEX *lex, plugin_ref rc CALLER_INFO_PROTO)
 #else
     if (!(plugin= (plugin_ref) my_malloc_ci(sizeof(pi), MYF(MY_WME))))
       DBUG_RETURN(NULL);
-    //if (0x4620a20L == (long) plugin)
-    if (0x4656b10L == (long) plugin)
-    {
-      DBUG_PRINT("debug",("trap"));
-    }
 
     *plugin= pi;
 #endif
@@ -656,6 +650,10 @@ plugin_ref plugin_lock(THD *thd, plugin_ref *ptr CALLER_INFO_PROTO)
   LEX *lex= NULL;
   plugin_ref rc;
   DBUG_ENTER("plugin_lock");
+  /*
+    thd->lex may point to a nested LEX or a stored procedure LEX.
+    main_lex is tightly coupled to the thread.
+  */
   if (thd)
     lex= !thd->lex ? &thd->main_lex : thd->lex;
   pthread_mutex_lock(&LOCK_plugin);
@@ -767,11 +765,9 @@ static bool plugin_add(MEM_ROOT *tmp_root,
             DBUG_RETURN(FALSE);
           }
           tmp_plugin_ptr->state= PLUGIN_IS_FREED;
-          goto err;
         }
         mysql_del_sys_var_chain(tmp.system_vars);
-        plugin_dl_del(dl);
-        DBUG_RETURN(TRUE);
+        goto err;
       }
       /* plugin was disabled */
       plugin_dl_del(dl);
@@ -939,7 +935,11 @@ static void intern_plugin_unlock(LEX *lex, plugin_ref plugin)
                      pi->name.str, pi->ref_count));
   if (lex)
   {
-    /* remove one instance of this plugin from the use list */
+    /*
+      Remove one instance of this plugin from the use list.
+      We are searching backwards so that plugins locked last
+      could be unlocked faster - optimizing for LIFO semantics.
+    */
     for (i= lex->plugins.elements - 1; i >= 0; i--)
       if (plugin == *dynamic_element(&lex->plugins, i, plugin_ref*))
       {
@@ -1283,6 +1283,7 @@ bool plugin_register_builtin(THD *thd, struct st_mysql_plugin *plugin)
   bzero(&tmp, sizeof(tmp));
   tmp.plugin= plugin;
 
+  pthread_mutex_lock(&LOCK_plugin);
   rw_wrlock(&LOCK_system_variables_hash);
 
   if (test_plugin_options(thd->mem_root, &tmp, &dummy_argc, NULL, true))
@@ -1293,6 +1294,7 @@ bool plugin_register_builtin(THD *thd, struct st_mysql_plugin *plugin)
 
 end:
   rw_unlock(&LOCK_system_variables_hash);
+  pthread_mutex_unlock(&LOCK_plugin);
 
   DBUG_RETURN(result);;
 }
@@ -1445,6 +1447,11 @@ void plugin_shutdown(void)
   {
     pthread_mutex_lock(&LOCK_plugin);
 
+    /*
+      release any plugin references held but don't yet free
+      memory for dynamic variables as some plugins may still
+      want to reference their global variables.
+    */
     cleanup_variables(NULL, &global_system_variables, false);
     cleanup_variables(NULL, &max_system_variables, false);
 
@@ -1471,6 +1478,9 @@ void plugin_shutdown(void)
       reap_plugins();
     }
 
+    if (count > 0)
+      sql_print_warning("Forcing shutdown of %d plugins", count);
+    
     plugins= (struct st_plugin_int **) my_alloca(sizeof(void*) * (count+1));
 
     /*
@@ -1496,7 +1506,6 @@ void plugin_shutdown(void)
         plugin_deinitialize(plugins[i], false);
       }
 
-    pthread_mutex_lock(&LOCK_plugin);
     
     /*
       We defer checking ref_counts until after all plugins are deinitialized
@@ -1511,15 +1520,24 @@ void plugin_shutdown(void)
       if (plugins[i]->state & PLUGIN_IS_UNINITIALIZED)
         plugin_del(plugins[i]);
 
+    /*
+      Now we can deallocate all memory.
+    */
+#if defined(SAFE_MUTEX) && !defined(DBUG_OFF)
+    /* neccessary to avoid safe_mutex_assert_owner() trap */
+    pthread_mutex_lock(&LOCK_plugin);
+#endif
     cleanup_variables(NULL, &global_system_variables, true);
     cleanup_variables(NULL, &max_system_variables, true);
+#if defined(SAFE_MUTEX) && !defined(DBUG_OFF)
+    pthread_mutex_unlock(&LOCK_plugin);
+#endif
 
     initialized= 0;
-    pthread_mutex_unlock(&LOCK_plugin);
     pthread_mutex_destroy(&LOCK_plugin);
 
-  }
     my_afree(plugins);
+  }
 
   /* Dispose of the memory */
   
@@ -2007,12 +2025,13 @@ sys_var *find_sys_var(THD *thd, const char *str, uint length)
   plugin_ref plugin;
   DBUG_ENTER("find_sys_var");
   
+  pthread_mutex_lock(&LOCK_plugin);
   rw_rdlock(&LOCK_system_variables_hash);
   if ((var= intern_find_sys_var(str, length, false)) &&
       (pi= var->cast_pluginvar()))
   {
+    rw_unlock(&LOCK_system_variables_hash);
     LEX *lex= thd ? ( !thd->lex ? &thd->main_lex : thd->lex ) : NULL;
-    pthread_mutex_lock(&LOCK_plugin);
     if (!(plugin= my_intern_plugin_lock(lex, plugin_int_to_ref(pi->plugin))))
       var= NULL; /* failed to lock it, it must be uninstalling */
     else
@@ -2022,9 +2041,10 @@ sys_var *find_sys_var(THD *thd, const char *str, uint length)
       var= NULL;
       intern_plugin_unlock(lex, plugin);
     }
-    pthread_mutex_unlock(&LOCK_plugin);
   }
+  else
     rw_unlock(&LOCK_system_variables_hash);
+  pthread_mutex_unlock(&LOCK_plugin);
   
   /*
     If the variable exists but the plugin it is associated with is not ready
@@ -2183,7 +2203,7 @@ static byte *intern_sys_var_ptr(THD* thd, int offset, bool global_lock)
     return (byte*) global_system_variables.dynamic_variables_ptr + offset;
   
   /*
-    dynamic_variables_size points to the largest valid offset
+    dynamic_variables_head points to the largest valid offset
   */
   if (!thd->variables.dynamic_variables_ptr ||
       (uint)offset > thd->variables.dynamic_variables_head)
@@ -2208,8 +2228,6 @@ static byte *intern_sys_var_ptr(THD* thd, int offset, bool global_lock)
              thd->variables.dynamic_variables_size,
            global_system_variables.dynamic_variables_size -
              thd->variables.dynamic_variables_size);
-    if (global_lock)
-      pthread_mutex_unlock(&LOCK_global_system_variables);
 
     /*
       now we need to iterate through any newly copied 'defaults'
@@ -2232,19 +2250,17 @@ static byte *intern_sys_var_ptr(THD* thd, int offset, bool global_lock)
       if ((pi->plugin_var->flags & PLUGIN_VAR_TYPEMASK) == PLUGIN_VAR_STR &&
           pi->plugin_var->flags & PLUGIN_VAR_MEMALLOC)
       {
-         char **pp;
-         if (global_lock)
-           pthread_mutex_lock(&LOCK_global_system_variables);
-         pp= (char**) (thd->variables.dynamic_variables_ptr + 
+         char **pp= (char**) (thd->variables.dynamic_variables_ptr + 
                              *(int*)(pi->plugin_var + 1));         
          if ((*pp= *(char**) (global_system_variables.dynamic_variables_ptr + 
                              *(int*)(pi->plugin_var + 1))))
            *pp= my_strdup(*pp, MYF(MY_WME|MY_FAE));
-         if (global_lock)
-           pthread_mutex_unlock(&LOCK_global_system_variables);
       }
     }       
 
+    if (global_lock)
+      pthread_mutex_unlock(&LOCK_global_system_variables);
+
     thd->variables.dynamic_variables_version= 
            global_system_variables.dynamic_variables_version;
     thd->variables.dynamic_variables_head= 
@@ -2817,7 +2833,7 @@ static int construct_options(MEM_ROOT *mem_root,
 
     options->name= optname;
     options->comment= opt->comment;
-    options->app_type= (long) opt;
+    options->app_type= opt;
     options->id= (options-1)->id + 1;
     
     if (opt->flags & PLUGIN_VAR_THDLOCAL)
@@ -2950,7 +2966,7 @@ static int test_plugin_options(MEM_ROOT *tmp_root, struct st_plugin_int *tmp,
         continue;
       
       if ((var= find_bookmark(tmp->plugin->name, o->name, o->flags)))
-        v= new (mem_root) sys_var_pluginvar(var->name + 1, tmp, o);
+        v= new (mem_root) sys_var_pluginvar(var->name + 1, o);
       else
       {
         len= strlen(tmp->plugin->name) + strlen(o->name) + 2;
@@ -2962,10 +2978,15 @@ static int test_plugin_options(MEM_ROOT *tmp_root, struct st_plugin_int *tmp,
           if (*p == '-')
             *p= '_';
 
-        v= new (mem_root) sys_var_pluginvar(varname, tmp, o);
+        v= new (mem_root) sys_var_pluginvar(varname, o);
       }
       DBUG_ASSERT(v); /* check that an object was actually constructed */
 
+      /*
+        Add to the chain of variables.
+        Done like this for easier debugging so that the 
+        pointer to v is not lost on optimized builds.
+      */
       v->chain_sys_var(&chain);
     }
     if (chain.first)

sql/sql_plugin.h

@@ -79,6 +79,11 @@ struct st_plugin_int
   sys_var *system_vars;         /* server variables for this plugin */
 };
 
+
+/*
+  See intern_plugin_lock() for the explanation for the
+  conditionally defined plugin_ref type
+*/
 #ifdef DBUG_OFF
 typedef struct st_plugin_int *plugin_ref;
 #define plugin_decl(pi) ((pi)->plugin)
@@ -86,6 +91,7 @@ typedef struct st_plugin_int *plugin_ref;
 #define plugin_data(pi,cast) ((cast)((pi)->data))
 #define plugin_name(pi) (&((pi)->name))
 #define plugin_state(pi) ((pi)->state)
+#define plugin_equals(p1,p2) ((p1) == (p2))
 #else
 typedef struct st_plugin_int **plugin_ref;
 #define plugin_decl(pi) ((pi)[0]->plugin)
@@ -93,6 +99,7 @@ typedef struct st_plugin_int **plugin_ref;
 #define plugin_data(pi,cast) ((cast)((pi)[0]->data))
 #define plugin_name(pi) (&((pi)[0]->name))
 #define plugin_state(pi) ((pi)[0]->state)
+#define plugin_equals(p1,p2) ((p1) && (p2) && (p1)[0] == (p2)[0])
 #endif
 
 typedef int (*plugin_type_init)(struct st_plugin_int *);

sql/table.cc

@@ -425,7 +425,6 @@ static int open_binary_frm(THD *thd, TABLE_SHARE *share, uchar *head,
   Field  **field_ptr, *reg_field;
   const char **interval_array;
   enum legacy_db_type legacy_db_type;
-  handlerton *hton;
   my_bitmap_map *bitmaps;
   DBUG_ENTER("open_binary_frm");
 
@@ -456,11 +455,15 @@ static int open_binary_frm(THD *thd, TABLE_SHARE *share, uchar *head,
   DBUG_PRINT("info", ("default_part_db_type = %u", head[61]));
 #endif
   legacy_db_type= (enum legacy_db_type) (uint) *(head+3);
-  if ((hton= ha_checktype(thd, legacy_db_type, 0, 0)) != share->db_type())
-  {
-    plugin_unlock(NULL, share->db_plugin);
-    share->db_plugin= ha_lock_engine(NULL, hton);
-  }
+  DBUG_ASSERT(share->db_plugin == NULL);
+  /*
+    if the storage engine is dynamic, no point in resolving it by its
+    dynamically allocated legacy_db_type. We will resolve it later by name.
+  */
+  if (legacy_db_type > DB_TYPE_UNKNOWN && 
+      legacy_db_type < DB_TYPE_FIRST_DYNAMIC)
+    share->db_plugin= ha_lock_engine(NULL, 
+                                     ha_checktype(thd, legacy_db_type, 0, 0));
   share->db_create_options= db_create_options= uint2korr(head+30);
   share->db_options_in_use= share->db_create_options;
   share->mysql_version= uint4korr(head+51);
@@ -620,8 +623,17 @@ static int open_binary_frm(THD *thd, TABLE_SHARE *share, uchar *head,
       uint str_db_type_length= uint2korr(next_chunk);
       LEX_STRING name= { next_chunk + 2, str_db_type_length };
       plugin_ref tmp_plugin= ha_resolve_by_name(thd, &name);
-      if (tmp_plugin != NULL)
+      if (tmp_plugin != NULL && !plugin_equals(tmp_plugin, share->db_plugin))
       {
+        if (legacy_db_type > DB_TYPE_UNKNOWN &&
+            legacy_db_type < DB_TYPE_FIRST_DYNAMIC &&
+            legacy_db_type != ha_legacy_type(
+                plugin_data(tmp_plugin, handlerton *)))
+        {
+          /* bad file, legacy_db_type did not match the name */
+          my_free(buff, MYF(0));
+          goto err;
+        }
         /*
           tmp_plugin is locked with a local lock.
           we unlock the old value of share->db_plugin before

storage/ndb/src/mgmsrv/InitConfigFileParser.cpp

@@ -612,10 +612,11 @@ static
 my_bool 
 parse_mycnf_opt(int, const struct my_option * opt, char * value)
 {
+  long *app_type= (long*) &opt->app_type;
   if(opt->comment)
-    ((struct my_option *)opt)->app_type++;
+    (*app_type)++;
   else
-    ((struct my_option *)opt)->app_type = order++;
+    *app_type = order++;
   return 0;
 }
 
@@ -948,22 +949,6 @@ InitConfigFileParser::parse_mycnf()
 
 template class Vector<struct my_option>;
 
-#if 0
-struct my_option
-{
-  const char *name;                     /* Name of the option */
-  int        id;                        /* unique id or short option */
-  const char *comment;                  /* option comment, for autom. --help */
-  gptr       *value;                    /* The variable value */
-  gptr       *u_max_value;              /* The user def. max variable value */
-  const char **str_values;              /* Pointer to possible values */
-  ulong     var_type;
-  enum get_opt_arg_type arg_type;
-  longlong   def_value;                 /* Default value */
-  longlong   min_value;                 /* Min allowed value */
-  longlong   max_value;                 /* Max allowed value */
-  longlong   sub_size;                  /* Subtract this from given value */
-  long       block_size;                /* Value should be a mult. of this */
-  int        app_type;                  /* To be used by an application */
-};
-#endif
+/*
+  See include/my_getopt.h for the declaration of struct my_option
+*/