List:Commits« Previous MessageNext Message »
From:Kristofer Pettersson Date:July 30 2009 4:22pm
Subject:bzr commit into mysql-pe branch (kristofer.pettersson:3491) Bug#44521
View as plain text  
#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]
Thread
bzr commit into mysql-pe branch (kristofer.pettersson:3491) Bug#44521Kristofer Pettersson30 Jul
  • Re: bzr commit into mysql-pe branch (kristofer.pettersson:3491)Bug#44521Davi Arnaut30 Jul