From 17695cb4ffcddb9634a9e27c459eab943ceae36e Mon Sep 17 00:00:00 2001 From: Gopal Shankar Date: Fri, 24 Aug 2012 09:51:42 +0530 Subject: [PATCH] Bug#14364558 ASSERT `TABLE_LIST->PRELOCKING_PLACEHOLDER==FALSE' FAILED IN CHECK_LOCK_AND_ST Problem: -------- lock_tables() is supposed to invoke check_lock_and_start_stmt() for TABLE_LIST which are directly used by top level statement. TABLE_LIST->prelocking_placeholder is set only for TABLE_LIST which are used indirectly by stored programs invoked by top level statement. Hence check_lock_and_start_stmt() should have TABLE_LIST->prelocking_placeholder==false always, but it is observed that this assert fails. The failure is found during RQG test rqg_signal_resignal. Analysis: --------- open_tables() invokes open_and_process_routines() where it finds all the TABLE_LIST that belong to the routine and adds it to thd->lex->query_tables. During this process if the open_and_process_routines() fail for some reason, we are supposed to chop-off all the TABLE_LIST found during calls to open_and_process_routines(). But, in practice this is not happening. thd->lex->query_tables_own_last is supposed to point to a node in thd->lex->query_tables, which would be a first TABLE_LIST used indirectly by stored programs invoked by top level statement. This is found to be not-set correctly when we plan to chop-off TABLE_LIST's, when open_and_process_routines() failed. close_tables_for_reopen() does chop-off all the TABLE_LIST added after thd->lex->query_table_own_last. This is invoked upon error in open_and_process_routines(). This call would not work as expected as thd->lex->query_tables_own_last is not set, or is not set to correctly. Further, when open_tables() restarts the process of finding TABLE_LIST belonging to stored programs, and as the thd->lex->query_tables_own_last points to in-correct node, there is possibility of new iteration setting the thd->lex->query_tables_own_last past some old nodes that belong to stored programs, added earlier and not removed. Later when open_tables() completes, lock_tables() ends up invoking check_lock_and_start_stmt() for TABLE_LIST which belong to stored programs, which is not expected behavior and hence we hit the assert TABLE_LIST->prelocking_placeholder==false. Due to above behavior, if a user application tries to execute a SQL statement which invokes some stored function and if the lock grant on stored function fails due to a deadlock, then mysqld crashes. Fix: ---- open_tables() remembers save_query_tables_last which points to thd-lex->query_tables_last before calls to open_and_process_routines(). If there is no known thd->lex->query_tables_own_last set, we are now setting thd->lex->query_tables_own_last to save_query_tables_last. This will make sure that the call to close_tables_for_reopen() will chop-off the list correctly, in other words we now remove all the nodes added to thd->lex->query_tables, by previous calls to open_and_process_routines(). Further, it is found that the problem exists starting from 5.5, due to a code refactoring effort related to open_tables(). Hence, the fix will be pushed in 5.5, 5.6 and trunk. --- sql/sql_base.cc | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/sql/sql_base.cc b/sql/sql_base.cc index efd9c4dfd4a..a7d04f12341 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -4955,8 +4955,6 @@ restart: */ if (thd->locked_tables_mode <= LTM_LOCK_TABLES) { - bool need_prelocking= FALSE; - TABLE_LIST **save_query_tables_last= thd->lex->query_tables_last; /* Process elements of the prelocking set which are present there since parsing stage or were added to it by invocations of @@ -4969,10 +4967,19 @@ restart: for (Sroutine_hash_entry *rt= *sroutine_to_open; rt; sroutine_to_open= &rt->next, rt= rt->next) { + bool need_prelocking= false; + TABLE_LIST **save_query_tables_last= thd->lex->query_tables_last; + error= open_and_process_routine(thd, thd->lex, rt, prelocking_strategy, has_prelocking_list, &ot_ctx, &need_prelocking); + if (need_prelocking && ! thd->lex->requires_prelocking()) + thd->lex->mark_as_requiring_prelocking(save_query_tables_last); + + if (need_prelocking && ! *start) + *start= thd->lex->query_tables; + if (error) { if (ot_ctx.can_recover_from_failed_open()) @@ -4993,12 +5000,6 @@ restart: goto err; } } - - if (need_prelocking && ! thd->lex->requires_prelocking()) - thd->lex->mark_as_requiring_prelocking(save_query_tables_last); - - if (need_prelocking && ! *start) - *start= thd->lex->query_tables; } } @@ -5271,6 +5272,12 @@ static bool check_lock_and_start_stmt(THD *thd, thr_lock_type lock_type; DBUG_ENTER("check_lock_and_start_stmt"); + /* + Prelocking placeholder is not set for TABLE_LIST that + are directly used by TOP level statement. + */ + DBUG_ASSERT(table_list->prelocking_placeholder == false); + /* TL_WRITE_DEFAULT and TL_READ_DEFAULT are supposed to be parser only types of locks so they should be converted to appropriate other types