From d7758debaee4f9f928e4fecfeea8c9df1964dce3 Mon Sep 17 00:00:00 2001 From: Dmitry Shulga Date: Wed, 13 Mar 2024 20:07:04 +0700 Subject: [PATCH] MDEV-33218: Assertion `active_arena->is_stmt_prepare_or_first_stmt_execute() || active_arena->state == Query_arena::STMT_SP_QUERY_ARGUMENTS' failed in st_select_lex::fix_prepare_information In case there is a view that queried from a stored routine or a prepared statement and this temporary table is dropped between executions of SP/PS, then it leads to hitting an assertion at the SELECT_LEX::fix_prepare_information. The fired assertion was added by the commit 85f2e4f8e8c82978bd9cc0af9bfd2b549ea04d65 (MDEV-32466: Potential memory leak on executing of create view statement). Firing of this assertion means memory leaking on execution of SP/PS. Moreover, if the added assert be commented out, different result sets can be produced by the statement SELECT * FROM the hidden table. Both hitting the assertion and different result sets have the same root cause. This cause is usage of temporary table's metadata after the table itself has been dropped. To fix the issue, reload the cache of stored routines. To do it cache of stored routines is reset at the end of execution of the function dispatch_command(). Next time any stored routine be called it will be loaded from the table mysql.proc. This happens inside the method Sp_handler::sp_cache_routine where loading of a stored routine is performed in case it missed in cache. Loading is performed unconditionally while previously it was controlled by the parameter lookup_only. By that reason the signature of the method Sroutine_hash_entry::sp_cache_routine was changed by removing unused parameter lookup_only. Clearing of sp caches affects the test main.lock_sync since it forces opening and locking the table mysql.proc but the test assumes that each statement locks its tables once during its execution. To keep this invariant the debug sync points with names "before_lock_tables_takes_lock" and "after_lock_tables_takes_lock" are not activated on handling the table mysql.proc --- mysql-test/main/ps.result | 13 ++++++++ mysql-test/main/ps.test | 12 +++++++ mysql-test/main/sp.result | 3 +- mysql-test/main/sp.test | 2 -- mysql-test/main/temp_table.result | 49 ++++++++++++++++++++++++++++ mysql-test/main/temp_table.test | 54 +++++++++++++++++++++++++++++++ sql/sp.cc | 22 ++++--------- sql/sp.h | 16 ++++----- sql/sp_cache.cc | 18 +++++++++++ sql/sql_base.cc | 18 ++++++++--- sql/sql_class.cc | 1 + sql/sql_class.h | 12 ++++++- sql/sql_parse.cc | 9 +++++- sql/sql_table.cc | 5 +++ 14 files changed, 200 insertions(+), 34 deletions(-) diff --git a/mysql-test/main/ps.result b/mysql-test/main/ps.result index aeed24f253b..7b5b9779624 100644 --- a/mysql-test/main/ps.result +++ b/mysql-test/main/ps.result @@ -5982,6 +5982,19 @@ EXECUTE stmt USING DEFAULT; # Clean up DEALLOCATE PREPARE stmt; DROP TABLE t1, t2; +# MDEV-33218: Assertion `active_arena->is_stmt_prepare_or_first_stmt_execute() || active_arena->state == Query_arena::STMT_SP_QUERY_ARGUMENTS' failed. in st_select_lex::fix_prepare_information +CREATE TABLE t1 AS SELECT 1 f; +PREPARE stmt FROM 'SHOW CREATE TABLE t1'; +DROP TABLE t1; +EXECUTE stmt; +ERROR 42S02: Table 'test.t1' doesn't exist +CREATE VIEW t1 AS SELECT 1; +EXECUTE stmt; +View Create View character_set_client collation_connection +t1 CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `t1` AS select 1 AS `1` latin1 latin1_swedish_ci +# Clean up +DEALLOCATE PREPARE stmt; +DROP VIEW t1; # # End of 10.4 tests # diff --git a/mysql-test/main/ps.test b/mysql-test/main/ps.test index 61d1513fa0b..99a479f854c 100644 --- a/mysql-test/main/ps.test +++ b/mysql-test/main/ps.test @@ -5416,6 +5416,18 @@ EXECUTE stmt USING DEFAULT; DEALLOCATE PREPARE stmt; DROP TABLE t1, t2; +--echo # MDEV-33218: Assertion `active_arena->is_stmt_prepare_or_first_stmt_execute() || active_arena->state == Query_arena::STMT_SP_QUERY_ARGUMENTS' failed. in st_select_lex::fix_prepare_information +CREATE TABLE t1 AS SELECT 1 f; +PREPARE stmt FROM 'SHOW CREATE TABLE t1'; +DROP TABLE t1; +--error ER_NO_SUCH_TABLE +EXECUTE stmt; +CREATE VIEW t1 AS SELECT 1; +EXECUTE stmt; +--echo # Clean up +DEALLOCATE PREPARE stmt; +DROP VIEW t1; + --echo # --echo # End of 10.4 tests --echo # diff --git a/mysql-test/main/sp.result b/mysql-test/main/sp.result index 2f733107e8a..bcd9e728921 100644 --- a/mysql-test/main/sp.result +++ b/mysql-test/main/sp.result @@ -7184,15 +7184,14 @@ CREATE VIEW t1 AS SELECT 10 AS f1; CALL p1(1); ERROR HY000: The target table t1 of the INSERT is not insertable-into CREATE TEMPORARY TABLE t1 (f1 INT); -# t1 still refers to the view since it was inlined CALL p1(2); -ERROR HY000: The target table t1 of the INSERT is not insertable-into DROP VIEW t1; # t1 now refers to the temporary table CALL p1(3); # Check which values were inserted into the temp table. SELECT * FROM t1; f1 +2 3 DROP TEMPORARY TABLE t1; DROP PROCEDURE p1; diff --git a/mysql-test/main/sp.test b/mysql-test/main/sp.test index 50d9a611db9..d417bc636d4 100644 --- a/mysql-test/main/sp.test +++ b/mysql-test/main/sp.test @@ -8633,8 +8633,6 @@ CALL p1(1); CREATE TEMPORARY TABLE t1 (f1 INT); ---echo # t1 still refers to the view since it was inlined ---error ER_NON_INSERTABLE_TABLE CALL p1(2); DROP VIEW t1; diff --git a/mysql-test/main/temp_table.result b/mysql-test/main/temp_table.result index 33ea1700474..bf5015d44d4 100644 --- a/mysql-test/main/temp_table.result +++ b/mysql-test/main/temp_table.result @@ -616,5 +616,54 @@ Tables_in_test # in 11.2 and above here should be listed above used temporary tables DROP TEMPORARY TABLE t1, t2; # +# MDEV-33218: Assertion `active_arena->is_stmt_prepare_or_first_stmt_execute() || active_arena->state == Query_arena::STMT_SP_QUERY_ARGUMENTS' failed. in st_select_lex::fix_prepare_information +# +CREATE VIEW v1 AS SELECT 5; +CREATE PROCEDURE sp() SELECT * FROM v1; +CREATE TEMPORARY TABLE v1 as SELECT 7; +# sp() accesses the temporary table v1 that hides the view with the same name +# Therefore expected output is the row (7) +CALL sp(); +7 +7 +DROP TEMPORARY TABLE v1; +# After the temporary table v1 has been dropped the next invocation of sp() +# accesses the view v1. So, expected output is the row (5) +CALL sp(); +5 +5 +# Clean up +DROP VIEW v1; +DROP PROCEDURE sp; +# Another use case is when a temporary table hides a view is dropped +# inside a stored routine being called. +CREATE VIEW t1 AS SELECT 1; +CREATE PROCEDURE p1() +BEGIN +DROP TEMPORARY TABLE t1; +END +| +CREATE FUNCTION f1() RETURNS INT +BEGIN +CALL p1(); +RETURN 1; +END +| +CREATE TEMPORARY TABLE t1 AS SELECT 1 AS a; +PREPARE stmt FROM 'SELECT f1()'; +EXECUTE stmt; +f1() +1 +# The temporary table t1 has been dropped on first +# execution of the prepared statement 'stmt', +# next time this statement is run it results in issuing +# the error ER_BAD_TABLE_ERROR +EXECUTE stmt; +ERROR 42S02: Unknown table 'test.t1' +# Clean up +DROP VIEW t1; +DROP FUNCTION f1; +DROP PROCEDURE p1; +# # End of 10.4 tests # diff --git a/mysql-test/main/temp_table.test b/mysql-test/main/temp_table.test index e1e671f5ae9..3984b447e91 100644 --- a/mysql-test/main/temp_table.test +++ b/mysql-test/main/temp_table.test @@ -674,6 +674,60 @@ SHOW TABLES; DROP TEMPORARY TABLE t1, t2; +--echo # +--echo # MDEV-33218: Assertion `active_arena->is_stmt_prepare_or_first_stmt_execute() || active_arena->state == Query_arena::STMT_SP_QUERY_ARGUMENTS' failed. in st_select_lex::fix_prepare_information +--echo # +CREATE VIEW v1 AS SELECT 5; +CREATE PROCEDURE sp() SELECT * FROM v1; +CREATE TEMPORARY TABLE v1 as SELECT 7; +--echo # sp() accesses the temporary table v1 that hides the view with the same name +--echo # Therefore expected output is the row (7) +CALL sp(); +DROP TEMPORARY TABLE v1; +--echo # After the temporary table v1 has been dropped the next invocation of sp() +--echo # accesses the view v1. So, expected output is the row (5) +CALL sp(); + +--echo # Clean up +DROP VIEW v1; +DROP PROCEDURE sp; + +--echo # Another use case is when a temporary table hides a view is dropped +--echo # inside a stored routine being called. + +CREATE VIEW t1 AS SELECT 1; + +--delimiter | +CREATE PROCEDURE p1() +BEGIN + DROP TEMPORARY TABLE t1; +END +| + +CREATE FUNCTION f1() RETURNS INT +BEGIN + CALL p1(); + RETURN 1; +END +| + +--delimiter ; + +CREATE TEMPORARY TABLE t1 AS SELECT 1 AS a; +PREPARE stmt FROM 'SELECT f1()'; +EXECUTE stmt; +--echo # The temporary table t1 has been dropped on first +--echo # execution of the prepared statement 'stmt', +--echo # next time this statement is run it results in issuing +--echo # the error ER_BAD_TABLE_ERROR +--error ER_BAD_TABLE_ERROR +EXECUTE stmt; + +--echo # Clean up +DROP VIEW t1; +DROP FUNCTION f1; +DROP PROCEDURE p1; + --echo # --echo # End of 10.4 tests --echo # diff --git a/sql/sp.cc b/sql/sp.cc index d25c7353ca4..d3b128d5c99 100644 --- a/sql/sp.cc +++ b/sql/sp.cc @@ -1907,7 +1907,7 @@ Sp_handler::sp_show_create_routine(THD *thd, DBUG_EXECUTE_IF("cache_sp_in_show_create", /* Some tests need just need a way to cache SP without other side-effects.*/ - sp_cache_routine(thd, name, false, &sp); + sp_cache_routine(thd, name, &sp); sp->show_create_routine(thd, this); DBUG_RETURN(false); ); @@ -2331,7 +2331,7 @@ Sp_handler::sp_cache_routine_reentrant(THD *thd, int ret; Parser_state *oldps= thd->m_parser_state; thd->m_parser_state= NULL; - ret= sp_cache_routine(thd, name, false, sp); + ret= sp_cache_routine(thd, name, sp); thd->m_parser_state= oldps; return ret; } @@ -2738,7 +2738,6 @@ void sp_update_stmt_used_routines(THD *thd, Query_tables_list *prelocking_ctx, */ int Sroutine_hash_entry::sp_cache_routine(THD *thd, - bool lookup_only, sp_head **sp) const { char qname_buff[NAME_LEN*2+1+1]; @@ -2751,7 +2750,7 @@ int Sroutine_hash_entry::sp_cache_routine(THD *thd, */ DBUG_ASSERT(mdl_request.ticket || this == thd->lex->sroutines_list.first); - return m_handler->sp_cache_routine(thd, &name, lookup_only, sp); + return m_handler->sp_cache_routine(thd, &name, sp); } @@ -2763,9 +2762,6 @@ int Sroutine_hash_entry::sp_cache_routine(THD *thd, @param[in] thd Thread context. @param[in] name Name of routine. - @param[in] lookup_only Only check that the routine is in the cache. - If it's not, don't try to load. If it is present, - but old, don't try to reload. @param[out] sp Pointer to sp_head object for routine, NULL if routine was not found. @@ -2776,7 +2772,6 @@ int Sroutine_hash_entry::sp_cache_routine(THD *thd, int Sp_handler::sp_cache_routine(THD *thd, const Database_qualified_name *name, - bool lookup_only, sp_head **sp) const { int ret= 0; @@ -2788,9 +2783,6 @@ int Sp_handler::sp_cache_routine(THD *thd, *sp= sp_cache_lookup(spc, name); - if (lookup_only) - DBUG_RETURN(SP_OK); - if (*sp) { sp_cache_flush_obsolete(spc, sp); @@ -2842,7 +2834,6 @@ int Sp_handler::sp_cache_routine(THD *thd, * name->m_db is a database name, e.g. "dbname" * name->m_name is a package-qualified name, e.g. "pkgname.spname" - @param lookup_only - don't load mysql.proc if not cached @param [OUT] sp - the result is returned here. @retval false - loaded or does not exists @retval true - error while loading mysql.proc @@ -2852,14 +2843,13 @@ int Sp_handler::sp_cache_package_routine(THD *thd, const LEX_CSTRING &pkgname_cstr, const Database_qualified_name *name, - bool lookup_only, sp_head **sp) const + sp_head **sp) const { DBUG_ENTER("sp_cache_package_routine"); DBUG_ASSERT(type() == TYPE_ENUM_FUNCTION || type() == TYPE_ENUM_PROCEDURE); sp_name pkgname(&name->m_db, &pkgname_cstr, false); sp_head *ph= NULL; int ret= sp_handler_package_body.sp_cache_routine(thd, &pkgname, - lookup_only, &ph); if (!ret) { @@ -2894,12 +2884,12 @@ Sp_handler::sp_cache_package_routine(THD *thd, int Sp_handler::sp_cache_package_routine(THD *thd, const Database_qualified_name *name, - bool lookup_only, sp_head **sp) const + sp_head **sp) const { DBUG_ENTER("Sp_handler::sp_cache_package_routine"); Prefix_name_buf pkgname(thd, name->m_name); DBUG_ASSERT(pkgname.length); - DBUG_RETURN(sp_cache_package_routine(thd, pkgname, name, lookup_only, sp)); + DBUG_RETURN(sp_cache_package_routine(thd, pkgname, name, sp)); } diff --git a/sql/sp.h b/sql/sp.h index 56007a2fa72..75347464c8a 100644 --- a/sql/sp.h +++ b/sql/sp.h @@ -101,10 +101,10 @@ protected: int sp_cache_package_routine(THD *thd, const LEX_CSTRING &pkgname_cstr, const Database_qualified_name *name, - bool lookup_only, sp_head **sp) const; + sp_head **sp) const; int sp_cache_package_routine(THD *thd, const Database_qualified_name *name, - bool lookup_only, sp_head **sp) const; + sp_head **sp) const; sp_head *sp_find_package_routine(THD *thd, const LEX_CSTRING pkgname_str, const Database_qualified_name *name, @@ -202,7 +202,7 @@ public: const Database_qualified_name *name, bool cache_only) const; virtual int sp_cache_routine(THD *thd, const Database_qualified_name *name, - bool lookup_only, sp_head **sp) const; + sp_head **sp) const; int sp_cache_routine_reentrant(THD *thd, const Database_qualified_name *nm, @@ -283,9 +283,9 @@ class Sp_handler_package_procedure: public Sp_handler_procedure { public: int sp_cache_routine(THD *thd, const Database_qualified_name *name, - bool lookup_only, sp_head **sp) const + sp_head **sp) const { - return sp_cache_package_routine(thd, name, lookup_only, sp); + return sp_cache_package_routine(thd, name, sp); } sp_head *sp_find_routine(THD *thd, const Database_qualified_name *name, @@ -332,9 +332,9 @@ class Sp_handler_package_function: public Sp_handler_function { public: int sp_cache_routine(THD *thd, const Database_qualified_name *name, - bool lookup_only, sp_head **sp) const + sp_head **sp) const { - return sp_cache_package_routine(thd, name, lookup_only, sp); + return sp_cache_package_routine(thd, name, sp); } sp_head *sp_find_routine(THD *thd, const Database_qualified_name *name, @@ -632,7 +632,7 @@ public: const Sp_handler *m_handler; - int sp_cache_routine(THD *thd, bool lookup_only, sp_head **sp) const; + int sp_cache_routine(THD *thd, sp_head **sp) const; }; diff --git a/sql/sp_cache.cc b/sql/sp_cache.cc index cfdfea9ae0a..c1b9815d26e 100644 --- a/sql/sp_cache.cc +++ b/sql/sp_cache.cc @@ -78,6 +78,8 @@ private: /* All routines in this cache */ HASH m_hashtable; +public: + void clear(); }; // class sp_cache #ifdef HAVE_PSI_INTERFACE @@ -313,6 +315,10 @@ sp_cache::cleanup() my_hash_free(&m_hashtable); } +void sp_cache::clear() +{ + my_hash_reset(&m_hashtable); +} void Sp_caches::sp_caches_clear() { @@ -321,3 +327,15 @@ void Sp_caches::sp_caches_clear() sp_cache_clear(&sp_package_spec_cache); sp_cache_clear(&sp_package_body_cache); } + +void Sp_caches::sp_caches_empty() +{ + if (sp_proc_cache) + sp_proc_cache->clear(); + if (sp_func_cache) + sp_func_cache->clear(); + if (sp_package_spec_cache) + sp_package_spec_cache->clear(); + if (sp_package_body_cache) + sp_package_body_cache->clear(); +} diff --git a/sql/sql_base.cc b/sql/sql_base.cc index e6a7110d45b..cffe1085302 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -3563,7 +3563,7 @@ open_and_process_routine(THD *thd, Query_tables_list *prelocking_ctx, DBUG_RETURN(TRUE); /* Ensures the routine is up-to-date and cached, if exists. */ - if (rt->sp_cache_routine(thd, has_prelocking_list, &sp)) + if (rt->sp_cache_routine(thd, &sp)) DBUG_RETURN(TRUE); /* Remember the version of the routine in the parse tree. */ @@ -3604,7 +3604,7 @@ open_and_process_routine(THD *thd, Query_tables_list *prelocking_ctx, Validating routine version is unnecessary, since CALL does not affect the prepared statement prelocked list. */ - if (rt->sp_cache_routine(thd, false, &sp)) + if (rt->sp_cache_routine(thd, &sp)) DBUG_RETURN(TRUE); } } @@ -5566,13 +5566,23 @@ bool lock_tables(THD *thd, TABLE_LIST *tables, uint count, uint flags) *(ptr++)= table->table; } - DEBUG_SYNC(thd, "before_lock_tables_takes_lock"); +#ifdef ENABLED_DEBUG_SYNC + if (!tables || + !(strcmp(tables->db.str, "mysql") == 0 && + strcmp(tables->table_name.str, "proc") == 0)) + DEBUG_SYNC(thd, "before_lock_tables_takes_lock"); +#endif if (! (thd->lock= mysql_lock_tables(thd, start, (uint) (ptr - start), flags))) DBUG_RETURN(TRUE); - DEBUG_SYNC(thd, "after_lock_tables_takes_lock"); +#ifdef ENABLED_DEBUG_SYNC + if (!tables || + !(strcmp(tables->db.str, "mysql") == 0 && + strcmp(tables->table_name.str, "proc") == 0)) + DEBUG_SYNC(thd, "after_lock_tables_takes_lock"); +#endif if (thd->lex->requires_prelocking() && thd->lex->sql_command != SQLCOM_LOCK_TABLES) diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 179e2a1a9a5..4f7759716dc 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -848,6 +848,7 @@ THD::THD(my_thread_id id, bool is_wsrep_applier) prepare_derived_at_open= FALSE; create_tmp_table_for_derived= FALSE; save_prep_leaf_list= FALSE; + reset_sp_cache= false; org_charset= 0; /* Restore THR_THD */ set_current_thd(old_THR_THD); diff --git a/sql/sql_class.h b/sql/sql_class.h index e849b21b95b..a6c56a6e023 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -1,5 +1,4 @@ /* - Copyright (c) 2000, 2016, Oracle and/or its affiliates. Copyright (c) 2009, 2022, MariaDB Corporation. This program is free software; you can redistribute it and/or modify @@ -2268,6 +2267,11 @@ public: swap_variables(sp_cache*, sp_package_body_cache, rhs.sp_package_body_cache); } void sp_caches_clear(); + /** + Clear content of sp related caches. + Don't delete cache objects itself. + */ + void sp_caches_empty(); }; @@ -2569,6 +2573,12 @@ public: bool save_prep_leaf_list; + /** + The data member reset_sp_cache is to signal that content of sp_cache + must be reset (all items be removed from it). + */ + bool reset_sp_cache; + /* container for handler's private per-connection data */ Ha_data ha_data[MAX_HA]; diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 7ca63b06c6b..68e5cbc8ca3 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -2402,6 +2402,7 @@ dispatch_end: { WSREP_DEBUG("THD is killed at dispatch_end"); } + wsrep_after_command_before_result(thd); if (wsrep_current_error(thd) && !wsrep_command_no_result(command)) { @@ -2429,6 +2430,12 @@ dispatch_end: #endif /* WITH_WSREP */ + if (thd->reset_sp_cache) + { + thd->sp_caches_empty(); + thd->reset_sp_cache= false; + } + if (do_end_of_statement) { DBUG_ASSERT(thd->derived_tables == NULL && @@ -6071,7 +6078,7 @@ mysql_execute_command(THD *thd) if (sph->sp_resolve_package_routine(thd, thd->lex->sphead, lex->spname, &sph, &pkgname)) return true; - if (sph->sp_cache_routine(thd, lex->spname, false, &sp)) + if (sph->sp_cache_routine(thd, lex->spname, &sp)) goto error; if (!sp || sp->show_routine_code(thd)) { diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 4a4d6f9cca2..d6ccd03cd42 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -2393,6 +2393,7 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, goto err; } table->table= 0; + thd->reset_sp_cache= true; } if ((drop_temporary && if_exists) || !real_table) @@ -2629,8 +2630,11 @@ log_query: } DBUG_PRINT("table", ("table: %p s: %p", table->table, table->table ? table->table->s : NULL)); + if (is_temporary_table(table)) + thd->reset_sp_cache= true; } DEBUG_SYNC(thd, "rm_table_no_locks_before_binlog"); + thd->thread_specific_used= TRUE; error= 0; err: @@ -5236,6 +5240,7 @@ int create_table_impl(THD *thd, const LEX_CSTRING &orig_db, if (is_trans != NULL) *is_trans= table->file->has_transactions(); + thd->reset_sp_cache= true; thd->thread_specific_used= TRUE; create_info->table= table; // Store pointer to table }