Skip to content

M
MariaDB
Commit 16c8298a authored by Mattias Jonsson's avatar Mattias Jonsson
Browse files
Options

Bug#42438: Crash ha_partition::change_table_ptr 

There was two problems:
The first was the symptom, caused by bad error handling in
ha_partition. It did not handle print_error etc. when
having no partitions (when used by dummy handler).

The second was the real problem that when dropping tables
it reused the table type (storage engine) from when the lock
was asked for, not the table type that it had when gaining
the exclusive name lock. So that it tried to delete tables
from wrong storage engines.

Solutions for the first problem was to accept some handler
calls to the partitioning handler even if it was not setup
with any partitions, and also if possible fallback
to use the base handler's default functions.

Solution for the second problem was to remove the optimization
to reuse the definition from the cache, instead always check
the frm-file when holding the LOCK_open mutex

(updated with a fix for a debug print crash and better
comments as required by reviewer, and removed optimization
to avoid reading the frm-file).

mysql-test/r/partition_debug_sync.result:
  Bug#42438: Crash ha_partition::change_table_ptr
  
  New result file using DEBUG_SYNC for deterministic results.
mysql-test/t/partition_debug_sync.test:
  Bug#42438: Crash ha_partition::change_table_ptr
  
  New test file using DEBUG_SYNC for deterministic results.
sql/ha_partition.cc:
  Bug#42438: Crash ha_partition::change_table_ptr
  
  allow some handler calls, used by error handling, even when
  no partitions are setup. Fallback to default handling if possible.
sql/sql_base.cc:
  Bug#42438: Crash ha_partition::change_table_ptr
  
  Added DEBUG_SYNC point for deterministic test cases.
sql/sql_table.cc:
  Bug#42438: Crash ha_partition::change_table_ptr
  
  Always use the table type written in the .frm-file
  (i.e. the current table type) when deleting a table.
  
  Moved the check for log-table to not depend of the cache.
  
  Added DEBUG_SYNC points for deterministic test cases.
