MDEV-11630 Call mutex_free() before freeing the mutex list

fil_space_crypt_cleanup(): Call mutex_free() to pair with
fil_space_crypt_init().

fil_space_destroy_crypt_data(): Call mutex_free() to pair with
fil_space_create_crypt_data() and fil_space_read_crypt_data().

fil_crypt_threads_cleanup(): Call mutex_free() to pair with
fil_crypt_threads_init().

fil_space_free_low(): Invoke fil_space_destroy_crypt_data().

fil_close(): Invoke fil_space_crypt_cleanup(), just like
fil_init() invoked fil_space_crypt_init().

Datafile::shutdown(): Set m_crypt_info=NULL without dereferencing
the pointer. The object will be freed along with the fil_space_t
in fil_space_free_low().
Remove some unnecessary conditions (ut_free(NULL) is OK).

srv_shutdown_all_bg_threads(): Shut down the encryption threads
by calling fil_crypt_threads_end().

srv_shutdown_bg_undo_sources(): Do not prematurely call
fil_crypt_threads_end(). Many pages can still be written by
change buffer merge, rollback of incomplete transactions, and
purge, especially in slow shutdown (innodb_fast_shutdown=0).

innobase_shutdown_for_mysql(): Call fil_crypt_threads_cleanup()
also when innodb_read_only=1, because the threads will have been
created also in that case.

sync_check_close(): Re-enable the invocation of sync_latch_meta_destroy()
to free the mutex list.

6 files changed, 18 insertions, 33 deletions

diff --git a/storage/innobase/fil/fil0crypt.cc b/storage/innobase/fil/fil0crypt.cc
index 9062bf1586b..75efcdfdab0 100644
--- a/storage/innobase/fil/fil0crypt.cc
+++ b/storage/innobase/fil/fil0crypt.cc
@@ -138,6 +138,8 @@ fil_space_crypt_cleanup()
 /*=====================*/
 {
 	os_event_destroy(fil_crypt_throttle_sleep_event);
+	mutex_free(&fil_crypt_key_mutex);
+	mutex_free(&crypt_stat_mutex);
 }
 
 /******************************************************************
@@ -335,15 +337,9 @@ fil_space_destroy_crypt_data(
 		and make it unawailable, this does not fully
 		avoid the race between drop table and crypt thread */
 		mutex_enter(&fil_crypt_threads_mutex);
-		mutex_enter(&(*crypt_data)->mutex);
-		(*crypt_data)->inited = false;
-		mutex_exit(&(*crypt_data)->mutex);
-		/* JAN: TODO:
-		mutex_free(& (*crypt_data)->mutex);
-		memset(*crypt_data, 0, sizeof(fil_space_crypt_t));
+		mutex_free(&(*crypt_data)->mutex);
 		free(*crypt_data);
-		(*crypt_data) = NULL;
-		*/
+		*crypt_data = NULL;
 		mutex_exit(&fil_crypt_threads_mutex);
 	}
 }
@@ -2468,6 +2464,7 @@ fil_crypt_threads_cleanup()
 {
 	os_event_destroy(fil_crypt_event);
 	os_event_destroy(fil_crypt_threads_event);
+	mutex_free(&fil_crypt_threads_mutex);
 	fil_crypt_threads_inited = false;
 }
 
diff --git a/storage/innobase/fil/fil0fil.cc b/storage/innobase/fil/fil0fil.cc
index 26e0593b266..3df1e5e0fef 100644
--- a/storage/innobase/fil/fil0fil.cc
+++ b/storage/innobase/fil/fil0fil.cc
@@ -1199,6 +1199,7 @@ fil_space_free_low(
 	ut_ad(space->size == 0);
 
 	rw_lock_free(&space->latch);
+	fil_space_destroy_crypt_data(&space->crypt_data);
 
 	ut_free(space->name);
 	ut_free(space);
@@ -6226,6 +6227,8 @@ fil_close(void)
 
 		ut_free(fil_system);
 		fil_system = NULL;
+
+		fil_space_crypt_cleanup();
 	}
 }
 
