#At file:///Users/thek/Development/mysql-pe/ based on revid:kristofer.pettersson@stripped
3491 Kristofer Pettersson 2009-07-30
Bug#44521 Executing a stored procedure as a prepared statement can sometimes cause
an assertion in a debug build.
The reason is that the C API doesn't support multiple result sets for prepared
statements and attempting to execute a stored routine which returns multiple result
sets sometimes lead to a network error. The network error sets the diagnostic area
prematurely which later leads to the assert when an attempt is made to set a second
server state.
This patch fixes the issue by changing the scope of the error code returned by
sp_instr_stmt::execute() to include any error which happened during the execution.
To assure that Diagnostic_area::is_sent really mean that the message was sent all
network related functions are checked for return status.
@ sql/protocol.cc
* Changed prototype to return success/failure status on net_send_error_packet(),
net_send_ok(), net_send_eof(), write_eof_packet().
@ sql/protocol.h
* Changed prototype to return success/failure status on net_send_error_packet(),
net_send_ok(), net_send_eof(), write_eof_packet().
* Changed protoype on virtual methods send_eof(), send_error() and send_ok()
@ sql/sp_head.cc
* If any error has been reported inside sp_instr_stmt::execute() the entire method
should return a failure status.
@ sql/sql_prepare.cc
* Adjusted implementation of base class prototype change to methods send_ok(), send_error() and send_eof()
modified:
libmysqld/emb_qcache.h
libmysqld/lib_sql.cc
mysql-test/r/sp_notembedded.result
mysql-test/t/sp_notembedded.test
sql/protocol.cc
sql/protocol.h
sql/sp_head.cc
sql/sql_prepare.cc
=== modified file 'libmysqld/emb_qcache.h'
--- a/libmysqld/emb_qcache.h 2009-07-30 11:04:12 +0000
+++ b/libmysqld/emb_qcache.h 2009-07-30 16:22:41 +0000
@@ -79,4 +79,4 @@ public:
uint emb_count_querycache_size(THD *thd);
int emb_load_querycache_result(THD *thd, Querycache_stream *src);
void emb_store_querycache_result(Querycache_stream *dst, THD* thd);
-void net_send_eof(THD *thd, uint server_status, uint total_warn_count);
+bool net_send_eof(THD *thd, uint server_status, uint total_warn_count);
=== modified file 'libmysqld/lib_sql.cc'
--- a/libmysqld/lib_sql.cc 2009-07-30 11:04:12 +0000
+++ b/libmysqld/lib_sql.cc 2009-07-30 16:22:41 +0000
@@ -804,11 +804,11 @@ MYSQL_DATA *THD::alloc_new_dataset()
*/
static
-void
+bool
write_eof_packet(THD *thd, uint server_status, uint statement_warn_count)
{
if (!thd->mysql) // bootstrap file handling
- return;
+ return FALSE;
/*
The following test should never be true, but it's better to do it
because if 'is_fatal_error' is set the server is not going to execute
@@ -823,6 +823,7 @@ write_eof_packet(THD *thd, uint server_s
*/
thd->cur_data->embedded_info->warning_count=
(thd->spcont ? 0 : min(statement_warn_count, 65535));
+ return FALSE;
}
@@ -1037,7 +1038,7 @@ bool Protocol_binary::write()
@return The function does not return errors.
*/
-void
+bool
net_send_ok(THD *thd,
uint server_status, uint statement_warn_count,
ulonglong affected_rows, ulonglong id, const char *message)
@@ -1047,18 +1048,18 @@ net_send_ok(THD *thd,
MYSQL *mysql= thd->mysql;
if (!mysql) // bootstrap file handling
- DBUG_VOID_RETURN;
+ DBUG_RETURN(FALSE);
if (!(data= thd->alloc_new_dataset()))
- return;
+ DBUG_RETURN(TRUE);
data->embedded_info->affected_rows= affected_rows;
data->embedded_info->insert_id= id;
if (message)
strmake(data->embedded_info->info, message,
sizeof(data->embedded_info->info)-1);
- write_eof_packet(thd, server_status, statement_warn_count);
+ bool error= write_eof_packet(thd, server_status, statement_warn_count);
thd->cur_data= 0;
- DBUG_VOID_RETURN;
+ DBUG_RETURN(error);
}
@@ -1070,15 +1071,15 @@ net_send_ok(THD *thd,
@return This function does not return errors.
*/
-void
+bool
net_send_eof(THD *thd, uint server_status, uint statement_warn_count)
{
- write_eof_packet(thd, server_status, statement_warn_count);
+ bool error= write_eof_packet(thd, server_status, statement_warn_count);
thd->cur_data= 0;
}
-void net_send_error_packet(THD *thd, uint sql_errno, const char *err,
+bool net_send_error_packet(THD *thd, uint sql_errno, const char *err,
const char* sqlstate)
{
uint error;
@@ -1090,7 +1091,7 @@ void net_send_error_packet(THD *thd, uin
if (!thd->mysql) // bootstrap file handling
{
fprintf(stderr, "ERROR: %d %s\n", sql_errno, err);
- return;
+ return FALSE;
}
if (!data)
@@ -1107,6 +1108,7 @@ void net_send_error_packet(THD *thd, uin
strmov(ei->sqlstate, sqlstate);
ei->server_status= thd->server_status;
thd->cur_data= 0;
+ return FALSE;
}
=== modified file 'mysql-test/r/sp_notembedded.result'
--- a/mysql-test/r/sp_notembedded.result 2009-07-30 11:04:12 +0000
+++ b/mysql-test/r/sp_notembedded.result 2009-07-30 16:22:41 +0000
@@ -251,3 +251,25 @@ DROP PROCEDURE p1;
DELETE FROM mysql.user WHERE User='mysqltest_1';
FLUSH PRIVILEGES;
set @@global.concurrent_insert= @old_concurrent_insert;
+#
+# Bug#44521 Prepared Statement: CALL p() - crashes: `! thd->main_da.is_sent' failed et.al.
+#
+SELECT GET_LOCK('Bug44521', 0);
+GET_LOCK('Bug44521', 0)
+1
+** Connection con1
+CREATE PROCEDURE p()
+BEGIN
+SELECT 1;
+SELECT GET_LOCK('Bug44521', 100);
+SELECT 2;
+END$
+CALL p();;
+** Default connection
+SELECT RELEASE_LOCK('Bug44521');
+RELEASE_LOCK('Bug44521')
+1
+DROP PROCEDURE p;
+# ------------------------------------------------------------------
+# -- End of 5.1 tests
+# ------------------------------------------------------------------
=== modified file 'mysql-test/t/sp_notembedded.test'
--- a/mysql-test/t/sp_notembedded.test 2009-07-30 11:04:12 +0000
+++ b/mysql-test/t/sp_notembedded.test 2009-07-30 16:22:41 +0000
@@ -379,3 +379,39 @@ set @@global.concurrent_insert= @old_con
# Wait till all disconnects are completed
--source include/wait_until_count_sessions.inc
+
+--echo #
+--echo # Bug#44521 Prepared Statement: CALL p() - crashes: `! thd->main_da.is_sent' failed et.al.
+--echo #
+SELECT GET_LOCK('Bug44521', 0);
+--connect (con1,localhost,root,,)
+--echo ** Connection con1
+delimiter $;
+CREATE PROCEDURE p()
+BEGIN
+ SELECT 1;
+ SELECT GET_LOCK('Bug44521', 100);
+ SELECT 2;
+END$
+delimiter ;$
+--send CALL p();
+--connection default
+--echo ** Default connection
+let $wait_condition=
+ SELECT count(*) = 1 FROM information_schema.processlist
+ WHERE state = "User lock" AND info = "SELECT GET_LOCK('Bug44521', 100)";
+--source include/wait_condition.inc
+let $conid =
+ `SELECT id FROM information_schema.processlist
+ WHERE state = "User lock" AND info = "SELECT GET_LOCK('Bug44521', 100)"`;
+dirty_close con1;
+SELECT RELEASE_LOCK('Bug44521');
+let $wait_condition=
+ SELECT count(*) = 0 FROM information_schema.processlist
+ WHERE id = $conid;
+--source include/wait_condition.inc
+DROP PROCEDURE p;
+
+--echo # ------------------------------------------------------------------
+--echo # -- End of 5.1 tests
+--echo # ------------------------------------------------------------------
=== modified file 'sql/protocol.cc'
--- a/sql/protocol.cc 2009-07-30 11:04:12 +0000
+++ b/sql/protocol.cc 2009-07-30 16:22:41 +0000
@@ -28,13 +28,13 @@
#include <stdarg.h>
static const unsigned int PACKET_BUFFER_EXTRA_ALLOC= 1024;
-void net_send_error_packet(THD *, uint, const char *, const char *);
+bool net_send_error_packet(THD *, uint, const char *, const char *);
/* Declared non-static only because of the embedded library. */
-void net_send_ok(THD *, uint, uint, ulonglong, ulonglong, const char *);
+bool net_send_ok(THD *, uint, uint, ulonglong, ulonglong, const char *);
/* Declared non-static only because of the embedded library. */
-void net_send_eof(THD *thd, uint server_status, uint statement_warn_count);
+bool net_send_eof(THD *thd, uint server_status, uint statement_warn_count);
#ifndef EMBEDDED_LIBRARY
-static void write_eof_packet(THD *, NET *, uint, uint);
+static bool write_eof_packet(THD *, NET *, uint, uint);
#endif
#ifndef EMBEDDED_LIBRARY
@@ -180,19 +180,20 @@ void net_send_error(THD *thd, uint sql_e
*/
#ifndef EMBEDDED_LIBRARY
-void
+bool
net_send_ok(THD *thd,
uint server_status, uint statement_warn_count,
ulonglong affected_rows, ulonglong id, const char *message)
{
NET *net= &thd->net;
uchar buff[MYSQL_ERRMSG_SIZE+10],*pos;
+ bool error= FALSE;
DBUG_ENTER("my_ok");
if (! net->vio) // hack for re-parsing queries
{
DBUG_PRINT("info", ("vio present: NO"));
- DBUG_VOID_RETURN;
+ DBUG_RETURN(FALSE);
}
buff[0]=0; // No fields
@@ -223,13 +224,15 @@ net_send_ok(THD *thd,
if (message && message[0])
pos= net_store_data(pos, (uchar*) message, strlen(message));
- (void) my_net_write(net, buff, (size_t) (pos-buff));
- (void) net_flush(net);
+ error= my_net_write(net, buff, (size_t) (pos-buff));
+ if (!error)
+ error= net_flush(net);
+
thd->stmt_da->can_overwrite_status= FALSE;
DBUG_PRINT("info", ("OK sent, so no more error sending allowed"));
- DBUG_VOID_RETURN;
+ DBUG_RETURN(error);
}
static uchar eof_buff[1]= { (uchar) 254 }; /* Marker for end of fields */
@@ -253,21 +256,23 @@ static uchar eof_buff[1]= { (uchar) 254
like in send_result_set_metadata().
*/
-void
+bool
net_send_eof(THD *thd, uint server_status, uint statement_warn_count)
{
- NET *net= &thd->net;
DBUG_ENTER("net_send_eof");
+ bool error= FALSE;
+ NET *net= &thd->net;
/* Set to TRUE if no active vio, to work well in case of --init-file */
if (net->vio != 0)
{
thd->stmt_da->can_overwrite_status= TRUE;
- write_eof_packet(thd, net, server_status, statement_warn_count);
- (void) net_flush(net);
+ error= write_eof_packet(thd, net, server_status, statement_warn_count);
+ if (!error)
+ error= net_flush(net);
thd->stmt_da->can_overwrite_status= FALSE;
DBUG_PRINT("info", ("EOF sent, so no more error sending allowed"));
}
- DBUG_VOID_RETURN;
+ DBUG_RETURN(error);
}
@@ -276,7 +281,7 @@ net_send_eof(THD *thd, uint server_statu
write it to the network output buffer.
*/
-static void write_eof_packet(THD *thd, NET *net,
+static bool write_eof_packet(THD *thd, NET *net,
uint server_status,
uint statement_warn_count)
{
@@ -298,10 +303,10 @@ static void write_eof_packet(THD *thd, N
if (thd->is_fatal_error)
server_status&= ~SERVER_MORE_RESULTS_EXISTS;
int2store(buff + 3, server_status);
- (void) my_net_write(net, buff, 5);
+ return(my_net_write(net, buff, 5));
}
else
- (void) my_net_write(net, eof_buff, 1);
+ return(my_net_write(net, eof_buff, 1));
}
/**
@@ -322,7 +327,7 @@ bool send_old_password_request(THD *thd)
}
-void net_send_error_packet(THD *thd, uint sql_errno, const char *err,
+bool net_send_error_packet(THD *thd, uint sql_errno, const char *err,
const char* sqlstate)
{
NET *net= &thd->net;
@@ -344,7 +349,7 @@ void net_send_error_packet(THD *thd, uin
/* In bootstrap it's ok to print on stderr */
fprintf(stderr,"ERROR: %d %s\n",sql_errno,err);
}
- DBUG_VOID_RETURN;
+ DBUG_RETURN(FALSE);
}
int2store(buff,sql_errno);
@@ -365,9 +370,8 @@ void net_send_error_packet(THD *thd, uin
(char*) buff);
err= (char*) buff;
- (void) net_write_command(net,(uchar) 255, (uchar*) "", 0, (uchar*) err,
- length);
- DBUG_VOID_RETURN;
+ DBUG_RETURN(net_write_command(net,(uchar) 255, (uchar*) "", 0, (uchar*) err,
+ length));
}
#endif /* EMBEDDED_LIBRARY */
@@ -455,6 +459,7 @@ void Protocol::end_statement()
{
DBUG_ENTER("Protocol::end_statement");
DBUG_ASSERT(! thd->stmt_da->is_sent);
+ bool error= FALSE;
/* Can not be true, but do not take chances in production. */
if (thd->stmt_da->is_sent)
@@ -463,30 +468,31 @@ void Protocol::end_statement()
switch (thd->stmt_da->status()) {
case Diagnostics_area::DA_ERROR:
/* The query failed, send error to log and abort bootstrap. */
- send_error(thd->stmt_da->sql_errno(),
- thd->stmt_da->message(),
- thd->stmt_da->get_sqlstate());
+ error= send_error(thd->stmt_da->sql_errno(),
+ thd->stmt_da->message(),
+ thd->stmt_da->get_sqlstate());
break;
case Diagnostics_area::DA_EOF:
- send_eof(thd->stmt_da->server_status(),
- thd->stmt_da->statement_warn_count());
+ error= send_eof(thd->stmt_da->server_status(),
+ thd->stmt_da->statement_warn_count());
break;
case Diagnostics_area::DA_OK:
- send_ok(thd->stmt_da->server_status(),
- thd->stmt_da->statement_warn_count(),
- thd->stmt_da->affected_rows(),
- thd->stmt_da->last_insert_id(),
- thd->stmt_da->message());
+ error= send_ok(thd->stmt_da->server_status(),
+ thd->stmt_da->statement_warn_count(),
+ thd->stmt_da->affected_rows(),
+ thd->stmt_da->last_insert_id(),
+ thd->stmt_da->message());
break;
case Diagnostics_area::DA_DISABLED:
break;
case Diagnostics_area::DA_EMPTY:
default:
DBUG_ASSERT(0);
- send_ok(thd->server_status, 0, 0, 0, NULL);
+ error= send_ok(thd->server_status, 0, 0, 0, NULL);
break;
}
- thd->stmt_da->is_sent= TRUE;
+ if (!error)
+ thd->stmt_da->is_sent= TRUE;
DBUG_VOID_RETURN;
}
@@ -500,16 +506,14 @@ void Protocol::end_statement()
on client side.
*/
-void Protocol::send_ok(uint server_status, uint statement_warn_count,
+bool Protocol::send_ok(uint server_status, uint statement_warn_count,
ulonglong affected_rows, ulonglong last_insert_id,
const char *message)
{
DBUG_ENTER("Protocol::send_ok");
- net_send_ok(thd, server_status, statement_warn_count, affected_rows,
- last_insert_id, message);
-
- DBUG_VOID_RETURN;
+ DBUG_RETURN(net_send_ok(thd, server_status, statement_warn_count,
+ affected_rows, last_insert_id, message));
}
@@ -519,13 +523,11 @@ void Protocol::send_ok(uint server_statu
Binary and text protocol do not differ in their EOF packet format.
*/
-void Protocol::send_eof(uint server_status, uint statement_warn_count)
+bool Protocol::send_eof(uint server_status, uint statement_warn_count)
{
DBUG_ENTER("Protocol::send_eof");
- net_send_eof(thd, server_status, statement_warn_count);
-
- DBUG_VOID_RETURN;
+ DBUG_RETURN(net_send_eof(thd, server_status, statement_warn_count));
}
@@ -535,13 +537,12 @@ void Protocol::send_eof(uint server_stat
Binary and text protocol do not differ in ERROR packet format.
*/
-void Protocol::send_error(uint sql_errno, const char *err_msg, const char *sql_state)
+bool Protocol::send_error(uint sql_errno, const char *err_msg,
+ const char *sql_state)
{
DBUG_ENTER("Protocol::send_error");
- net_send_error_packet(thd, sql_errno, err_msg, sql_state);
-
- DBUG_VOID_RETURN;
+ DBUG_RETURN(net_send_error_packet(thd, sql_errno, err_msg, sql_state));
}
=== modified file 'sql/protocol.h'
--- a/sql/protocol.h 2009-07-30 11:04:12 +0000
+++ b/sql/protocol.h 2009-07-30 16:22:41 +0000
@@ -49,13 +49,14 @@ protected:
bool store_string_aux(const char *from, size_t length,
CHARSET_INFO *fromcs, CHARSET_INFO *tocs);
- virtual void send_ok(uint server_status, uint statement_warn_count,
+ virtual bool send_ok(uint server_status, uint statement_warn_count,
ulonglong affected_rows, ulonglong last_insert_id,
const char *message);
- virtual void send_eof(uint server_status, uint statement_warn_count);
+ virtual bool send_eof(uint server_status, uint statement_warn_count);
- virtual void send_error(uint sql_errno, const char *err_msg, const char *sql_state);
+ virtual bool send_error(uint sql_errno, const char *err_msg,
+ const char *sql_state);
public:
Protocol() {}
=== modified file 'sql/sp_head.cc'
--- a/sql/sp_head.cc 2009-07-30 11:04:12 +0000
+++ b/sql/sp_head.cc 2009-07-30 16:22:41 +0000
@@ -2881,7 +2881,7 @@ sp_instr_stmt::execute(THD *thd, uint *n
if (!thd->is_error())
thd->stmt_da->reset_diagnostics_area();
}
- DBUG_RETURN(res);
+ DBUG_RETURN(res || thd->is_error());
}
=== modified file 'sql/sql_prepare.cc'
--- a/sql/sql_prepare.cc 2009-07-15 18:31:27 +0000
+++ b/sql/sql_prepare.cc 2009-07-30 16:22:41 +0000
@@ -236,12 +236,12 @@ protected:
#endif
virtual enum enum_protocol_type type() { return PROTOCOL_LOCAL; };
- virtual void send_ok(uint server_status, uint statement_warn_count,
+ virtual bool send_ok(uint server_status, uint statement_warn_count,
ulonglong affected_rows, ulonglong last_insert_id,
const char *message);
- virtual void send_eof(uint server_status, uint statement_warn_count);
- virtual void send_error(uint sql_errno, const char *err_msg, const char* sqlstate);
+ virtual bool send_eof(uint server_status, uint statement_warn_count);
+ virtual bool send_error(uint sql_errno, const char *err_msg, const char* sqlstate);
private:
bool store_string(const char *str, size_t length,
CHARSET_INFO *src_cs, CHARSET_INFO *dst_cs);
@@ -4342,7 +4342,7 @@ bool Protocol_local::send_out_parameters
/** Called for statements that don't have a result set, at statement end. */
-void
+bool
Protocol_local::send_ok(uint server_status, uint statement_warn_count,
ulonglong affected_rows, ulonglong last_insert_id,
const char *message)
@@ -4351,6 +4351,7 @@ Protocol_local::send_ok(uint server_stat
Just make sure nothing is sent to the client, we have grabbed
the status information in the connection diagnostics area.
*/
+ return FALSE;
}
@@ -4362,7 +4363,7 @@ Protocol_local::send_ok(uint server_stat
building of the result set at hand.
*/
-void Protocol_local::send_eof(uint server_status, uint statement_warn_count)
+bool Protocol_local::send_eof(uint server_status, uint statement_warn_count)
{
Ed_result_set *ed_result_set;
@@ -4377,7 +4378,7 @@ void Protocol_local::send_eof(uint serve
m_rset= NULL;
if (! ed_result_set)
- return;
+ return TRUE;
/* In case of successful allocation memory ownership was transferred. */
DBUG_ASSERT(!alloc_root_inited(&m_rset_root));
@@ -4387,18 +4388,20 @@ void Protocol_local::send_eof(uint serve
result sets. Never fails.
*/
m_connection->add_result_set(ed_result_set);
+ return FALSE;
}
/** Called to send an error to the client at the end of a statement. */
-void
+bool
Protocol_local::send_error(uint sql_errno, const char *err_msg, const char*)
{
/*
Just make sure that nothing is sent to the client (default
implementation).
*/
+ return FALSE;
}
Attachment: [text/bzr-bundle]