parent d7abca9a
Hide whitespace changes
Inline Side-by-side
Showing with 198 additions and 37 deletions
mysql-test/r/partition_debug_sync.result 0 → 100644
View file @ 16c8298a
DROP TABLE IF EXISTS t1, t2;
SET DEBUG_SYNC= 'RESET';
#
# Bug#42438: Crash ha_partition::change_table_ptr
# Test when remove partitioning is done while drop table is waiting
# for the table.
# Con 1
SET DEBUG_SYNC= 'RESET';
CREATE TABLE t1
(a INTEGER,
b INTEGER NOT NULL,
KEY (b))
ENGINE = MYISAM
/*!50100 PARTITION BY RANGE (a)
(PARTITION p0 VALUES LESS THAN (2),
PARTITION p1 VALUES LESS THAN (20),
PARTITION p2 VALUES LESS THAN (100),
PARTITION p3 VALUES LESS THAN MAXVALUE ) */;
SET DEBUG_SYNC= 'alter_table_before_create_table_no_lock SIGNAL removing_partitioning WAIT_FOR waiting_for_alter';
SET DEBUG_SYNC= 'alter_table_before_main_binlog SIGNAL partitioning_removed';
ALTER TABLE t1 REMOVE PARTITIONING;
# Con default
SET DEBUG_SYNC= 'now WAIT_FOR removing_partitioning';
SET DEBUG_SYNC= 'waiting_for_table SIGNAL waiting_for_alter';
SET DEBUG_SYNC= 'rm_table_part2_before_delete_table WAIT_FOR partitioning_removed';
DROP TABLE IF EXISTS t1;
# Con 1
SET DEBUG_SYNC= 'RESET';
SET DEBUG_SYNC= 'RESET';
#
# Bug#42438: Crash ha_partition::change_table_ptr
# Test when remove partitioning is failing due to drop table is already
# in progress.
CREATE TABLE t2
(a INTEGER,
b INTEGER NOT NULL,
KEY (b))
ENGINE = MYISAM
/*!50100 PARTITION BY RANGE (a)
(PARTITION p0 VALUES LESS THAN (2),
PARTITION p1 VALUES LESS THAN (20),
PARTITION p2 VALUES LESS THAN (100),
PARTITION p3 VALUES LESS THAN MAXVALUE ) */;
SET DEBUG_SYNC= 'before_lock_tables_takes_lock SIGNAL removing_partitions WAIT_FOR waiting_for_alter';
SET DEBUG_SYNC= 'alter_table_before_rename_result_table WAIT_FOR delete_done';
ALTER TABLE t2 REMOVE PARTITIONING;
# Con default
SET DEBUG_SYNC= 'now WAIT_FOR removing_partitions';
SET DEBUG_SYNC= 'waiting_for_table SIGNAL waiting_for_alter';
SET DEBUG_SYNC= 'rm_table_part2_before_binlog SIGNAL delete_done';
DROP TABLE IF EXISTS t2;
# Con 1
ERROR 42S02: Table 'test.t2' doesn't exist
SET DEBUG_SYNC= 'RESET';
# Con default
SET DEBUG_SYNC= 'RESET';
End of 5.1 tests
mysql-test/t/partition_debug_sync.test 0 → 100644
View file @ 16c8298a
#--disable_abort_on_error
#
# Test for the partition storage engine which require DEBUG_SYNC feature to
# Created by Mattias Jonsson
#
--source include/have_partition.inc
--source include/have_debug_sync.inc
--disable_warnings
DROP TABLE IF EXISTS t1, t2;
SET DEBUG_SYNC= 'RESET';
--enable_warnings
--echo #
--echo # Bug#42438: Crash ha_partition::change_table_ptr
--echo # Test when remove partitioning is done while drop table is waiting
--echo # for the table.
connect(con1, localhost, root,,);
--echo # Con 1
SET DEBUG_SYNC= 'RESET';
CREATE TABLE t1
(a INTEGER,
b INTEGER NOT NULL,
KEY (b))
ENGINE = MYISAM
/*!50100 PARTITION BY RANGE (a)
(PARTITION p0 VALUES LESS THAN (2),
PARTITION p1 VALUES LESS THAN (20),
PARTITION p2 VALUES LESS THAN (100),
PARTITION p3 VALUES LESS THAN MAXVALUE ) */;
SET DEBUG_SYNC= 'alter_table_before_create_table_no_lock SIGNAL removing_partitioning WAIT_FOR waiting_for_alter';
SET DEBUG_SYNC= 'alter_table_before_main_binlog SIGNAL partitioning_removed';
--send ALTER TABLE t1 REMOVE PARTITIONING
connection default;
--echo # Con default
SET DEBUG_SYNC= 'now WAIT_FOR removing_partitioning';
SET DEBUG_SYNC= 'waiting_for_table SIGNAL waiting_for_alter';
SET DEBUG_SYNC= 'rm_table_part2_before_delete_table WAIT_FOR partitioning_removed';
DROP TABLE IF EXISTS t1;
--echo # Con 1
connection con1;
--reap
connection default;
SET DEBUG_SYNC= 'RESET';
connection con1;
SET DEBUG_SYNC= 'RESET';
--echo #
--echo # Bug#42438: Crash ha_partition::change_table_ptr
--echo # Test when remove partitioning is failing due to drop table is already
--echo # in progress.
CREATE TABLE t2
(a INTEGER,
b INTEGER NOT NULL,
KEY (b))
ENGINE = MYISAM
/*!50100 PARTITION BY RANGE (a)
(PARTITION p0 VALUES LESS THAN (2),
PARTITION p1 VALUES LESS THAN (20),
PARTITION p2 VALUES LESS THAN (100),
PARTITION p3 VALUES LESS THAN MAXVALUE ) */;
SET DEBUG_SYNC= 'before_lock_tables_takes_lock SIGNAL removing_partitions WAIT_FOR waiting_for_alter';
SET DEBUG_SYNC= 'alter_table_before_rename_result_table WAIT_FOR delete_done';
--send ALTER TABLE t2 REMOVE PARTITIONING
connection default;
--echo # Con default
SET DEBUG_SYNC= 'now WAIT_FOR removing_partitions';
SET DEBUG_SYNC= 'waiting_for_table SIGNAL waiting_for_alter';
SET DEBUG_SYNC= 'rm_table_part2_before_binlog SIGNAL delete_done';
DROP TABLE IF EXISTS t2;
--echo # Con 1
connection con1;
--error ER_NO_SUCH_TABLE
--reap
SET DEBUG_SYNC= 'RESET';
disconnect con1;
connection default;
--echo # Con default
SET DEBUG_SYNC= 'RESET';
--echo End of 5.1 tests
sql/ha_partition.cc
View file @ 16c8298a
......@@ -1719,13 +1719,23 @@ void ha_partition::update_create_info(HA_CREATE_INFO *create_info)
void ha_partition::change_table_ptr(TABLE *table_arg, TABLE_SHARE *share)
{
handler **file_array= m_file;
handler **file_array;
table= table_arg;
table_share= share;
do
/*
m_file can be NULL when using an old cached table in DROP TABLE, when the
table just has REMOVED PARTITIONING, see Bug#42438
*/
if (m_file)
{
(*file_array)->change_table_ptr(table_arg, share);
} while (*(++file_array));
file_array= m_file;
DBUG_ASSERT(*file_array);
do
{
(*file_array)->change_table_ptr(table_arg, share);
} while (*(++file_array));
}
if (m_added_file && m_added_file[0])
{
/* if in middle of a drop/rename etc */
......@@ -5986,7 +5996,13 @@ void ha_partition::print_error(int error, myf errflag)
if (error == HA_ERR_NO_PARTITION_FOUND)
m_part_info->print_no_partition_found(table);
else
m_file[m_last_part]->print_error(error, errflag);
{
/* In case m_file has not been initialized, like in bug#42438 */
if (m_file)
m_file[m_last_part]->print_error(error, errflag);
else
handler::print_error(error, errflag);
}
DBUG_VOID_RETURN;
}
......@@ -5996,7 +6012,12 @@ bool ha_partition::get_error_message(int error, String *buf)
DBUG_ENTER("ha_partition::get_error_message");
/* Should probably look for my own errors first */
DBUG_RETURN(m_file[m_last_part]->get_error_message(error, buf));
/* In case m_file has not been initialized, like in bug#42438 */
if (m_file)
DBUG_RETURN(m_file[m_last_part]->get_error_message(error, buf));
DBUG_RETURN(handler::get_error_message(error, buf));
}
......
sql/sql_base.cc
View file @ 16c8298a
......@@ -2165,6 +2165,7 @@ void wait_for_condition(THD *thd, pthread_mutex_t *mutex, pthread_cond_t *cond)
proc_info=thd->proc_info;
thd_proc_info(thd, "Waiting for table");
DBUG_ENTER("wait_for_condition");
DEBUG_SYNC(thd, "waiting_for_table");
if (!thd->killed)
(void) pthread_cond_wait(cond, mutex);
......
sql/sql_table.cc
View file @ 16c8298a
......@@ -22,6 +22,9 @@
#include "sp_head.h"
#include "sql_trigger.h"
#include "sql_show.h"
#if defined(ENABLED_DEBUG_SYNC)
#include "debug_sync.h"
#endif
#ifdef __WIN__
#include <io.h>
......@@ -1870,30 +1873,6 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists,
pthread_mutex_lock(&LOCK_open);
/*
If we have the table in the definition cache, we don't have to check the
.frm file to find if the table is a normal table (not view) and what
engine to use.
*/
for (table= tables; table; table= table->next_local)
{
TABLE_SHARE *share;
table->db_type= NULL;
if ((share= get_cached_table_share(table->db, table->table_name)))
table->db_type= share->db_type();
/* Disable drop of enabled log tables */
if (share && (share->table_category == TABLE_CATEGORY_PERFORMANCE) &&
check_if_log_table(table->db_length, table->db,
table->table_name_length, table->table_name, 1))
{
my_error(ER_BAD_LOG_STATEMENT, MYF(0), "DROP");
pthread_mutex_unlock(&LOCK_open);
DBUG_RETURN(1);
}
}
if (!drop_temporary && lock_table_names_exclusively(thd, tables))
{
pthread_mutex_unlock(&LOCK_open);
......@@ -1904,7 +1883,7 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists,
{
char *db=table->db;
handlerton *table_type;
enum legacy_db_type frm_db_type;
enum legacy_db_type frm_db_type= DB_TYPE_UNKNOWN;
DBUG_PRINT("table", ("table_l: '%s'.'%s' table: 0x%lx s: 0x%lx",
table->db, table->table_name, (long) table->table,
......@@ -1945,6 +1924,15 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists,
error= 0;
}
/* Disable drop of enabled log tables */
if (check_if_log_table(table->db_length, table->db,
table->table_name_length, table->table_name, 1))
{
my_error(ER_BAD_LOG_STATEMENT, MYF(0), "DROP");
pthread_mutex_unlock(&LOCK_open);
DBUG_RETURN(1);
}
/*
If row-based replication is used and the table is not a
temporary table, we add the table name to the drop statement
......@@ -1969,7 +1957,6 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists,
built_query.append("`,");
}
table_type= table->db_type;
if (!drop_temporary)
{
TABLE *locked_table;
......@@ -1996,9 +1983,9 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists,
table->internal_tmp_table ?
FN_IS_TMP : 0);
}
DEBUG_SYNC(thd, "rm_table_part2_before_delete_table");
if (drop_temporary ||
((table_type == NULL &&
access(path, F_OK) &&
((access(path, F_OK) &&
ha_create_table_from_engine(thd, db, alias)) ||
(!drop_view &&
mysql_frm_type(thd, path, &frm_db_type) != FRMTYPE_TABLE)))
......@@ -2014,15 +2001,25 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists,
else
{
char *end;
if (table_type == NULL)
/*
Cannot use the db_type from the table, since that might have changed
while waiting for the exclusive name lock. We are under LOCK_open,
so reading from the frm-file is safe.
*/
if (frm_db_type == DB_TYPE_UNKNOWN)
{
mysql_frm_type(thd, path, &frm_db_type);
table_type= ha_resolve_by_legacy_type(thd, frm_db_type);
mysql_frm_type(thd, path, &frm_db_type);
DBUG_PRINT("info", ("frm_db_type %d from %s", frm_db_type, path));
}
table_type= ha_resolve_by_legacy_type(thd, frm_db_type);
// Remove extension for delete
*(end= path + path_length - reg_ext_length)= '\0';
DBUG_PRINT("info", ("deleting table of type %d",
(table_type ? table_type->db_type : 0)));
error= ha_delete_table(thd, table_type, path, db, table->table_name,
!dont_log_query);
/* No error if non existent table and 'IF EXIST' clause or view */
if ((error == ENOENT || error == HA_ERR_NO_SUCH_TABLE) &&
(if_exists || table_type == NULL))
{
......@@ -2062,6 +2059,7 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists,
on the table name.
*/
pthread_mutex_unlock(&LOCK_open);
DEBUG_SYNC(thd, "rm_table_part2_before_binlog");
thd->thread_specific_used|= tmp_table_deleted;
error= 0;
if (wrong_tables.length())
......@@ -7097,6 +7095,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
else
create_info->data_file_name=create_info->index_file_name=0;
DEBUG_SYNC(thd, "alter_table_before_create_table_no_lock");
/*
Create a table with a temporary name.
With create_info->frm_only == 1 this creates a .frm file only.
......@@ -7296,6 +7295,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
intern_close_table(new_table);
my_free(new_table,MYF(0));
}
DEBUG_SYNC(thd, "alter_table_before_rename_result_table");
VOID(pthread_mutex_lock(&LOCK_open));
if (error)
{
......@@ -7438,6 +7438,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
thd_proc_info(thd, "end");
DBUG_EXECUTE_IF("sleep_alter_before_main_binlog", my_sleep(6000000););
DEBUG_SYNC(thd, "alter_table_before_main_binlog");
ha_binlog_log_query(thd, create_info->db_type, LOGCOM_ALTER_TABLE,
thd->query(), thd->query_length(),
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7