diff --git a/storage/innobase/fsp/fsp0file.cc b/storage/innobase/fsp/fsp0file.cc
index 501a8e58622..3c12bde0acf 100644
--- a/storage/innobase/fsp/fsp0file.cc
+++ b/storage/innobase/fsp/fsp0file.cc
@@ -63,22 +63,17 @@ Datafile::shutdown()
 	ut_free(m_name);
 	m_name = NULL;
 
-	free_filepath();
-
-	if (m_encryption_key != NULL) {
-		ut_free(m_encryption_key);
-		m_encryption_key = NULL;
-	}
+	ut_free(m_encryption_key);
+	m_encryption_key = NULL;
 
-	if (m_crypt_info) {
-		fil_space_destroy_crypt_data(&m_crypt_info);
-	}
+	/* The fil_space_t::crypt_data was freed in
+	fil_space_free_low(). Invalidate our redundant pointer. */
+	m_crypt_info = NULL;
 
-	if (m_encryption_iv != NULL) {
-		ut_free(m_encryption_iv);
-		m_encryption_iv = NULL;
-	}
+	ut_free(m_encryption_iv);
+	m_encryption_iv = NULL;
 
+	free_filepath();
 	free_first_page();
 }
 
diff --git a/storage/innobase/include/fil0fil.h b/storage/innobase/include/fil0fil.h
index bc1121d1530..bd31ed58283 100644
--- a/storage/innobase/include/fil0fil.h
+++ b/storage/innobase/include/fil0fil.h
@@ -97,8 +97,6 @@ extern const char general_space_name[];
 struct trx_t;
 class page_id_t;
 class truncate_t;
-struct fil_node_t;
-struct fil_space_t;
 struct btr_create_t;
 
 /* structure containing encryption specification */
diff --git a/storage/innobase/srv/srv0start.cc b/storage/innobase/srv/srv0start.cc
index 49c9feb6ac5..3eeb698d5f6 100644
--- a/storage/innobase/srv/srv0start.cc
+++ b/storage/innobase/srv/srv0start.cc
@@ -1226,6 +1226,7 @@ srv_shutdown_all_bg_threads()
 	ulint	i;
 
 	srv_shutdown_state = SRV_SHUTDOWN_EXIT_THREADS;
+	fil_crypt_threads_end();
 
 	if (!srv_start_state) {
 		return;
@@ -2769,9 +2770,6 @@ srv_shutdown_bg_undo_sources(void)
 {
 	fts_optimize_shutdown();
 	dict_stats_shutdown();
-
-	/* Shutdown key rotation threads */
-	fil_crypt_threads_end();
 }
 
 
@@ -2839,9 +2837,7 @@ innobase_shutdown_for_mysql(void)
 		}
 	}
 
-	if (!srv_read_only_mode) {
-		fil_crypt_threads_cleanup();
-	}
+	fil_crypt_threads_cleanup();
 
 	/* Cleanup data for datafile scrubbing */
 	btr_scrub_cleanup();
diff --git a/storage/innobase/sync/sync0debug.cc b/storage/innobase/sync/sync0debug.cc
index 072555750dc..c9b37cbbb09 100644
--- a/storage/innobase/sync/sync0debug.cc
+++ b/storage/innobase/sync/sync0debug.cc
@@ -1582,7 +1582,6 @@ sync_latch_meta_init()
 }
 
 /** Destroy the latch meta data */
-#ifdef JAN_DISABLED_FOR_NOW_AS_THIS_CAUSES_CRASH
 static
 void
 sync_latch_meta_destroy()
@@ -1596,7 +1595,6 @@ sync_latch_meta_destroy()
 
 	latch_meta.clear();
 }
-#endif
 
 /** Track mutex file creation name and line number. This is to avoid storing
 { const char* name; uint16_t line; } in every instance. This results in the
@@ -1810,8 +1808,6 @@ sync_check_close()
 
 	create_tracker = NULL;
 
-#ifdef JAN_DISABLED_FOR_NOW_AS_THIS_CAUSES_CRASH
 	sync_latch_meta_destroy();
-#endif
 }