Commit 607e2a62 authored by unknown's avatar unknown

Fixed mutexes lock order in maria_close(): LOCK_trn_list ->...

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).
parent 29d7e16d
......@@ -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)
......
......@@ -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;
......
......@@ -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();
}
......
......@@ -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);
......@@ -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*/
......
......@@ -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);
}
......@@ -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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment