From b97083dfc76eac4dab7566786ae875ce8a1415ba Mon Sep 17 00:00:00 2001 From: Alfranio Correia Date: Wed, 18 Mar 2009 10:31:17 +0000 Subject: [PATCH] Bug #42861 Assigning invalid directories to --slave-load-tmpdir crashes the slave Compiling with debug and assigning an invalid directory to --slave-load-tmpdir was crashing the slave due to the following assertion DBUG_ASSERT(! is_set() || can_overwrite_status). This assertion assumes that a thread can change its state once (i.e. ok,error, etc) before aborting, cleaning/resuming or completing its execution unless the overwrite flag (i.e. can_overwrite_status) is true. The Append_block_log_event::do_apply_event which is responsible for creating temporary file(s) was not cleaning the thread state. Thus a failure while trying to create a file in an invalid temporary directory was causing the crash. To fix the problem we check if the temporary directory is valid before starting the SQL Thread and reset the thread state before creating a file in Append_block_log_event::do_apply_event. --- .../suite/rpl/r/rpl_slave_load_in.result | 9 ++++ .../r/rpl_slave_load_remove_tmpfile.result | 15 ++++++ .../r/rpl_slave_load_tmpdir_not_exist.result | 2 + mysql-test/suite/rpl/t/rpl_slave_load_in.test | 19 +++++++- .../t/rpl_slave_load_remove_tmpfile-slave.opt | 1 + .../rpl/t/rpl_slave_load_remove_tmpfile.test | 46 +++++++++++++++++++ .../rpl_slave_load_tmpdir_not_exist-slave.opt | 1 + .../t/rpl_slave_load_tmpdir_not_exist.test | 15 ++++++ sql/log_event.cc | 12 ++++- sql/slave.cc | 43 +++++++++++++++++ 10 files changed, 160 insertions(+), 3 deletions(-) create mode 100644 mysql-test/suite/rpl/r/rpl_slave_load_remove_tmpfile.result create mode 100644 mysql-test/suite/rpl/r/rpl_slave_load_tmpdir_not_exist.result create mode 100644 mysql-test/suite/rpl/t/rpl_slave_load_remove_tmpfile-slave.opt create mode 100644 mysql-test/suite/rpl/t/rpl_slave_load_remove_tmpfile.test create mode 100644 mysql-test/suite/rpl/t/rpl_slave_load_tmpdir_not_exist-slave.opt create mode 100644 mysql-test/suite/rpl/t/rpl_slave_load_tmpdir_not_exist.test diff --git a/mysql-test/suite/rpl/r/rpl_slave_load_in.result b/mysql-test/suite/rpl/r/rpl_slave_load_in.result index 0685d024f26..2cc83fd0a19 100644 --- a/mysql-test/suite/rpl/r/rpl_slave_load_in.result +++ b/mysql-test/suite/rpl/r/rpl_slave_load_in.result @@ -5,6 +5,15 @@ reset slave; drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9; start slave; create table t1(a int not null auto_increment, b int, primary key(a)); +create table t2(a int not null auto_increment, b int, primary key(a)) engine=innodb; load data infile '../../std_data/rpl_loaddata.dat' into table t1; +start transaction; +insert into t2(b) values (1); +insert into t2(b) values (2); +load data infile '../../std_data/rpl_loaddata.dat' into table t2; +load data infile '../../std_data/rpl_loaddata.dat' into table t2; +commit; Comparing tables master:test.t1 and slave:test.t1 +Comparing tables master:test.t2 and slave:test.t2 drop table t1; +drop table t2; diff --git a/mysql-test/suite/rpl/r/rpl_slave_load_remove_tmpfile.result b/mysql-test/suite/rpl/r/rpl_slave_load_remove_tmpfile.result new file mode 100644 index 00000000000..abb6a598c53 --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_slave_load_remove_tmpfile.result @@ -0,0 +1,15 @@ +stop slave; +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9; +reset master; +reset slave; +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9; +start slave; +create table t1(a int not null auto_increment, b int, primary key(a)) engine=innodb; +start transaction; +insert into t1(b) values (1); +insert into t1(b) values (2); +load data infile '../../std_data/rpl_loaddata.dat' into table t1; +commit; +Error in Begin_load_query event: write to '../../tmp/SQL_LOAD-2-1-1.data' failed +drop table t1; +drop table t1; diff --git a/mysql-test/suite/rpl/r/rpl_slave_load_tmpdir_not_exist.result b/mysql-test/suite/rpl/r/rpl_slave_load_tmpdir_not_exist.result new file mode 100644 index 00000000000..4cd2cfd33d7 --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_slave_load_tmpdir_not_exist.result @@ -0,0 +1,2 @@ +start slave; +Unable to use slave's temporary directory ../../../error - Can't read dir of '../../../error' (Errcode: 2) diff --git a/mysql-test/suite/rpl/t/rpl_slave_load_in.test b/mysql-test/suite/rpl/t/rpl_slave_load_in.test index f920c136d43..54ebdffce69 100644 --- a/mysql-test/suite/rpl/t/rpl_slave_load_in.test +++ b/mysql-test/suite/rpl/t/rpl_slave_load_in.test @@ -3,10 +3,11 @@ # event while the "--secure-file-priv" option is set. # # The test is divided in two steps: -# 1 - Creates a table and populates it through "LOAD DATA INFILE". +# 1 - Creates tables and populates them through "LOAD DATA INFILE". # 2 - Compares the master and slave. ########################################################################## -source include/master-slave.inc; +--source include/have_innodb.inc +--source include/master-slave.inc ########################################################################## # Loading data @@ -14,8 +15,17 @@ source include/master-slave.inc; connection master; create table t1(a int not null auto_increment, b int, primary key(a)); +create table t2(a int not null auto_increment, b int, primary key(a)) engine=innodb; + load data infile '../../std_data/rpl_loaddata.dat' into table t1; +start transaction; + insert into t2(b) values (1); + insert into t2(b) values (2); + load data infile '../../std_data/rpl_loaddata.dat' into table t2; + load data infile '../../std_data/rpl_loaddata.dat' into table t2; +commit; + ########################################################################## # Checking Consistency ########################################################################## @@ -25,11 +35,16 @@ let $diff_table_1=master:test.t1; let $diff_table_2=slave:test.t1; source include/diff_tables.inc; +let $diff_table_1=master:test.t2; +let $diff_table_2=slave:test.t2; +source include/diff_tables.inc; + ########################################################################## # Clean up ########################################################################## connection master; drop table t1; +drop table t2; sync_slave_with_master; diff --git a/mysql-test/suite/rpl/t/rpl_slave_load_remove_tmpfile-slave.opt b/mysql-test/suite/rpl/t/rpl_slave_load_remove_tmpfile-slave.opt new file mode 100644 index 00000000000..51e410f911f --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_slave_load_remove_tmpfile-slave.opt @@ -0,0 +1 @@ +--loose-debug=d,remove_slave_load_file_before_write diff --git a/mysql-test/suite/rpl/t/rpl_slave_load_remove_tmpfile.test b/mysql-test/suite/rpl/t/rpl_slave_load_remove_tmpfile.test new file mode 100644 index 00000000000..be7741f2e4b --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_slave_load_remove_tmpfile.test @@ -0,0 +1,46 @@ +########################################################################## +# This test verifies if the slave fails gracefully when the temporary +# file used to load data is removed while it is about to be used it. +# Similar errors are caught if the temporary directory is removed. +# +# Steps: +# 1 - Creates a table and populates it through "LOAD DATA INFILE". +# 2 - Catches error. +########################################################################## +--source include/have_binlog_format_mixed_or_statement.inc +--source include/have_innodb.inc +--source include/have_debug.inc +--source include/master-slave.inc + +########################################################################## +# Loading data +########################################################################## +connection master; + +create table t1(a int not null auto_increment, b int, primary key(a)) engine=innodb; + +start transaction; + insert into t1(b) values (1); + insert into t1(b) values (2); + load data infile '../../std_data/rpl_loaddata.dat' into table t1; +commit; + +########################################################################## +# Catch Error +########################################################################## +connection slave; +source include/wait_for_slave_sql_to_stop.inc; + +let $error=query_get_value("show slave status", Last_SQL_Error, 1); +echo $error; + +########################################################################## +# Clean up +########################################################################## +connection master; + +drop table t1; + +connection slave; + +drop table t1; diff --git a/mysql-test/suite/rpl/t/rpl_slave_load_tmpdir_not_exist-slave.opt b/mysql-test/suite/rpl/t/rpl_slave_load_tmpdir_not_exist-slave.opt new file mode 100644 index 00000000000..c4f91e97e3e --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_slave_load_tmpdir_not_exist-slave.opt @@ -0,0 +1 @@ +--slave-load-tmpdir=../../../error diff --git a/mysql-test/suite/rpl/t/rpl_slave_load_tmpdir_not_exist.test b/mysql-test/suite/rpl/t/rpl_slave_load_tmpdir_not_exist.test new file mode 100644 index 00000000000..dbdb8e79e4e --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_slave_load_tmpdir_not_exist.test @@ -0,0 +1,15 @@ +########################################################################## +# This test verifies if the start slave fails gracefuly when an +# invalid directory is used to set --slave-load-tmpdir. +########################################################################## +connect (master,127.0.0.1,root,,test,$MASTER_MYPORT,); +connect (master1,127.0.0.1,root,,test,$MASTER_MYPORT,); +connect (slave,127.0.0.1,root,,test,$SLAVE_MYPORT,); +connect (slave1,127.0.0.1,root,,test,$SLAVE_MYPORT,); + +connection slave; +start slave; + +source include/wait_for_slave_sql_to_stop.inc; +let $error=query_get_value("show slave status", Last_SQL_Error, 1); +echo $error; diff --git a/sql/log_event.cc b/sql/log_event.cc index 1941a304e8e..1cfbcfc2e0e 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -382,7 +382,7 @@ static void cleanup_load_tmpdir() uint i; char fname[FN_REFLEN], prefbuf[31], *p; - if (!(dirp=my_dir(slave_load_tmpdir,MYF(MY_WME)))) + if (!(dirp=my_dir(slave_load_tmpdir,MYF(0)))) return; /* @@ -6178,6 +6178,12 @@ int Append_block_log_event::do_apply_event(Relay_log_info const *rli) thd_proc_info(thd, proc_info); if (get_create_or_append()) { + /* + Usually lex_start() is called by mysql_parse(), but we need it here + as the present method does not call mysql_parse(). + */ + lex_start(thd); + mysql_reset_thd_for_next_command(thd); my_delete(fname, MYF(0)); // old copy may exist already if ((fd= my_create(fname, CREATE_MODE, O_WRONLY | O_BINARY | O_EXCL | O_NOFOLLOW, @@ -6197,6 +6203,10 @@ int Append_block_log_event::do_apply_event(Relay_log_info const *rli) get_type_str(), fname); goto err; } + + DBUG_EXECUTE_IF("remove_slave_load_file_before_write", + my_close(fd,MYF(0)); fd= -1; my_delete(fname, MYF(0));); + if (my_write(fd, (uchar*) block, block_len, MYF(MY_WME+MY_NABP))) { rli->report(ERROR_LEVEL, my_errno, diff --git a/sql/slave.cc b/sql/slave.cc index 22c61b3ec6c..8c29cb8a72f 100644 --- a/sql/slave.cc +++ b/sql/slave.cc @@ -2632,6 +2632,41 @@ err: DBUG_RETURN(0); // Can't return anything here } +/* + Check the temporary directory used by commands like + LOAD DATA INFILE. + */ +static +int check_temp_dir(char* tmp_dir, char *tmp_file) +{ + int fd; + MY_DIR *dirp; + + DBUG_ENTER("check_temp_dir"); + + /* + Check if the directory exists. + */ + if (!(dirp=my_dir(tmp_dir,MYF(MY_WME)))) + DBUG_RETURN(1); + my_dirend(dirp); + + /* + Check permissions to create a file. + */ + if ((fd= my_create(tmp_file, CREATE_MODE, + O_WRONLY | O_BINARY | O_EXCL | O_NOFOLLOW, + MYF(MY_WME))) < 0) + DBUG_RETURN(1); + + /* + Clean up. + */ + my_close(fd, MYF(0)); + my_delete(tmp_file, MYF(0)); + + DBUG_RETURN(0); +} /** Slave SQL thread entry point. @@ -2763,6 +2798,14 @@ log '%s' at position %s, relay log '%s' position: %s", RPL_LOG_NAME, llstr(rli->group_master_log_pos,llbuff),rli->group_relay_log_name, llstr(rli->group_relay_log_pos,llbuff1)); + if (check_temp_dir(slave_load_tmpdir, rli->slave_patternload_file)) + { + rli->report(ERROR_LEVEL, thd->main_da.sql_errno(), + "Unable to use slave's temporary directory %s - %s", + slave_load_tmpdir, thd->main_da.message()); + goto err; + } + /* execute init_slave variable */ if (sys_init_slave.value_length) {