#At file:///home/uchum/work/bzr/38816-5.0/ based on revid:azundris@stripped
2768 Gleb Shchepa 2009-06-11
Bug #38816: kill + flush tables with read lock + stored
procedures causes crashes!
The problem of that bugreport was mostly fixed by the
patch for bug 38691.
However, attached test case focused on another crash or
valgrind warning problem: SHOW PROCESSLIST query accesses
freed memory of SP instruction that run in a parallel
connection.
Changes of thd->query/thd->query_length in dangerous
places have been guarded with the new per-thread
LOCK_query_string mutex.
@ sql/ha_myisam.cc
Bug #38816: kill + flush tables with read lock + stored
procedures causes crashes!
Modification of THD::query/query_length has been guarded
with the a THD::set_query() method call/LOCK_query_string
mutex.
Unnecessary locking with the global LOCK_thread_count
mutex has been removed.
@ sql/log_event.cc
Bug #38816: kill + flush tables with read lock + stored
procedures causes crashes!
Modification of THD::query/query_length has been guarded
with the a THD::set_query()/reset_query() method calls/
LOCK_query_string mutex.
@ sql/slave.cc
Bug #38816: kill + flush tables with read lock + stored
procedures causes crashes!
Modification of THD::query/query_length has been guarded
with the a THD::set_query()/reset_query() method calls/
LOCK_query_string mutex.
@ sql/sp_head.cc
Bug #38816: kill + flush tables with read lock + stored
procedures causes crashes!
Modification of THD::query/query_length has been guarded
with the a THD::set_query() method calls/ LOCK_query_string
mutex.
@ sql/sql_class.cc
Bug #38816: kill + flush tables with read lock + stored
procedures causes crashes!
The new THD::LOCK_query_string mutex and THD::set_query(),
THD::memdup_query_w_gap() and THD::reset_query() methods
have been added to guard modifications of THD::query/
THD::query_length fields, also the Statement::set_statement()
method has been overloaded in the THD class.
@ sql/sql_class.h
Bug #38816: kill + flush tables with read lock + stored
procedures causes crashes!
The new THD::LOCK_query_string mutex and THD::set_query(),
THD::memdup_query_w_gap() and THD::reset_query() methods
have been added to guard modifications of THD::query/
THD::query_length fields, also the Statement::set_statement()
method has been overloaded in the THD class.
@ sql/sql_insert.cc
Bug #38816: kill + flush tables with read lock + stored
procedures causes crashes!
Modification of THD::query/query_length has been guarded
with the a THD::set_query() method calls/ LOCK_query_string
mutex.
@ sql/sql_parse.cc
Bug #38816: kill + flush tables with read lock + stored
procedures causes crashes!
Modification of THD::query/query_length has been guarded
with the a THD::set_query()/memdup_query_w_gap()/reset_query()
method calls/LOCK_query_string mutex.
@ sql/sql_show.cc
Bug #38816: kill + flush tables with read lock + stored
procedures causes crashes!
Inter-thread read of THD::query/query_length field has
been protected with a new per-thread LOCK_query_string
mutex in the mysqld_list_processes function.
modified:
sql/ha_myisam.cc
sql/log_event.cc
sql/slave.cc
sql/sp_head.cc
sql/sql_class.cc
sql/sql_class.h
sql/sql_insert.cc
sql/sql_parse.cc
sql/sql_show.cc
=== modified file 'sql/ha_myisam.cc'
--- a/sql/ha_myisam.cc 2009-06-04 22:23:08 +0000
+++ b/sql/ha_myisam.cc 2009-06-10 22:28:04 +0000
@@ -1487,10 +1487,8 @@ bool ha_myisam::check_and_repair(THD *th
old_query= thd->query;
old_query_length= thd->query_length;
- pthread_mutex_lock(&LOCK_thread_count);
- thd->query= (char*) table->s->table_name;
- thd->query_length= (uint32) strlen(table->s->table_name);
- pthread_mutex_unlock(&LOCK_thread_count);
+ thd->set_query((char*) table->s->table_name,
+ (uint32) strlen(table->s->table_name));
if ((marked_crashed= mi_is_crashed(file)) || check(thd, &check_opt))
{
@@ -1503,10 +1501,7 @@ bool ha_myisam::check_and_repair(THD *th
if (repair(thd, &check_opt))
error=1;
}
- pthread_mutex_lock(&LOCK_thread_count);
- thd->query= old_query;
- thd->query_length= old_query_length;
- pthread_mutex_unlock(&LOCK_thread_count);
+ thd->set_query(old_query, old_query_length);
DBUG_RETURN(error);
}
=== modified file 'sql/log_event.cc'
--- a/sql/log_event.cc 2009-05-31 03:26:58 +0000
+++ b/sql/log_event.cc 2009-06-10 22:28:04 +0000
@@ -1959,8 +1959,7 @@ int Query_log_event::exec_event(struct s
db_ok(thd->db, replicate_do_db, replicate_ignore_db))
{
thd->set_time((time_t)when);
- thd->query_length= q_len_arg;
- thd->query= (char*)query_arg;
+ thd->set_query((char*)query_arg, q_len_arg);
VOID(pthread_mutex_lock(&LOCK_thread_count));
thd->query_id = next_query_id();
VOID(pthread_mutex_unlock(&LOCK_thread_count));
@@ -2177,8 +2176,7 @@ end:
thd->catalog= 0;
thd->set_db(NULL, 0); /* will free the current database */
DBUG_PRINT("info", ("end: query= 0"));
- thd->query= 0; // just to be sure
- thd->query_length= 0;
+ thd->reset_query();
VOID(pthread_mutex_unlock(&LOCK_thread_count));
close_thread_tables(thd);
free_root(thd->mem_root,MYF(MY_KEEP_PREALLOC));
@@ -3258,8 +3256,7 @@ int Load_log_event::exec_event(NET* net,
print_query(FALSE, load_data_query, &end, (char **)&thd->lex->fname_start,
(char **)&thd->lex->fname_end);
*end= 0;
- thd->query_length= (uint) (end - load_data_query);
- thd->query= load_data_query;
+ thd->set_query(load_data_query, (uint) (end - load_data_query));
if (sql_ex.opt_flags & REPLACE_FLAG)
{
@@ -3368,8 +3365,7 @@ error:
VOID(pthread_mutex_lock(&LOCK_thread_count));
thd->catalog= 0;
thd->set_db(NULL, 0); /* will free the current database */
- thd->query= 0;
- thd->query_length= 0;
+ thd->reset_query();
VOID(pthread_mutex_unlock(&LOCK_thread_count));
close_thread_tables(thd);
if (thd->query_error)
=== modified file 'sql/slave.cc'
--- a/sql/slave.cc 2009-05-22 23:15:21 +0000
+++ b/sql/slave.cc 2009-06-10 22:28:04 +0000
@@ -1608,15 +1608,13 @@ static int create_table_from_dump(THD* t
DBUG_RETURN(1);
}
thd->command = COM_TABLE_DUMP;
- thd->query_length= packet_len;
- /* Note that we should not set thd->query until the area is initalized */
if (!(query = thd->strmake((char*) net->read_pos, packet_len)))
{
sql_print_error("create_table_from_dump: out of memory");
my_message(ER_GET_ERRNO, "Out of memory", MYF(0));
DBUG_RETURN(1);
}
- thd->query= query;
+ thd->set_query(query, packet_len);
thd->query_error = 0;
thd->net.no_send_ok = 1;
@@ -3868,8 +3866,7 @@ err:
sql_print_information("Slave I/O thread exiting, read up to log '%s', position %s",
IO_RPL_LOG_NAME, llstr(mi->master_log_pos,llbuff));
VOID(pthread_mutex_lock(&LOCK_thread_count));
- thd->query= 0; // extra safety
- thd->query_length= 0;
+ thd->reset_query();
thd->reset_db(NULL, 0);
VOID(pthread_mutex_unlock(&LOCK_thread_count));
if (mysql)
@@ -4113,8 +4110,7 @@ the slave SQL thread with \"SLAVE START\
*/
thd->catalog= 0;
thd->reset_db(NULL, 0);
- thd->query= 0;
- thd->query_length= 0;
+ thd->reset_query();
VOID(pthread_mutex_unlock(&LOCK_thread_count));
thd_proc_info(thd, "Waiting for slave mutex on exit");
pthread_mutex_lock(&rli->run_lock);
=== modified file 'sql/sp_head.cc'
--- a/sql/sp_head.cc 2009-05-06 13:06:32 +0000
+++ b/sql/sp_head.cc 2009-06-10 22:28:04 +0000
@@ -949,8 +949,7 @@ subst_spvars(THD *thd, sp_instr *instr,
else
DBUG_RETURN(TRUE);
- thd->query= pbuf;
- thd->query_length= qbuf.length();
+ thd->set_query(pbuf, qbuf.length());
DBUG_RETURN(FALSE);
}
@@ -2654,8 +2653,7 @@ sp_instr_stmt::execute(THD *thd, uint *n
}
else
*nextp= m_ip+1;
- thd->query= query;
- thd->query_length= query_length;
+ thd->set_query(query, query_length);
thd->query_name_consts= 0;
}
DBUG_RETURN(res);
=== modified file 'sql/sql_class.cc'
--- a/sql/sql_class.cc 2009-05-15 09:29:41 +0000
+++ b/sql/sql_class.cc 2009-06-10 22:28:04 +0000
@@ -260,6 +260,7 @@ THD::THD()
active_vio = 0;
#endif
pthread_mutex_init(&LOCK_delete, MY_MUTEX_INIT_FAST);
+ pthread_mutex_init(&LOCK_query_string, MY_MUTEX_INIT_FAST);
/* Variables with default values */
proc_info="login";
@@ -513,6 +514,7 @@ THD::~THD()
free_root(&transaction.mem_root,MYF(0));
#endif
mysys_var=0; // Safety (shouldn't be needed)
+ pthread_mutex_destroy(&LOCK_query_string);
pthread_mutex_destroy(&LOCK_delete);
#ifndef DBUG_OFF
dbug_sentry= THD_SENTRY_GONE;
@@ -2422,3 +2424,31 @@ void xid_cache_delete(XID_STATE *xid_sta
pthread_mutex_unlock(&LOCK_xid_cache);
}
+
+void THD::set_statement(Statement *stmt)
+{
+ pthread_mutex_lock(&LOCK_query_string);
+ Statement::set_statement(stmt);
+ pthread_mutex_unlock(&LOCK_query_string);
+}
+
+
+void THD::set_query(char *str, uint32 length)
+{
+ pthread_mutex_lock(&LOCK_query_string);
+ query= str;
+ query_length= length;
+ pthread_mutex_unlock(&LOCK_query_string);
+}
+
+
+bool THD::memdup_query_w_gap(const char *str, uint32 size, uint gap)
+{
+ char *s= (char*) memdup_w_gap((gptr) (str), size, gap);
+ if (!s)
+ return TRUE;
+ s[size]= 0;
+ set_query(s, size);
+ return FALSE;
+}
+
=== modified file 'sql/sql_class.h'
--- a/sql/sql_class.h 2009-05-05 21:03:23 +0000
+++ b/sql/sql_class.h 2009-06-10 22:28:04 +0000
@@ -866,7 +866,7 @@ public:
virtual ~Statement();
/* Assign execution context (note: not all members) of given stmt to self */
- void set_statement(Statement *stmt);
+ virtual void set_statement(Statement *stmt);
void set_n_backup_statement(Statement *stmt, Statement *backup);
void restore_backup_statement(Statement *stmt, Statement *backup);
/* return class type */
@@ -1230,6 +1230,7 @@ public:
THR_LOCK_OWNER *lock_id; // If not main_lock_id, points to
// the lock_id of a cursor.
pthread_mutex_t LOCK_delete; // Locked before thd is deleted
+ pthread_mutex_t LOCK_query_string; // Guard query & query_length fields
/* all prepared statements and cursors of this connection */
Statement_map stmt_map;
/*
@@ -1882,6 +1883,17 @@ public:
*/
void pop_internal_handler();
+ /* Overloaded to guard query/query_length fields */
+ virtual void set_statement(Statement *stmt);
+
+ /*
+ Functions to protect modifications of query and query_length fields
+ with LOCK_query_string.
+ */
+ void set_query(char *str, uint32 length);
+ bool memdup_query_w_gap(const char *str, uint32 size, uint gap);
+ void reset_query() { set_query(NULL, 0); }
+
private:
/** The current internal error handler for this thread, or NULL. */
Internal_error_handler *m_internal_handler;
=== modified file 'sql/sql_insert.cc'
--- a/sql/sql_insert.cc 2009-06-04 22:23:08 +0000
+++ b/sql/sql_insert.cc 2009-06-10 22:28:04 +0000
@@ -1827,7 +1827,7 @@ bool delayed_get_table(THD *thd, TABLE_L
thread_count++;
pthread_mutex_unlock(&LOCK_thread_count);
di->thd.set_db(table_list->db, (uint) strlen(table_list->db));
- di->thd.query= my_strdup(table_list->table_name, MYF(MY_WME));
+ di->thd.set_query(my_strdup(table_list->table_name, MYF(MY_WME)), 0);
if (di->thd.db == NULL || di->thd.query == NULL)
{
/* The error is reported */
=== modified file 'sql/sql_parse.cc'
--- a/sql/sql_parse.cc 2009-05-06 13:06:32 +0000
+++ b/sql/sql_parse.cc 2009-06-10 22:28:04 +0000
@@ -1106,8 +1106,7 @@ void execute_init_command(THD *thd, sys_
values of init_command_var can't be changed
*/
rw_rdlock(var_mutex);
- thd->query= init_command_var->value;
- thd->query_length= init_command_var->value_length;
+ thd->set_query(init_command_var->value, init_command_var->value_length);
save_client_capabilities= thd->client_capabilities;
thd->client_capabilities|= CLIENT_MULTI_QUERIES;
/*
@@ -1350,10 +1349,8 @@ pthread_handler_t handle_bootstrap(void
buff[length-1] == ';'))
length--;
buff[length]=0;
- thd->query_length=length;
- thd->query= thd->memdup_w_gap(buff, length+1,
- thd->db_length+1+QUERY_CACHE_FLAGS_SIZE);
- thd->query[length] = '\0';
+ thd->memdup_query_w_gap(buff, length,
+ thd->db_length + 1 + QUERY_CACHE_FLAGS_SIZE + 1);
DBUG_PRINT("query",("%-.4096s",thd->query));
#if defined(ENABLED_PROFILING) && defined(COMMUNITY_SERVER)
thd->profiling.set_query_source(thd->query, length);
@@ -1463,8 +1460,7 @@ int mysql_table_dump(THD* thd, char* db,
if (check_one_table_access(thd, SELECT_ACL, table_list))
goto err;
thd->free_list = 0;
- thd->query_length=(uint) strlen(tbl_name);
- thd->query = tbl_name;
+ thd->set_query(tbl_name, (uint) strlen(tbl_name));
if ((error = mysqld_dump_create_info(thd, table_list, -1)))
{
my_error(ER_GET_ERRNO, MYF(0), my_errno);
@@ -1988,8 +1984,7 @@ bool dispatch_command(enum enum_server_c
#endif
VOID(pthread_mutex_lock(&LOCK_thread_count));
- thd->query_length= length;
- thd->query= next_packet;
+ thd->set_query(next_packet, length);
/*
Count each statement from the client.
*/
@@ -2041,9 +2036,10 @@ bool dispatch_command(enum enum_server_c
table_list.schema_table= schema_table;
}
- thd->query_length= (uint) strlen(packet); // for simplicity: don't optimize
- if (!(thd->query=fields=thd->memdup(packet,thd->query_length+1)))
+ uint query_length= (uint) strlen(packet);
+ if (!(fields= thd->memdup(packet,query_length + 1)))
break;
+ thd->set_query(fields, query_length);
mysql_log.write(thd,command,"%s %s",table_list.table_name, fields);
if (lower_case_table_names)
my_casedn_str(files_charset_info, table_list.table_name);
@@ -2330,8 +2326,7 @@ bool dispatch_command(enum enum_server_c
VOID(pthread_mutex_lock(&LOCK_thread_count)); // For process list
thd_proc_info(thd, 0);
thd->command=COM_SLEEP;
- thd->query=0;
- thd->query_length=0;
+ thd->reset_query();
thread_running--;
VOID(pthread_mutex_unlock(&LOCK_thread_count));
thd->packet.shrink(thd->variables.net_buffer_length); // Reclaim some memory
@@ -2551,14 +2546,9 @@ bool alloc_query(THD *thd, const char *p
packet_length--;
}
/* We must allocate some extra memory for query cache */
- thd->query_length= 0; // Extra safety: Avoid races
- if (!(thd->query= (char*) thd->memdup_w_gap((gptr) (packet),
- packet_length,
- thd->db_length+ 1 +
- QUERY_CACHE_FLAGS_SIZE)))
+ if (thd->memdup_query_w_gap(packet, packet_length,
+ thd->db_length + 1 + QUERY_CACHE_FLAGS_SIZE))
return TRUE;
- thd->query[packet_length]=0;
- thd->query_length= packet_length;
/* Reclaim some memory */
thd->packet.shrink(thd->variables.net_buffer_length);
=== modified file 'sql/sql_show.cc'
--- a/sql/sql_show.cc 2009-05-05 21:03:23 +0000
+++ b/sql/sql_show.cc 2009-06-10 22:28:04 +0000
@@ -1417,8 +1417,10 @@ void mysqld_list_processes(THD *thd,cons
the comment in sql_class.h why this prevents crashes in possible
races with query_length
*/
+ pthread_mutex_lock(&tmp->LOCK_query_string);
uint length= min(max_query_length, tmp->query_length);
thd_info->query=(char*) thd->strmake(tmp->query,length);
+ pthread_mutex_unlock(&tmp->LOCK_query_string);
}
thread_infos.append(thd_info);
}
Attachment: [text/bzr-bundle] bzr/gshchepa@mysql.com-20090610222804-s37tq9darlt9n9k0.bundle
