Commit 9a714c93 authored by Luis Soares's avatar Luis Soares

BUG#55387: binlog.binlog_tmp_table crashes the server

           sporadically

There are two problems:

1. When closing temporary tables, during the THD clean up - and
   after the session connection was already closed, there is a
   chance we can push an error into the THD diagnostics area, if
   the writing of the implicit DROP event to the binary log fails
   for some reason. As a consequence an assertion can be
   triggered, because at that point the diagnostics area is
   already set.

2. Using push_warning with MYSQL_ERROR::WARN_LEVEL_ERROR is a 
   bug.

Given that close_temporary_tables is mostly called from
THD::cleanup - ie, with the session already closed, we fix
problem #1 by allowing the diagnostics area to be
overwritten. There is one other place in the code that calls
close_temporary_tables - while applying Start_log_event_v3. To
cover that case, we make close_temporary_tables to return the
error, thus, propagating upwards in the stack.

To fix problem #2, we replace push_warning with sql_print_error.

sql/log_event.cc:
  Added handling of error returned by close_temporary_tables to
  Start_log_event_v3::do_apply_event.
sql/sql_base.cc:
  Three changes to close_temporary_tables:
  1. it returns a boolean now (instead of void)
  2. it uses sql_print_error instead of push_warning when writing to
     binary log fails
  3. we set can_overwrite_status before writing to the binary log,
     thence not risking triggering an assertion by any other push
     into diagnostics area happening inside mysql_bin_log.write.
sql/sql_base.h:
  Changed the interface of close_temporary_tables so that it returns
  bool instead of void.
