Bug#34953650: L seen in weekly-trunk mysql_component_sys_variable_imp…

…::register_variable

Several problems stacked up together:
1. The component initialization, when failing should clean up after
itself.

Fixed the validate_password component's init method to properly clean up
in case of failures.

2. The validate_password component had an REQUIRES_SERIVCE(registry).

While this is not wrong per se, it collided with the implicit
REQUIRES_SERVICE(registry) done by the BEGIN_COMPONENT_REQUIRES() macro
in that it was using the same placeholder global variable.
So now the same service handle was released twice on error or component
unload.
Fixed by removing the second REQUIRES_SERVICE(registry).

3. The dynamic loader is releasing the newly acquired service references
for the required services on initialization error. However after doing
that it was actually setting the service handle placeholder to NULL.
This is not wrong, but combined with problem #2 was causing a reference
to the registry service to be acquired twice, stored into the same
placeholder and then released just once, since after the first release
the placeholder was set to null and thus the second release is a no-op.

Fixed by not resetting the handle placeholder after releasing the
service reference.

4. The system variable registration service wouldn't release the
intermediate memory slots it was allocating on error.

Fixed by using std::unique_ptr to handle the proper releasing.

Change-Id: Ib2c7ae80736c591838af8c182fda1980be1e1f0e

components/libminchassis/dynamic_loader.cc

@@ -846,10 +846,8 @@ bool mysql_dynamic_loader_imp::load_do_resolve_dependencies(
   /* List of services acquired for component dependencies. */
   std::vector<my_h_service *> acquired_services;
   auto guard = create_scope_guard([&acquired_services]() {
-    for (my_h_service *service_storage : acquired_services) {
+    for (my_h_service *service_storage : acquired_services)
       mysql_registry_imp::release(*service_storage);
-      *service_storage = nullptr;
-    }
   });
 
   /* Acquire services to meet component dependencies. */

components/validate_password/validate_password_imp.cc

@@ -870,9 +870,27 @@ static mysql_service_status_t validate_password_init() {
   dictionary_words = new set_type();
   init_validate_password_psi_keys();
   mysql_rwlock_init(key_validate_password_LOCK_dict_file, &LOCK_dict_file);
-  if (log_service_init() || register_system_variables() ||
-      register_status_variables())
+  if (log_service_init()) {
+    delete dictionary_words;
+    dictionary_words = nullptr;
+    mysql_rwlock_destroy(&LOCK_dict_file);
     return true;
+  }
+  if (register_system_variables()) {
+    log_service_deinit();
+    delete dictionary_words;
+    dictionary_words = nullptr;
+    mysql_rwlock_destroy(&LOCK_dict_file);
+    return true;
+  }
+  if (register_status_variables()) {
+    unregister_system_variables();
+    log_service_deinit();
+    delete dictionary_words;
+    dictionary_words = nullptr;
+    mysql_rwlock_destroy(&LOCK_dict_file);
+    return true;
+  }
   read_dictionary_file();
   /* Check if validate_password_length needs readjustment */
   readjust_validate_password_length();
@@ -931,8 +949,7 @@ REQUIRES_MYSQL_RWLOCK_SERVICE_PLACEHOLDER;
    component load time and disposes off them at unload.
 */
 BEGIN_COMPONENT_REQUIRES(validate_password)
-REQUIRES_SERVICE(registry), REQUIRES_SERVICE(log_builtins),
-    REQUIRES_SERVICE(log_builtins_string),
+REQUIRES_SERVICE(log_builtins), REQUIRES_SERVICE(log_builtins_string),
     REQUIRES_SERVICE(mysql_string_factory), REQUIRES_SERVICE(mysql_string_case),
     REQUIRES_SERVICE(mysql_string_converter),
     REQUIRES_SERVICE(mysql_string_iterator),

sql/server_component/component_sys_var_service.cc

@@ -123,7 +123,7 @@ DEFINE_BOOL_METHOD(mysql_component_sys_variable_imp::register_variable,
   try {
     struct sys_var_chain chain = {nullptr, nullptr};
     sys_var *sysvar [[maybe_unused]];
-    char *com_sys_var_name, *optname;
+    char *com_sys_var_name, *optname, *com_sys_var_name_copy;
     int com_sys_var_len;
     SYS_VAR *opt = nullptr;
     my_option *opts = nullptr;
@@ -163,6 +163,7 @@ DEFINE_BOOL_METHOD(mysql_component_sys_variable_imp::register_variable,
     opts->arg_source = opts_arg_source;
     opts->arg_source->m_path_name[0] = 0;
     opts->arg_source->m_source = enum_variable_source::COMPILED;
+    std::unique_ptr<SYS_VAR, decltype(&my_free)> unique_opt(nullptr, &my_free);
 
     switch (flags & PLUGIN_VAR_WITH_SIGN_TYPEMASK) {
       case PLUGIN_VAR_BOOL:
@@ -298,6 +299,7 @@ DEFINE_BOOL_METHOD(mysql_component_sys_variable_imp::register_variable,
                component_name);
         goto end;
     }
+    unique_opt.reset(opt);
 
     plugin_opt_set_limits(opts, opt);
     opts->value = opts->u_max_value = *(uchar ***)(opt + 1);
@@ -379,14 +381,20 @@ DEFINE_BOOL_METHOD(mysql_component_sys_variable_imp::register_variable,
       }
     }
 
-    sysvar = reinterpret_cast<sys_var *>(new sys_var_pluginvar(
-        &chain, my_strdup(key_memory_comp_sys_var, com_sys_var_name, MYF(0)),
-        opt));
+    com_sys_var_name_copy =
+        my_strdup(key_memory_comp_sys_var, com_sys_var_name, MYF(0));
+    if (com_sys_var_name_copy == nullptr) {
+      LogErr(ERROR_LEVEL, ER_SYS_VAR_COMPONENT_OOM, var_name);
+      goto end;
+    }
+    sysvar = reinterpret_cast<sys_var *>(
+        new sys_var_pluginvar(&chain, com_sys_var_name_copy, opt));
 
     if (sysvar == nullptr) {
       LogErr(ERROR_LEVEL, ER_SYS_VAR_COMPONENT_OOM, var_name);
       goto end;
-    }
+    } else
+      unique_opt.release();
 
     sysvar->set_arg_source(opts->arg_source);
     sysvar->set_is_plugin(false);