Commit e1c8d9c9 authored by unknown's avatar unknown

Fix for bug #16593 "Deadlock or crash in stress test for case where

trigger starts trigger".

In short, the deadlock/crash happened when execution of statement, which used
stored functions or activated triggers, coincided with alteration of the
tables used by these functions or triggers (in highly concurrent environment).

Bug was caused by the incorrect handling of tables from prelocked set in
open_tables() functions in situations when refresh happened. This fix replaces
old smart but not very robust way of handling tables after refresh (which was
closing only old tables), with new one which simply closes all tables opened so
far and restarts open_tables().
Also fixed handling of temporary tables in close_tables_for_reopen().

No test case present since bug manifests itself only in concurrent environment.


sql/mysql_priv.h:
  In order to handle correctly case when table list completely consists from tables
  from prelocked set close_tables_for_reopen() have to accept table list as in/out
  parameter.
sql/sql_base.cc:
  open_tables():
    Removed part of comment  which was out of date.
    Changed handling of case when refresh happens during opening of tables, now
    instead of having code which decides for each table if it should be closed
    we simply close all tables. Old code also incorrectly handled tables from
    prelocked set in this situation which resulted in bug #16593 "Deadlock or
    crash in stress test for case where triggers starting trigger".
  close_tables_for_reopen():
    Now we correctly handle the case when table list completely consists from
    tables from prelocked set. Also now we simply close all tables instead
    leaving temporary tables non-closed (such approach allows easily handle
    correctly tables from prelocked set).
sql/sql_prepare.cc:
  In order to handle correctly case when table list completely consists from tables
  from prelocked set close_tables_for_reopen() have to accept table list as in/out
  parameter.
sql/sql_update.cc:
  In order to handle correctly case when table list completely consists from tables
  from prelocked set close_tables_for_reopen() have to accept table list as in/out
  parameter.