parent 74d67316
...@@ -3719,6 +3719,7 @@ bool Start_log_event_v3::write(IO_CACHE* file) ...@@ -3719,6 +3719,7 @@ bool Start_log_event_v3::write(IO_CACHE* file)
int Start_log_event_v3::do_apply_event(Relay_log_info const *rli) int Start_log_event_v3::do_apply_event(Relay_log_info const *rli)
{ {
DBUG_ENTER("Start_log_event_v3::do_apply_event"); DBUG_ENTER("Start_log_event_v3::do_apply_event");
int error= 0;
switch (binlog_version) switch (binlog_version)
{ {
case 3: case 3:
...@@ -3731,7 +3732,7 @@ int Start_log_event_v3::do_apply_event(Relay_log_info const *rli) ...@@ -3731,7 +3732,7 @@ int Start_log_event_v3::do_apply_event(Relay_log_info const *rli)
*/ */
if (created) if (created)
{ {
close_temporary_tables(thd); error= close_temporary_tables(thd);
cleanup_load_tmpdir(); cleanup_load_tmpdir();
} }
else else
...@@ -3759,7 +3760,7 @@ int Start_log_event_v3::do_apply_event(Relay_log_info const *rli) ...@@ -3759,7 +3760,7 @@ int Start_log_event_v3::do_apply_event(Relay_log_info const *rli)
Can distinguish, based on the value of 'created': this event was Can distinguish, based on the value of 'created': this event was
generated at master startup. generated at master startup.
*/ */
close_temporary_tables(thd); error= close_temporary_tables(thd);
} }
/* /*
Otherwise, can't distinguish a Start_log_event generated at Otherwise, can't distinguish a Start_log_event generated at
...@@ -3771,7 +3772,7 @@ int Start_log_event_v3::do_apply_event(Relay_log_info const *rli) ...@@ -3771,7 +3772,7 @@ int Start_log_event_v3::do_apply_event(Relay_log_info const *rli)
/* this case is impossible */ /* this case is impossible */
DBUG_RETURN(1); DBUG_RETURN(1);
} }
DBUG_RETURN(0); DBUG_RETURN(error);
} }
#endif /* defined(HAVE_REPLICATION) && !defined(MYSQL_CLIENT) */ #endif /* defined(HAVE_REPLICATION) && !defined(MYSQL_CLIENT) */
......
...@@ -1634,7 +1634,7 @@ static inline uint tmpkeyval(THD *thd, TABLE *table) ...@@ -1634,7 +1634,7 @@ static inline uint tmpkeyval(THD *thd, TABLE *table)
creates one DROP TEMPORARY TABLE binlog event for each pseudo-thread creates one DROP TEMPORARY TABLE binlog event for each pseudo-thread
*/ */
void close_temporary_tables(THD *thd) bool close_temporary_tables(THD *thd)
{ {
DBUG_ENTER("close_temporary_tables"); DBUG_ENTER("close_temporary_tables");
TABLE *table; TABLE *table;
...@@ -1642,9 +1642,10 @@ void close_temporary_tables(THD *thd) ...@@ -1642,9 +1642,10 @@ void close_temporary_tables(THD *thd)
TABLE *prev_table; TABLE *prev_table;
/* Assume thd->variables.option_bits has OPTION_QUOTE_SHOW_CREATE */ /* Assume thd->variables.option_bits has OPTION_QUOTE_SHOW_CREATE */
bool was_quote_show= TRUE; bool was_quote_show= TRUE;
bool error= 0;
if (!thd->temporary_tables) if (!thd->temporary_tables)
DBUG_VOID_RETURN; DBUG_RETURN(FALSE);
if (!mysql_bin_log.is_open()) if (!mysql_bin_log.is_open())
{ {
...@@ -1655,7 +1656,7 @@ void close_temporary_tables(THD *thd) ...@@ -1655,7 +1656,7 @@ void close_temporary_tables(THD *thd)
close_temporary(table, 1, 1); close_temporary(table, 1, 1);
} }
thd->temporary_tables= 0; thd->temporary_tables= 0;
DBUG_VOID_RETURN; DBUG_RETURN(FALSE);
} }
/* Better add "if exists", in case a RESET MASTER has been done */ /* Better add "if exists", in case a RESET MASTER has been done */
...@@ -1754,11 +1755,27 @@ void close_temporary_tables(THD *thd) ...@@ -1754,11 +1755,27 @@ void close_temporary_tables(THD *thd)
qinfo.db= db.ptr(); qinfo.db= db.ptr();
qinfo.db_len= db.length(); qinfo.db_len= db.length();
thd->variables.character_set_client= cs_save; thd->variables.character_set_client= cs_save;
if (mysql_bin_log.write(&qinfo))
thd->stmt_da->can_overwrite_status= TRUE;
if ((error= (mysql_bin_log.write(&qinfo) || error)))
{ {
push_warning(thd, MYSQL_ERROR::WARN_LEVEL_ERROR, MYF(0), /*
"Failed to write the DROP statement for temporary tables to binary log"); If we're here following THD::cleanup, thence the connection
has been closed already. So lets print a message to the
error log instead of pushing yet another error into the
stmt_da.
Also, we keep the error flag so that we propagate the error
up in the stack. This way, if we're the SQL thread we notice
that close_temporary_tables failed. (Actually, the SQL
thread only calls close_temporary_tables while applying old
Start_log_event_v3 events.)
*/
sql_print_error("Failed to write the DROP statement for "
"temporary tables to binary log");
} }
thd->stmt_da->can_overwrite_status= FALSE;
thd->variables.pseudo_thread_id= save_pseudo_thread_id; thd->variables.pseudo_thread_id= save_pseudo_thread_id;
thd->thread_specific_used= save_thread_specific_used; thd->thread_specific_used= save_thread_specific_used;
} }
...@@ -1771,7 +1788,8 @@ void close_temporary_tables(THD *thd) ...@@ -1771,7 +1788,8 @@ void close_temporary_tables(THD *thd)
if (!was_quote_show) if (!was_quote_show)
thd->variables.option_bits&= ~OPTION_QUOTE_SHOW_CREATE; /* restore option */ thd->variables.option_bits&= ~OPTION_QUOTE_SHOW_CREATE; /* restore option */
thd->temporary_tables=0; thd->temporary_tables=0;
DBUG_VOID_RETURN;
DBUG_RETURN(error);
} }
/* /*
......
...@@ -222,7 +222,7 @@ int decide_logging_format(THD *thd, TABLE_LIST *tables); ...@@ -222,7 +222,7 @@ int decide_logging_format(THD *thd, TABLE_LIST *tables);
void free_io_cache(TABLE *entry); void free_io_cache(TABLE *entry);
void intern_close_table(TABLE *entry); void intern_close_table(TABLE *entry);
bool close_thread_table(THD *thd, TABLE **table_ptr); bool close_thread_table(THD *thd, TABLE **table_ptr);
void close_temporary_tables(THD *thd); bool close_temporary_tables(THD *thd);
TABLE_LIST *unique_table(THD *thd, TABLE_LIST *table, TABLE_LIST *table_list, TABLE_LIST *unique_table(THD *thd, TABLE_LIST *table, TABLE_LIST *table_list,
bool check_alias); bool check_alias);
int drop_temporary_table(THD *thd, TABLE_LIST *table_list); int drop_temporary_table(THD *thd, TABLE_LIST *table_list);
......
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