Skip to content

Commit

Permalink
MDEV-33218: Assertion `active_arena->is_stmt_prepare_or_first_stmt_ex…
Browse files Browse the repository at this point in the history
…ecute() || 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 85f2e4f
(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
  • Loading branch information
dmitryshulga committed Mar 14, 2024
1 parent 0a6f469 commit d7758de
Show file tree
Hide file tree
Showing 14 changed files with 200 additions and 34 deletions.
13 changes: 13 additions & 0 deletions mysql-test/main/ps.result
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
12 changes: 12 additions & 0 deletions mysql-test/main/ps.test
Original file line number Diff line number Diff line change
Expand Up @@ -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 #
3 changes: 1 addition & 2 deletions mysql-test/main/sp.result
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 0 additions & 2 deletions mysql-test/main/sp.test
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
49 changes: 49 additions & 0 deletions mysql-test/main/temp_table.result
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
54 changes: 54 additions & 0 deletions mysql-test/main/temp_table.test
Original file line number Diff line number Diff line change
Expand Up @@ -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 #
22 changes: 6 additions & 16 deletions sql/sp.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
);
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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];
Expand All @@ -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);
}


Expand All @@ -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.
Expand All @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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)
{
Expand Down Expand Up @@ -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));
}


Expand Down
16 changes: 8 additions & 8 deletions sql/sp.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,10 @@ class Sp_handler
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,
Expand Down Expand Up @@ -202,7 +202,7 @@ class Sp_handler
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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -632,7 +632,7 @@ class Sroutine_hash_entry

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;
};


Expand Down
18 changes: 18 additions & 0 deletions sql/sp_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ class sp_cache

/* All routines in this cache */
HASH m_hashtable;
public:
void clear();
}; // class sp_cache

#ifdef HAVE_PSI_INTERFACE
Expand Down Expand Up @@ -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()
{
Expand All @@ -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();
}
Loading

0 comments on commit d7758de

Please sign in to comment.