Bug#14771682 PERFORMANCE SCHEMA LEAKS MEMORY ON SHUTDOWN

Currently, memory allocated for the performance schema is never freed,
which causes valgrind warnings.

To resolve completely the issue, two changes are needed:

1) 
Resolve bug#56666 Race condition between the server main thread and the
kill server thread,
which is the scope of WL#6407.

2)
Fix the function cleanup_performance_schema() itself to free all memory used
by the performance schema.

This fix implements the second part only.

Code is commented out using #ifdef HAVE_WL6407,
while waiting for part 1).

The function cleanup_performance_schema() has been tested (with #define
HAVE_WL6407), which exposed some leaks, which have been all fixed.

Executing unit tests under valgrind also exposed additional issues,
all fixed in the unit tests.

Author: marcalff

storage/perfschema/pfs_account.cc

@@ -125,14 +125,19 @@ int init_account(const PFS_global_param *param)
   return 0;
 }
 
-/** Cleanup all the user buffers. */
+/** Cleanup all the account buffers. */
 void cleanup_account(void)
 {
-  // FIXME, seems incomplete
   pfs_free(account_array);
   account_array= NULL;
   pfs_free(account_instr_class_waits_array);
   account_instr_class_waits_array= NULL;
+  pfs_free(account_instr_class_stages_array);
+  account_instr_class_stages_array= NULL;
+  pfs_free(account_instr_class_statements_array);
+  account_instr_class_statements_array= NULL;
+  pfs_free(account_instr_class_memory_array);
+  account_instr_class_memory_array= NULL;
   account_max= 0;
 }
 

storage/perfschema/pfs_instr.cc

@@ -547,6 +547,12 @@ void cleanup_instruments(void)
   thread_statements_stack_array= NULL;
   pfs_free(thread_instr_class_waits_array);
   thread_instr_class_waits_array= NULL;
+  pfs_free(thread_instr_class_stages_array);
+  thread_instr_class_stages_array= NULL;
+  pfs_free(thread_instr_class_statements_array);
+  thread_instr_class_statements_array= NULL;
+  pfs_free(thread_instr_class_memory_array);
+  thread_instr_class_memory_array= NULL;
   pfs_free(global_instr_class_stages_array);
   global_instr_class_stages_array= NULL;
   pfs_free(global_instr_class_statements_array);

storage/perfschema/pfs_program.cc

@@ -88,6 +88,7 @@ void cleanup_program(void)
   /*  Free memory allocated to program_array. */
   pfs_free(program_array);
   program_array= NULL;
+  program_max= 0;
 }
 
 C_MODE_START

storage/perfschema/pfs_server.cc