parent 86733db8
...@@ -981,7 +981,7 @@ void free_io_cache(TABLE *entry); ...@@ -981,7 +981,7 @@ void free_io_cache(TABLE *entry);
void intern_close_table(TABLE *entry); void intern_close_table(TABLE *entry);
bool close_thread_table(THD *thd, TABLE **table_ptr); bool close_thread_table(THD *thd, TABLE **table_ptr);
void close_temporary_tables(THD *thd); void close_temporary_tables(THD *thd);
void close_tables_for_reopen(THD *thd, TABLE_LIST *tables); void close_tables_for_reopen(THD *thd, TABLE_LIST **tables);
TABLE_LIST *find_table_in_list(TABLE_LIST *table, TABLE_LIST *find_table_in_list(TABLE_LIST *table,
uint offset_to_list, uint offset_to_list,
const char *db_name, const char *db_name,
......
...@@ -1982,22 +1982,11 @@ int open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags) ...@@ -1982,22 +1982,11 @@ int open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags)
statement for which table list for prelocking is already built, let statement for which table list for prelocking is already built, let
us cache routines and try to build such table list. us cache routines and try to build such table list.
NOTE: We can't delay prelocking until we will met some sub-statement
which really uses tables, since this will imply that we have to restore
its table list to be able execute it in some other context.
And current views implementation assumes that view tables are added to
global table list only once during PS preparing/first SP execution.
Also locking at earlier stage is probably faster altough may decrease
concurrency a bit.
NOTE: We will mark statement as requiring prelocking only if we will NOTE: We will mark statement as requiring prelocking only if we will
have non empty table list. But this does not guarantee that in prelocked have non empty table list. But this does not guarantee that in prelocked
mode we will have some locked tables, because queries which use only mode we will have some locked tables, because queries which use only
derived/information schema tables and views possible. Thus "counter" derived/information schema tables and views possible. Thus "counter"
may be still zero for prelocked statement... may be still zero for prelocked statement...
NOTE: The above notes may be out of date. Please wait for psergey to
document new prelocked behavior.
*/ */
if (!thd->prelocked_mode && !thd->lex->requires_prelocking() && if (!thd->prelocked_mode && !thd->lex->requires_prelocking() &&
...@@ -2083,48 +2072,23 @@ int open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags) ...@@ -2083,48 +2072,23 @@ int open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags)
if (refresh) // Refresh in progress if (refresh) // Refresh in progress
{ {
/* close all 'old' tables used by this thread */
pthread_mutex_lock(&LOCK_open);
// if query_id is not reset, we will get an error
// re-opening a temp table
thd->version=refresh_version;
TABLE **prev_table= &thd->open_tables;
bool found=0;
for (TABLE_LIST *tmp= *start; tmp; tmp= tmp->next_global)
{
/* Close normal (not temporary) changed tables */
if (tmp->table && ! tmp->table->s->tmp_table)
{
if (tmp->table->s->version != refresh_version ||
! tmp->table->db_stat)
{
VOID(hash_delete(&open_cache,(byte*) tmp->table));
tmp->table=0;
found=1;
}
else
{
*prev_table= tmp->table; // Relink open list
prev_table= &tmp->table->next;
}
}
}
*prev_table=0;
pthread_mutex_unlock(&LOCK_open);
if (found)
VOID(pthread_cond_broadcast(&COND_refresh)); // Signal to refresh
/* /*
Let us prepare for recalculation of set of prelocked tables. We have met name-locked or old version of table. Now we have
First we pretend that we have finished calculation which we to close all tables which are not up to date. We also have to
were doing currently. Then we restore list of tables to be throw away set of prelocked tables (and thus close tables from
opened and set of used routines to the state in which they were this set that were open by now) since it possible that one of
before first open_tables() call for this statement (i.e. before tables which determined its content was changed.
we have calculated current set of tables for prelocking).
Instead of implementing complex/non-robust logic mentioned
above we simply close and then reopen all tables.
In order to prepare for recalculation of set of prelocked tables
we pretend that we have finished calculation which we were doing
currently.
*/ */
if (query_tables_last_own) if (query_tables_last_own)
thd->lex->mark_as_requiring_prelocking(query_tables_last_own); thd->lex->mark_as_requiring_prelocking(query_tables_last_own);
thd->lex->chop_off_not_own_tables(); close_tables_for_reopen(thd, start);
sp_remove_not_own_routines(thd->lex);
goto restart; goto restart;
} }
result= -1; // Fatal error result= -1; // Fatal error
...@@ -2335,7 +2299,7 @@ int simple_open_n_lock_tables(THD *thd, TABLE_LIST *tables) ...@@ -2335,7 +2299,7 @@ int simple_open_n_lock_tables(THD *thd, TABLE_LIST *tables)
break; break;
if (!need_reopen) if (!need_reopen)
DBUG_RETURN(-1); DBUG_RETURN(-1);
close_tables_for_reopen(thd, tables); close_tables_for_reopen(thd, &tables);
} }
DBUG_RETURN(0); DBUG_RETURN(0);
} }
...@@ -2372,7 +2336,7 @@ bool open_and_lock_tables(THD *thd, TABLE_LIST *tables) ...@@ -2372,7 +2336,7 @@ bool open_and_lock_tables(THD *thd, TABLE_LIST *tables)
break; break;
if (!need_reopen) if (!need_reopen)
DBUG_RETURN(-1); DBUG_RETURN(-1);
close_tables_for_reopen(thd, tables); close_tables_for_reopen(thd, &tables);
} }
if (mysql_handle_derived(thd->lex, &mysql_derived_prepare) || if (mysql_handle_derived(thd->lex, &mysql_derived_prepare) ||
(thd->fill_derived_tables() && (thd->fill_derived_tables() &&
...@@ -2600,18 +2564,24 @@ int lock_tables(THD *thd, TABLE_LIST *tables, uint count, bool *need_reopen) ...@@ -2600,18 +2564,24 @@ int lock_tables(THD *thd, TABLE_LIST *tables, uint count, bool *need_reopen)
SYNOPSIS SYNOPSIS
close_tables_for_reopen() close_tables_for_reopen()
thd Thread context thd in Thread context
tables List of tables which we were trying to open and lock tables in/out List of tables which we were trying to open and lock
*/ */
void close_tables_for_reopen(THD *thd, TABLE_LIST *tables) void close_tables_for_reopen(THD *thd, TABLE_LIST **tables)
{ {
/*
If table list consists only from tables from prelocking set, table list
for new attempt should be empty, so we have to update list's root pointer.
*/
if (thd->lex->first_not_own_table() == *tables)
*tables= 0;
thd->lex->chop_off_not_own_tables(); thd->lex->chop_off_not_own_tables();
sp_remove_not_own_routines(thd->lex); sp_remove_not_own_routines(thd->lex);
for (TABLE_LIST *tmp= tables; tmp; tmp= tmp->next_global) for (TABLE_LIST *tmp= *tables; tmp; tmp= tmp->next_global)
if (tmp->table && !tmp->table->s->tmp_table) tmp->table= 0;
tmp->table= 0; mark_used_tables_as_free_for_reuse(thd, thd->temporary_tables);
close_thread_tables(thd); close_thread_tables(thd);
} }
......
...@@ -1135,7 +1135,7 @@ static int mysql_test_update(Prepared_statement *stmt, ...@@ -1135,7 +1135,7 @@ static int mysql_test_update(Prepared_statement *stmt,
break; break;
if (!need_reopen) if (!need_reopen)
goto error; goto error;
close_tables_for_reopen(thd, table_list); close_tables_for_reopen(thd, &table_list);
} }
/* /*
......
...@@ -158,7 +158,7 @@ int mysql_update(THD *thd, ...@@ -158,7 +158,7 @@ int mysql_update(THD *thd,
break; break;
if (!need_reopen) if (!need_reopen)
DBUG_RETURN(1); DBUG_RETURN(1);
close_tables_for_reopen(thd, table_list); close_tables_for_reopen(thd, &table_list);
} }
if (mysql_handle_derived(thd->lex, &mysql_derived_prepare) || if (mysql_handle_derived(thd->lex, &mysql_derived_prepare) ||
...@@ -813,7 +813,7 @@ reopen_tables: ...@@ -813,7 +813,7 @@ reopen_tables:
for (TABLE_LIST *tbl= table_list; tbl; tbl= tbl->next_global) for (TABLE_LIST *tbl= table_list; tbl; tbl= tbl->next_global)
tbl->cleanup_items(); tbl->cleanup_items();
close_tables_for_reopen(thd, table_list); close_tables_for_reopen(thd, &table_list);
goto reopen_tables; goto reopen_tables;
} }
......
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