summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--mysql-test/r/lock.result14
-rw-r--r--mysql-test/t/lock.test14
-rw-r--r--sql/sql_admin.cc4
-rw-r--r--sql/sql_base.cc77
-rw-r--r--sql/sql_class.h10
-rw-r--r--sql/sql_parse.cc3
-rw-r--r--sql/sql_partition.cc8
-rw-r--r--sql/sql_plugin.cc2
-rw-r--r--sql/sql_table.cc6
-rw-r--r--sql/sql_udf.cc4
-rw-r--r--sql/table.cc11
-rw-r--r--sql/table.h12
-rw-r--r--sql/temporary_tables.cc2
-rw-r--r--sql/tztime.cc2
-rw-r--r--storage/innobase/handler/ha_innodb.cc2
15 files changed, 136 insertions, 35 deletions
diff --git a/mysql-test/r/lock.result b/mysql-test/r/lock.result
index 339cfcaa441..2801407130d 100644
--- a/mysql-test/r/lock.result
+++ b/mysql-test/r/lock.result
@@ -505,3 +505,17 @@ disconnect con1;
connection default;
UNLOCK TABLES;
DROP TABLE t1, t2;
+#
+# MDEV-21398 Deadlock (server hang) or assertion failure in
+# Diagnostics_area::set_error_status upon ALTER under lock
+#
+CREATE TABLE t1 (a INT) ENGINE=MyISAM;
+LOCK TABLE t1 WRITE, t1 AS t1a READ;
+ALTER TABLE t1 CHANGE COLUMN IF EXISTS x xx INT;
+Warnings:
+Note 1054 Unknown column 'x' in 't1'
+UNLOCK TABLES;
+DROP TABLE t1;
+#
+# End of 10.2 tests
+#
diff --git a/mysql-test/t/lock.test b/mysql-test/t/lock.test
index ff77b4991c0..af4a35a5139 100644
--- a/mysql-test/t/lock.test
+++ b/mysql-test/t/lock.test
@@ -619,3 +619,17 @@ UNLOCK TABLES;
UNLOCK TABLES;
DROP TABLE t1, t2;
+--echo #
+--echo # MDEV-21398 Deadlock (server hang) or assertion failure in
+--echo # Diagnostics_area::set_error_status upon ALTER under lock
+--echo #
+
+CREATE TABLE t1 (a INT) ENGINE=MyISAM;
+LOCK TABLE t1 WRITE, t1 AS t1a READ;
+ALTER TABLE t1 CHANGE COLUMN IF EXISTS x xx INT;
+UNLOCK TABLES;
+DROP TABLE t1;
+
+--echo #
+--echo # End of 10.2 tests
+--echo #
diff --git a/sql/sql_admin.cc b/sql/sql_admin.cc
index 0613495f202..7ace3144dc3 100644
--- a/sql/sql_admin.cc
+++ b/sql/sql_admin.cc
@@ -1087,7 +1087,7 @@ send_result_message:
}
/* Make sure this table instance is not reused after the operation. */
if (table->table)
- table->table->m_needs_reopen= true;
+ table->table->mark_table_for_reopen();
}
result_code= result_code ? HA_ADMIN_FAILED : HA_ADMIN_OK;
table->next_local= save_next_local;
@@ -1212,7 +1212,7 @@ err:
trans_rollback(thd);
if (table && table->table)
{
- table->table->m_needs_reopen= true;
+ table->table->mark_table_for_reopen();
table->table= 0;
}
close_thread_tables(thd); // Shouldn't be needed
diff --git a/sql/sql_base.cc b/sql/sql_base.cc
index 765c743aba2..6c7ef7a85c9 100644
--- a/sql/sql_base.cc
+++ b/sql/sql_base.cc
@@ -2161,9 +2161,9 @@ Locked_tables_list::init_locked_tables(THD *thd)
in reopen_tables(). reopen_tables() is a critical
path and we don't want to complicate it with extra allocations.
*/
- m_reopen_array= (TABLE**)alloc_root(&m_locked_tables_root,
- sizeof(TABLE*) *
- (m_locked_tables_count+1));
+ m_reopen_array= (TABLE_LIST**)alloc_root(&m_locked_tables_root,
+ sizeof(TABLE_LIST*) *
+ (m_locked_tables_count+1));
if (m_reopen_array == NULL)
{
reset();
@@ -2273,6 +2273,7 @@ void Locked_tables_list::reset()
m_locked_tables_last= &m_locked_tables;
m_reopen_array= NULL;
m_locked_tables_count= 0;
+ some_table_marked_for_reopen= 0;
}
@@ -2368,7 +2369,7 @@ unlink_all_closed_tables(THD *thd, MYSQL_LOCK *lock, size_t reopen_count)
in reopen_tables() always links the opened table
to the beginning of the open_tables list.
*/
- DBUG_ASSERT(thd->open_tables == m_reopen_array[reopen_count]);
+ DBUG_ASSERT(thd->open_tables == m_reopen_array[reopen_count]->table);
thd->open_tables->pos_in_locked_tables->table= NULL;
thd->open_tables->pos_in_locked_tables= 0;
@@ -2398,10 +2399,36 @@ unlink_all_closed_tables(THD *thd, MYSQL_LOCK *lock, size_t reopen_count)
}
+/*
+ Mark all instances of the table to be reopened
+
+ This is only needed when LOCK TABLES is active
+*/
+
+void Locked_tables_list::mark_table_for_reopen(THD *thd, TABLE *table)
+{
+ TABLE_SHARE *share= table->s;
+
+ for (TABLE_LIST *table_list= m_locked_tables;
+ table_list; table_list= table_list->next_global)
+ {
+ if (table_list->table->s == share)
+ table_list->table->internal_set_needs_reopen(true);
+ }
+ /* This is needed in the case where lock tables where not used */
+ table->internal_set_needs_reopen(true);
+ some_table_marked_for_reopen= 1;
+}
+
+
/**
Reopen the tables locked with LOCK TABLES and temporarily closed
by a DDL statement or FLUSH TABLES.
+ @param need_reopen If set, reopen open tables that are marked with
+ for reopen.
+ If not set, reopen tables that where closed.
+
@note This function is a no-op if we're not under LOCK TABLES.
@return TRUE if an error reopening the tables. May happen in
@@ -2419,6 +2446,12 @@ Locked_tables_list::reopen_tables(THD *thd, bool need_reopen)
MYSQL_LOCK *merged_lock;
DBUG_ENTER("Locked_tables_list::reopen_tables");
+ DBUG_ASSERT(some_table_marked_for_reopen || !need_reopen);
+
+
+ /* Reset flag that some table was marked for reopen */
+ some_table_marked_for_reopen= 0;
+
for (TABLE_LIST *table_list= m_locked_tables;
table_list; table_list= table_list->next_global)
{
@@ -2442,24 +2475,32 @@ Locked_tables_list::reopen_tables(THD *thd, bool need_reopen)
else
{
if (table_list->table) /* The table was not closed */
- continue;
- }
-
- /* Links into thd->open_tables upon success */
- if (open_table(thd, table_list, &ot_ctx))
- {
- unlink_all_closed_tables(thd, 0, reopen_count);
- DBUG_RETURN(TRUE);
+ continue;
}
- table_list->table->pos_in_locked_tables= table_list;
- /* See also the comment on lock type in init_locked_tables(). */
- table_list->table->reginfo.lock_type= table_list->lock_type;
DBUG_ASSERT(reopen_count < m_locked_tables_count);
- m_reopen_array[reopen_count++]= table_list->table;
+ m_reopen_array[reopen_count++]= table_list;
}
if (reopen_count)
{
+ TABLE **tables= (TABLE**) my_alloca(reopen_count * sizeof(TABLE*));
+
+ for (uint i= 0 ; i < reopen_count ; i++)
+ {
+ TABLE_LIST *table_list= m_reopen_array[i];
+ /* Links into thd->open_tables upon success */
+ if (open_table(thd, table_list, &ot_ctx))
+ {
+ unlink_all_closed_tables(thd, 0, i);
+ my_afree((void*) tables);
+ DBUG_RETURN(TRUE);
+ }
+ tables[i]= table_list->table;
+ table_list->table->pos_in_locked_tables= table_list;
+ /* See also the comment on lock type in init_locked_tables(). */
+ table_list->table->reginfo.lock_type= table_list->lock_type;
+ }
+
thd->in_lock_tables= 1;
/*
We re-lock all tables with mysql_lock_tables() at once rather
@@ -2472,7 +2513,7 @@ Locked_tables_list::reopen_tables(THD *thd, bool need_reopen)
works fine. Patching legacy code of thr_lock.c is risking to
break something else.
*/
- lock= mysql_lock_tables(thd, m_reopen_array, reopen_count,
+ lock= mysql_lock_tables(thd, tables, reopen_count,
MYSQL_OPEN_REOPEN);
thd->in_lock_tables= 0;
if (lock == NULL || (merged_lock=
@@ -2481,9 +2522,11 @@ Locked_tables_list::reopen_tables(THD *thd, bool need_reopen)
unlink_all_closed_tables(thd, lock, reopen_count);
if (! thd->killed)
my_error(ER_LOCK_DEADLOCK, MYF(0));
+ my_afree((void*) tables);
DBUG_RETURN(TRUE);
}
thd->lock= merged_lock;
+ my_afree((void*) tables);
}
DBUG_RETURN(FALSE);
}
diff --git a/sql/sql_class.h b/sql/sql_class.h
index 111c407f117..80aa5710b62 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -1823,20 +1823,23 @@ private:
TABLE_LIST *m_locked_tables;
TABLE_LIST **m_locked_tables_last;
/** An auxiliary array used only in reopen_tables(). */
- TABLE **m_reopen_array;
+ TABLE_LIST **m_reopen_array;
/**
Count the number of tables in m_locked_tables list. We can't
rely on thd->lock->table_count because it excludes
non-transactional temporary tables. We need to know
an exact number of TABLE objects.
*/
- size_t m_locked_tables_count;
+ uint m_locked_tables_count;
public:
+ bool some_table_marked_for_reopen;
+
Locked_tables_list()
:m_locked_tables(NULL),
m_locked_tables_last(&m_locked_tables),
m_reopen_array(NULL),
- m_locked_tables_count(0)
+ m_locked_tables_count(0),
+ some_table_marked_for_reopen(0)
{
init_sql_alloc(&m_locked_tables_root, MEM_ROOT_BLOCK_SIZE, 0,
MYF(MY_THREAD_SPECIFIC));
@@ -1859,6 +1862,7 @@ public:
bool restore_lock(THD *thd, TABLE_LIST *dst_table_list, TABLE *table,
MYSQL_LOCK *lock);
void add_back_last_deleted_lock(TABLE_LIST *dst_table_list);
+ void mark_table_for_reopen(THD *thd, TABLE *table);
};
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index 13576daae98..f1c9479adc7 100644
--- a/sql/sql_parse.cc
+++ b/sql/sql_parse.cc
@@ -5998,7 +5998,8 @@ finish:
lex->unit.cleanup();
/* close/reopen tables that were marked to need reopen under LOCK TABLES */
- if (! thd->lex->requires_prelocking())
+ if (unlikely(thd->locked_tables_list.some_table_marked_for_reopen) &&
+ !thd->lex->requires_prelocking())
thd->locked_tables_list.reopen_tables(thd, true);
if (! thd->in_sub_stmt)
diff --git a/sql/sql_partition.cc b/sql/sql_partition.cc
index 4a200dd670b..071c9c05129 100644
--- a/sql/sql_partition.cc
+++ b/sql/sql_partition.cc
@@ -4592,7 +4592,7 @@ uint prep_alter_part_table(THD *thd, TABLE *table, Alter_info *alter_info,
{
*fast_alter_table= true;
/* Force table re-open for consistency with the main case. */
- table->m_needs_reopen= true;
+ table->mark_table_for_reopen();
}
else
{
@@ -4640,7 +4640,7 @@ uint prep_alter_part_table(THD *thd, TABLE *table, Alter_info *alter_info,
must be reopened.
*/
*fast_alter_table= true;
- table->m_needs_reopen= true;
+ table->mark_table_for_reopen();
}
else
{
@@ -6418,7 +6418,7 @@ void handle_alter_part_error(ALTER_PARTITION_PARAM_TYPE *lpt,
THD *thd= lpt->thd;
TABLE *table= lpt->table;
DBUG_ENTER("handle_alter_part_error");
- DBUG_ASSERT(table->m_needs_reopen);
+ DBUG_ASSERT(table->needs_reopen());
if (close_table)
{
@@ -6637,7 +6637,7 @@ uint fast_alter_partition_table(THD *thd, TABLE *table,
bool frm_install= FALSE;
MDL_ticket *mdl_ticket= table->mdl_ticket;
DBUG_ENTER("fast_alter_partition_table");
- DBUG_ASSERT(table->m_needs_reopen);
+ DBUG_ASSERT(table->needs_reopen());
part_info= table->part_info;
lpt->thd= thd;
diff --git a/sql/sql_plugin.cc b/sql/sql_plugin.cc
index 44b2a8220b3..42d2d110f2b 100644
--- a/sql/sql_plugin.cc
+++ b/sql/sql_plugin.cc
@@ -1844,7 +1844,7 @@ static void plugin_load(MEM_ROOT *tmp_root)
sql_print_error(ER_THD(new_thd, ER_GET_ERRNO), my_errno,
table->file->table_type());
end_read_record(&read_record_info);
- table->m_needs_reopen= TRUE; // Force close to free memory
+ table->mark_table_for_reopen();
close_mysql_tables(new_thd);
end:
delete new_thd;
diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index e53d51777b4..8d81911bf29 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -7783,7 +7783,8 @@ mysql_prepare_alter_table(THD *thd, TABLE *table,
if (field->default_value)
field->default_value->expr->walk(&Item::rename_fields_processor, 1,
&column_rename_param);
- table->m_needs_reopen= 1; // because new column name is on thd->mem_root
+ // Force reopen because new column name is on thd->mem_root
+ table->mark_table_for_reopen();
}
/* Check if field is changed */
@@ -8195,7 +8196,8 @@ mysql_prepare_alter_table(THD *thd, TABLE *table,
{
check->expr->walk(&Item::rename_fields_processor, 1,
&column_rename_param);
- table->m_needs_reopen= 1; // because new column name is on thd->mem_root
+ // Force reopen because new column name is on thd->mem_root
+ table->mark_table_for_reopen();
}
new_constraint_list.push_back(check, thd->mem_root);
}
diff --git a/sql/sql_udf.cc b/sql/sql_udf.cc
index 2a72c86707c..9099345a64c 100644
--- a/sql/sql_udf.cc
+++ b/sql/sql_udf.cc
@@ -255,7 +255,9 @@ void udf_init()
if (error > 0)
sql_print_error("Got unknown error: %d", my_errno);
end_read_record(&read_record_info);
- table->m_needs_reopen= TRUE; // Force close to free memory
+
+ // Force close to free memory
+ table->mark_table_for_reopen();
end:
close_mysql_tables(new_thd);
diff --git a/sql/table.cc b/sql/table.cc
index 995852651d0..611b8d1296f 100644
--- a/sql/table.cc
+++ b/sql/table.cc
@@ -8647,3 +8647,14 @@ void TABLE::initialize_quick_structures()
bzero(quick_costs, sizeof(quick_costs));
bzero(quick_n_ranges, sizeof(quick_n_ranges));
}
+
+/*
+ Mark table to be reopened after query
+*/
+
+void TABLE::mark_table_for_reopen()
+{
+ THD *thd= in_use;
+ DBUG_ASSERT(thd);
+ thd->locked_tables_list.mark_table_for_reopen(thd, this);
+}
diff --git a/sql/table.h b/sql/table.h
index 0cf0e4efe00..ba912438a93 100644
--- a/sql/table.h
+++ b/sql/table.h
@@ -1290,8 +1290,8 @@ public:
bool insert_or_update; /* Can be used by the handler */
bool alias_name_used; /* true if table_name is alias */
bool get_fields_in_item_tree; /* Signal to fix_field */
- bool m_needs_reopen;
private:
+ bool m_needs_reopen;
bool created; /* For tmp tables. TRUE <=> tmp table was actually created.*/
public:
#ifdef HAVE_REPLICATION
@@ -1401,6 +1401,16 @@ public:
/** Should this instance of the table be reopened? */
inline bool needs_reopen()
{ return !db_stat || m_needs_reopen; }
+ /*
+ Mark that all current connection instances of the table should be
+ reopen at end of statement
+ */
+ void mark_table_for_reopen();
+ /* Should only be called from Locked_tables_list::mark_table_for_reopen() */
+ void internal_set_needs_reopen(bool value)
+ {
+ m_needs_reopen= value;
+ }
bool alloc_keys(uint key_count);
bool check_tmp_key(uint key, uint key_parts,
diff --git a/sql/temporary_tables.cc b/sql/temporary_tables.cc
index e2179a71625..005a520ff64 100644
--- a/sql/temporary_tables.cc
+++ b/sql/temporary_tables.cc
@@ -1065,7 +1065,7 @@ TABLE *THD::find_temporary_table(const char *key, uint key_length,
case TMP_TABLE_ANY: found= true; break;
}
}
- if (table && unlikely(table->m_needs_reopen))
+ if (table && unlikely(table->needs_reopen()))
{
share->all_tmp_tables.remove(table);
free_temporary_table(table);
diff --git a/sql/tztime.cc b/sql/tztime.cc
index 941530b9656..f58801bf4b7 100644
--- a/sql/tztime.cc
+++ b/sql/tztime.cc
@@ -1699,7 +1699,7 @@ my_tz_init(THD *org_thd, const char *default_tzname, my_bool bootstrap)
{
tl->table->use_all_columns();
/* Force close at the end of the function to free memory. */
- tl->table->m_needs_reopen= TRUE;
+ tl->table->mark_table_for_reopen();
}
/*
diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc
index 11f6c137b67..ac196f58416 100644
--- a/storage/innobase/handler/ha_innodb.cc
+++ b/storage/innobase/handler/ha_innodb.cc
@@ -3468,7 +3468,7 @@ ha_innobase::reset_template(void)
/* Force table to be freed in close_thread_table(). */
DBUG_EXECUTE_IF("free_table_in_fts_query",
if (m_prebuilt->in_fts_query) {
- table->m_needs_reopen = true;
+ table->mark_table_for_reopen();
}
);
View Lorry Controller Status