@@ -168,39 +168,100 @@ static void destroy_pfs_thread(void *key)
 
 static void cleanup_performance_schema(void)
 {
+  /*
+    my.cnf options
+  */
+
   cleanup_instrument_config();
-/*  Disabled: Bug#5666
-  cleanup_instruments();
+
+// Waiting for Bug#56666 / WL#6407
+//
+// #define HAVE_WL6407
+
+#ifdef HAVE_WL6407
+
+  /*
+    All the LF_HASH
+  */
+
+  cleanup_setup_actor_hash();
+  cleanup_setup_object_hash();
+  cleanup_account_hash();
+  cleanup_host_hash();
+  cleanup_user_hash();
+  cleanup_program_hash();
+  cleanup_table_share_hash();
+  cleanup_file_hash();
+
+  /*
+    Then the lookup tables
+  */
+
+  cleanup_setup_actor();
+  cleanup_setup_object();
+
+  /*
+    Then the history tables
+  */
+
+  cleanup_events_waits_history_long();
+  cleanup_events_stages_history_long();
+  cleanup_events_statements_history_long();
+
+  /*
+    Then the various aggregations
+  */
+
+  cleanup_digest();
+  cleanup_account();
+  cleanup_host();
+  cleanup_user();
+
+  /*
+    Then the instrument classes.
+    Once a class is cleaned up,
+    find_XXX_class(key)
+    will return PSI_NOT_INSTRUMENTED
+  */
+  cleanup_program();
   cleanup_sync_class();
   cleanup_thread_class();
   cleanup_table_share();
   cleanup_file_class();
   cleanup_stage_class();
   cleanup_statement_class();
   cleanup_socket_class();
-  cleanup_events_waits_history_long();
-  cleanup_events_stages_history_long();
-  cleanup_events_statements_history_long();
-  cleanup_table_share_hash();
-  cleanup_file_hash();
-  cleanup_setup_actor();
-  cleanup_setup_actor_hash();
-  cleanup_setup_object();
-  cleanup_setup_object_hash();
-  cleanup_host();
-  cleanup_host_hash();
-  cleanup_user();
-  cleanup_user_hash();
-  cleanup_account();
-  cleanup_account_hash();
-  cleanup_digest();
+  cleanup_memory_class();
+
+  cleanup_instruments();
+
   PFS_atomic::cleanup();
-*/
+#endif
 }
 
 void shutdown_performance_schema(void)
 {
   pfs_initialized= false;
+
+  /* disable everything, especially for this thread. */
+  flag_events_stages_current= false;
+  flag_events_stages_history= false;
+  flag_events_stages_history_long= false;
+  flag_events_statements_current= false;
+  flag_events_statements_history= false;
+  flag_events_statements_history_long= false;
+  flag_events_waits_current= false;
+  flag_events_waits_history= false;
+  flag_events_waits_history_long= false;
+  flag_global_instrumentation= false;
+  flag_thread_instrumentation= false;
+  flag_statements_digest= false;
+
+  global_table_io_class.m_enabled= false;
+  global_table_lock_class.m_enabled= false;
+  global_idle_class.m_enabled= false;
+  global_metadata_class.m_enabled= false;
+
   cleanup_performance_schema();
 #if 0
   /*

storage/perfschema/unittest/pfs-t.cc

@@ -1633,6 +1633,8 @@ void test_event_name_index()
   ok(global_table_io_class.m_event_name_index == 0, "index 0");
   ok(global_table_lock_class.m_event_name_index == 1, "index 1");
   ok(wait_class_max= 314, "314 event names"); // 4 global classes
+
+  shutdown_performance_schema();
 }
 
 void test_memory_instruments()
@@ -1720,6 +1722,57 @@ void test_memory_instruments()
   shutdown_performance_schema();
 }
 
+void test_leaks()
+{
+  PSI_bootstrap *boot;
+  PFS_global_param param;
+
+  /* Allocate everything, to make sure cleanup does not forget anything. */
+
+  memset(& param, 0xFF, sizeof(param));
+  param.m_enabled= true;
+  param.m_mutex_class_sizing= 10;
+  param.m_rwlock_class_sizing= 10;
+  param.m_cond_class_sizing= 10;
+  param.m_thread_class_sizing= 10;
+  param.m_table_share_sizing= 10;
+  param.m_file_class_sizing= 10;
+  param.m_socket_class_sizing= 10;
+  param.m_mutex_sizing= 1000;
+  param.m_rwlock_sizing= 1000;
+  param.m_cond_sizing= 1000;
+  param.m_thread_sizing= 1000;
+  param.m_table_sizing= 1000;
+  param.m_file_sizing= 1000;
+  param.m_file_handle_sizing= 1000;
+  param.m_socket_sizing= 1000;
+  param.m_events_waits_history_sizing= 10;
+  param.m_events_waits_history_long_sizing= 1000;
+  param.m_setup_actor_sizing= 1000;
+  param.m_setup_object_sizing= 1000;
+  param.m_host_sizing= 1000;
+  param.m_user_sizing= 1000;
+  param.m_account_sizing= 1000;
+  param.m_stage_class_sizing= 10;
+  param.m_events_stages_history_sizing= 10;
+  param.m_events_stages_history_long_sizing= 1000;
+  param.m_statement_class_sizing= 10;
+  param.m_events_statements_history_sizing= 10;
+  param.m_events_statements_history_long_sizing= 1000;
+  param.m_session_connect_attrs_sizing= 1000;
+  param.m_memory_class_sizing= 10;
+  param.m_metadata_lock_sizing= 1000;
+  param.m_digest_sizing= 1000;
+  param.m_program_sizing= 1000;
+  param.m_statement_stack_sizing= 10;
+
+  boot= initialize_performance_schema(& param);
+  ok(boot != NULL, "bootstrap");
+  shutdown_performance_schema();
+
+  /* Leaks will be reported with valgrind */
+}
+
 void do_all_tests()
 {
   /* Using initialize_performance_schema(), no partial init needed. */
@@ -1731,11 +1784,12 @@ void do_all_tests()
   test_file_instrumentation_leak();
   test_event_name_index();
   test_memory_instruments();
+  test_leaks();
 }
 
 int main(int, char **)
 {
-  plan(228);
+  plan(229);
 
   MY_INIT("pfs-t");
   do_all_tests();