mirror/mariadb
1
0
Fork 0
mirror of https://github.com/MariaDB/server.git synced 2026-05-17 20:37:12 +02:00
Code Issues Projects Releases Packages Wiki Activity

MDEV-23132 Assertion `trn->locked_tables == 0 && trn->used_instances == 0' failed in trnman_end_trn

Browse source 
The reason for the crash was that two tables where updating Aria's
TRN->used_instances at the same time.  This could happen when a
thread started a sub transaction with Aria tables, like reading a
stored procedure from the proc table, at the same time another table
was clearing the table list after committing a transaction involving
Aria tables.  The timing window for this to happen is very small,
which is why we did not notice this issue for 5 years.

The fix was to change reset_thd_trn() to clear the table links directly,
instead of calling _ma_reset_trn_for_table() which removed the table
from the linked list, which included updating TRN->used_instances.

This bug could happen when maria_commit or maria_rollback() where
called but not in ha_maria::implicit_commit() which had already
a fix for this problem.

Other things:
- Removed duplicate call to thd_set_ha_data(thd, maria_hton, trn)
  in ha_maria::implicit_commit() and maria_commit() when TRN is null.
This commit is contained in:
Monty 2025-12-01 11:19:17 +02:00
commit eedee45885
2 changed files with 32 additions and 5 deletions
Download patch file Download diff file Expand all files Collapse all files

25
storage/maria/ha_maria.cc
View file

@ -3028,17 +3028,23 @@ int ha_maria::start_stmt(THD *thd, thr_lock_type lock_type)
Reset THD_TRN and all file->trn related to the transaction
This is needed as some calls, like extra() or external_lock() may access
it before next transaction is started
Note that trn for the table is already freed and may be reused by another
thread.
*/
static void reset_thd_trn(THD *thd, MARIA_HA *first_table)
{
MARIA_HA *next;
TRN *trn __attribute__((unused)) = first_table ? first_table->trn : 0;
DBUG_ENTER("reset_thd_trn");
thd_set_ha_data(thd, maria_hton, 0);
MARIA_HA *next;
for (MARIA_HA *table= first_table; table ; table= next)
{
DBUG_ASSERT(table->trn == trn);
next= table->trn_next;
_ma_reset_trn_for_table(table);
table->trn_prev= 0;
table->trn_next= 0;
table->trn= 0;
/*
If table has changed by this statement, invalidate it from the query
@ -3121,13 +3127,15 @@ int ha_maria::implicit_commit(THD *thd, bool new_trn)
statement assuming they have a trn (see ha_maria::start_stmt()).
*/
trn= trnman_new_trn(& thd->transaction->wt);
thd_set_ha_data(thd, maria_hton, trn);
if (unlikely(trn == NULL))
{
reset_thd_trn(thd, used_tables);
reset_thd_trn(thd, used_tables); // Calls thd_set_ha_data()
error= HA_ERR_OUT_OF_MEM;
goto end;
}
else
thd_set_ha_data(thd, maria_hton, trn);
/*
Move all locked tables to the new transaction
We must do it here as otherwise file->thd and file->state may be
@ -3142,6 +3150,14 @@ int ha_maria::implicit_commit(THD *thd, bool new_trn)
trn_next= handler->trn_next;
DBUG_ASSERT(handler->s->base.born_transactional);
/*
We reset the link to the old trn to avoid the asserts
in _ma_set_trn_for_table()
*/
handler->trn_next= 0;
handler->trn_prev= 0;
handler->trn= 0;
/* If handler uses versioning */
if (handler->s->lock_key_trees)
{
@ -3569,7 +3585,6 @@ static int maria_commit(handlerton *hton __attribute__ ((unused)),
if (ma_commit(trn))
res= HA_ERR_COMMIT_ERROR;
reset_thd_trn(thd, used_instances);
thd_set_ha_data(thd, maria_hton, 0);
DBUG_RETURN(res);
}

12
storage/maria/ma_trnman.h
View file

@ -31,6 +31,10 @@ static inline void _ma_set_trn_for_table(MARIA_HA *tbl, TRN *newtrn)
/* check that we are not calling this twice in a row */
DBUG_ASSERT(newtrn->used_instances != (void*) tbl);
DBUG_ASSERT(newtrn != &dummy_transaction_object);
DBUG_ASSERT(tbl->trn == 0);
DBUG_ASSERT(tbl->trn_next == 0);
DBUG_ASSERT(tbl->trn_prev == 0);
tbl->trn= newtrn;
/* Link into used list */
@ -63,17 +67,25 @@ static inline void _ma_set_tmp_trn_for_table(MARIA_HA *tbl, TRN *newtrn)
static inline void _ma_reset_trn_for_table(MARIA_HA *tbl)
{
TRN *trn __attribute__((unused))= tbl->trn;
DBUG_PRINT("info",("table: %p trn: %p -> NULL", tbl, tbl->trn));
/* The following is only false if tbl->trn == &dummy_transaction_object */
if (tbl->trn_prev)
{
if (tbl->trn_next)
{
DBUG_ASSERT(tbl->trn_next->trn == trn);
tbl->trn_next->trn_prev= tbl->trn_prev;
}
*tbl->trn_prev= tbl->trn_next;
tbl->trn_prev= 0;
tbl->trn_next= 0;
}
else
{
DBUG_ASSERT(tbl->trn_next == 0);
}
tbl->trn= 0;
}