Fixed mutexes lock order in maria_close(): LOCK_trn_list -> MARIA_SHARE::intern_lock (then will be WL to revert the order).

(BUG#40981 Maria: deadlock between checkpoint and maria_close() when freeing state info)

storage/maria/ma_checkpoint.c:
  The argument added to the function to use it in maria_close().
storage/maria/ma_close.c:
  Locking/unlocking MARIA_SHARE::intern_lock added to use correct order of the mutexes taking.
storage/maria/ma_state.c:
  Removed assert becase now we have externally locked mutex in maria_close().
  The argument added to the _ma_remove_not_visible_states_with_lock() to use it in maria_close().
  _ma_remove_not_visible_states_with_lock() fixed tio be usable from maria_chk where transaction manager is not initialized.
storage/maria/ma_state.h:
  The argument added to the function to use it in maria_close().
storage/maria/maria_def.h:
  Fixed comment to the variable.
storage/maria/trnman.c:
  The debugging assert added.
  New function to detect transaction manager initialization added (maria_chk do not initialize it).
storage/maria/trnman_public.h:
  New function to detect transaction manager initialization added (maria_chk do not initialize it).

7 files changed, 49 insertions, 18 deletions

diff --git a/storage/maria/ma_checkpoint.c b/storage/maria/ma_checkpoint.c
index 91cb026d5ed..4538865fa02 100644
--- a/storage/maria/ma_checkpoint.c
+++ b/storage/maria/ma_checkpoint.c
@@ -1059,7 +1059,7 @@ static int collect_tables(LEX_STRING *str, LSN checkpoint_start_log_horizon)
       since last call.
     */
     pthread_mutex_unlock(&share->intern_lock);
-    _ma_remove_not_visible_states_with_lock(share);
+    _ma_remove_not_visible_states_with_lock(share, FALSE);
     pthread_mutex_lock(&share->intern_lock);
 
     if (share->in_checkpoint & MARIA_CHECKPOINT_SHOULD_FREE_ME)
diff --git a/storage/maria/ma_close.c b/storage/maria/ma_close.c
index 9463ad8078d..3f91a3723a1 100644
--- a/storage/maria/ma_close.c
+++ b/storage/maria/ma_close.c
@@ -124,22 +124,30 @@ int maria_close(register MARIA_HA *info)
     }
 #endif
     DBUG_ASSERT(share->now_transactional == share->base.born_transactional);
-    if (share->in_checkpoint == MARIA_CHECKPOINT_LOOKS_AT_ME)
+    /*
+      We assign -1 because checkpoint does not need to flush (in case we
+      have concurrent checkpoint if no then we do not need it here also)
+    */
+    share->kfile.file= -1;
+
+    /*
+      Remember share->history for future opens
+
+      We have to unlock share->intern_lock then lock it after
+      LOCK_trn_list (trnman_lock()) to avoid dead locks.
+    */
+    pthread_mutex_unlock(&share->intern_lock);
+    _ma_remove_not_visible_states_with_lock(share, TRUE);
+    pthread_mutex_lock(&share->intern_lock);
+
+    if (share->in_checkpoint & MARIA_CHECKPOINT_LOOKS_AT_ME)
     {
-      share->kfile.file= -1; /* because Checkpoint does not need to flush */
       /* we cannot my_free() the share, Checkpoint would see a bad pointer */
       share->in_checkpoint|= MARIA_CHECKPOINT_SHOULD_FREE_ME;
     }
     else
       share_can_be_freed= TRUE;
 
-    /*
-      Remember share->history for future opens
-      Here we take share->intern_lock followed by trans_lock but this is
-      safe as no other thread one can use 'share' here.
-    */
-    share->state_history= _ma_remove_not_visible_states(share->state_history,
-                                                        1, 0);
     if (share->state_history)
     {
       MARIA_STATE_HISTORY_CLOSED *history;
diff --git a/storage/maria/ma_state.c b/storage/maria/ma_state.c
index 785f1689a37..4f7cebcd86a 100644
--- a/storage/maria/ma_state.c
+++ b/storage/maria/ma_state.c
@@ -166,7 +166,6 @@ MARIA_STATE_HISTORY
 
   if (all && parent == &org_history->next)
   {
-    DBUG_ASSERT(trnman_is_locked == 0);
     /* There is only one state left. Delete this if it's visible for all */
     if (last_trid < trnman_get_min_trid())
     {
@@ -181,6 +180,11 @@ MARIA_STATE_HISTORY
 /**
    @brief Remove not used state history
 
+   @param share          Maria table information
+   @param all            1 if we should delete the first state if it's
+                         visible for all.  For the moment this is only used
+                         on close() of table.
+
    @notes
    share and trnman are not locked.
 
@@ -189,14 +193,19 @@ MARIA_STATE_HISTORY
    takes share->intern_lock.
 */
 
-void _ma_remove_not_visible_states_with_lock(MARIA_SHARE *share)
+void _ma_remove_not_visible_states_with_lock(MARIA_SHARE *share,
+                                             my_bool all)
 {
-  trnman_lock();
+  my_bool is_lock_trman;
+  if ((is_lock_trman= trman_is_inited()))
+    trnman_lock();
+
   pthread_mutex_lock(&share->intern_lock);
-  share->state_history=  _ma_remove_not_visible_states(share->state_history, 0,
-                                                       1);
+  share->state_history=  _ma_remove_not_visible_states(share->state_history,
+                                                       all, 1);
   pthread_mutex_unlock(&share->intern_lock);
-  trnman_unlock();
+  if (is_lock_trman)
+    trnman_unlock();
 }
 
 
diff --git a/storage/maria/ma_state.h b/storage/maria/ma_state.h
index 968c526cd98..b5790aa17b8 100644
--- a/storage/maria/ma_state.h
+++ b/storage/maria/ma_state.h
@@ -78,6 +78,7 @@ my_bool _ma_trnman_end_trans_hook(TRN *trn, my_bool commit,
 my_bool _ma_row_visible_always(MARIA_HA *info);
 my_bool _ma_row_visible_non_transactional_table(MARIA_HA *info);
 my_bool _ma_row_visible_transactional_table(MARIA_HA *info);
-void _ma_remove_not_visible_states_with_lock(struct st_maria_share *share);
+void _ma_remove_not_visible_states_with_lock(struct st_maria_share *share,
+                                             my_bool all);
 void _ma_remove_table_from_trnman(struct st_maria_share *share, TRN *trn);
 void _ma_reset_history(struct st_maria_share *share);
diff --git a/storage/maria/maria_def.h b/storage/maria/maria_def.h
index 125cd461570..bd2aaba29d7 100644
--- a/storage/maria/maria_def.h
+++ b/storage/maria/maria_def.h
@@ -353,7 +353,7 @@ typedef struct st_maria_share
   PAGECACHE_FILE kfile;			/* Shared keyfile */
   File data_file;			/* Shared data file */
   int mode;				/* mode of file on open */
-  uint reopen;				/* How many times reopened */
+  uint reopen;				/* How many times opened */
   uint in_trans;                        /* Number of references by trn */
   uint w_locks, r_locks, tot_locks;	/* Number of read/write locks */
   uint block_size;			/* block_size of keyfile & data file*/
diff --git a/storage/maria/trnman.c b/storage/maria/trnman.c
index 538dc75b663..f7afea51a63 100644
--- a/storage/maria/trnman.c
+++ b/storage/maria/trnman.c
@@ -874,6 +874,7 @@ my_bool trnman_exists_active_transactions(TrID min_id, TrID max_id,
 
   if (!trnman_is_locked)
     pthread_mutex_lock(&LOCK_trn_list);
+  safe_mutex_assert_owner(&LOCK_trn_list);
   for (trn= active_list_min.next; trn != &active_list_max; trn= trn->next)
   {
     if (trn->trid > min_id && trn->trid < max_id)
@@ -897,6 +898,7 @@ void trnman_lock()
   pthread_mutex_lock(&LOCK_trn_list);
 }
 
+
 /**
    unlock transaction list
 */
@@ -905,3 +907,13 @@ void trnman_unlock()
 {
   pthread_mutex_unlock(&LOCK_trn_list);
 }
+
+
+/**
+  Is trman initialized
+*/
+
+my_bool trman_is_inited()
+{
+  return (short_trid_to_active_trn != NULL);
+}
diff --git a/storage/maria/trnman_public.h b/storage/maria/trnman_public.h
index b89ce23df37..d29ada159ac 100644
--- a/storage/maria/trnman_public.h
+++ b/storage/maria/trnman_public.h
@@ -69,5 +69,6 @@ my_bool trnman_exists_active_transactions(TrID min_id, TrID max_id,
 #define transid_korr(P) uint6korr(P)
 void trnman_lock();
 void trnman_unlock();
+my_bool trman_is_inited();
 C_MODE_END
 #endif