List:Commits« Previous MessageNext Message »
From:Kristofer Pettersson Date:July 27 2009 4:08pm
Subject:bzr commit into mysql-5.1-bugteam branch (kristofer.pettersson:2935)
Bug#44521
View as plain text  
#At file:///Users/thek/Development/51-bug44521/ based on revid:pstoev@stripped

 2935 Kristofer Pettersson	2009-07-27
      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/net_serv.cc
        * Added error injection code.
        * Removed debug assert in order to get proper error code in debug build.
     @ 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 on net_send_error() so that a success/failure status is
          reported.
     @ 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_parse.cc
        * Added error injection code for the regression test.

    modified:
      mysql-test/r/sp.result
      mysql-test/t/sp.test
      sql/net_serv.cc
      sql/protocol.cc
      sql/protocol.h
      sql/sp_head.cc
      sql/sql_parse.cc
=== modified file 'mysql-test/r/sp.result'
--- a/mysql-test/r/sp.result	2009-06-04 11:53:15 +0000
+++ b/mysql-test/r/sp.result	2009-07-27 16:08:36 +0000
@@ -6963,6 +6963,17 @@ CALL p1();
 CALL p1();
 DROP PROCEDURE p1;
 DROP TABLE t1;
+#
+# Bug44521 Prepared Statement: CALL p() - crashes: `! thd->main_da.is_sent' failed et.al.
+#
+DROP PROCEDURE IF EXISTS p;
+CREATE PROCEDURE p() BEGIN SELECT 1; SELECT 2; END$
+set session debug="+d,net_real_write_failure";
+CALL p();
+ERROR HY000: Lost connection to MySQL server during query
+# Server should still be up and running on the default connection.
+DROP PROCEDURE p;
+#
 # ------------------------------------------------------------------
 # -- End of 5.1 tests
 # ------------------------------------------------------------------

=== modified file 'mysql-test/t/sp.test'
--- a/mysql-test/t/sp.test	2009-06-04 11:53:15 +0000
+++ b/mysql-test/t/sp.test	2009-07-27 16:08:36 +0000
@@ -8242,6 +8242,25 @@ while ($tab_count)
 DROP PROCEDURE p1;
 DROP TABLE t1;
 
+--echo #
+--echo # Bug44521 Prepared Statement: CALL p() - crashes: `! thd->main_da.is_sent' failed et.al.
+--echo #
+--disable_warnings
+DROP PROCEDURE IF EXISTS p;
+--enable_warnings
+--connect (con1,localhost,root,,)
+--connection con1
+delimiter $;
+CREATE PROCEDURE p() BEGIN SELECT 1; SELECT 2; END$
+delimiter ;$
+set session debug="+d,net_real_write_failure";
+--error 2013
+CALL p();
+--connection default
+--echo # Server should still be up and running on the default connection.
+DROP PROCEDURE p;
+--echo #
+
 --echo # ------------------------------------------------------------------
 --echo # -- End of 5.1 tests
 --echo # ------------------------------------------------------------------

=== modified file 'sql/net_serv.cc'
--- a/sql/net_serv.cc	2009-02-13 16:41:47 +0000
+++ b/sql/net_serv.cc	2009-07-27 16:08:36 +0000
@@ -48,6 +48,7 @@
 #include <violite.h>
 #include <signal.h>
 #include <errno.h>
+#include <ctype.h>
 #ifdef __NETWARE__
 #include <sys/select.h>
 #endif
@@ -330,13 +331,13 @@ void net_clear(NET *net, my_bool clear_b
   DBUG_VOID_RETURN;
 }
 
-
 /** Flush write_buffer if not empty. */
 
 my_bool net_flush(NET *net)
 {
   my_bool error= 0;
   DBUG_ENTER("net_flush");
+
   if (net->buff != net->write_pos)
   {
     error=test(net_real_write(net, net->buff,
@@ -561,6 +562,10 @@ net_real_write(NET *net,const uchar *pac
   my_bool net_blocking = vio_is_blocking(net->vio);
   DBUG_ENTER("net_real_write");
 
+#ifndef DBUG_OFF
+  DBUG_EXECUTE_IF("net_real_write_failure2", DBUG_RETURN(-1););
+#endif
+
 #if defined(MYSQL_SERVER) && defined(USE_QUERY_CACHE)
   query_cache_insert(net, (char*) packet, len);
 #endif
@@ -907,7 +912,6 @@ my_real_read(NET *net, size_t *complen)
 		    (int) net->buff[net->where_b + 3],
 		    (uint) (uchar) net->pkt_nr);
             fflush(stderr);
-            DBUG_ASSERT(0);
 #endif
 	  }
 	  len= packet_error;

=== modified file 'sql/protocol.cc'
--- a/sql/protocol.cc	2009-03-24 13:58:52 +0000
+++ b/sql/protocol.cc	2009-07-27 16:08:36 +0000
@@ -29,11 +29,11 @@
 
 static const unsigned int PACKET_BUFFER_EXTRA_ALLOC= 1024;
 /* Declared non-static only because of the embedded library. */
-void net_send_error_packet(THD *thd, uint sql_errno, const char *err);
-void net_send_ok(THD *, uint, uint, ha_rows, ulonglong, const char *);
-void net_send_eof(THD *thd, uint server_status, uint total_warn_count);
+bool net_send_error_packet(THD *thd, uint sql_errno, const char *err);
+bool net_send_ok(THD *, uint, uint, ha_rows, ulonglong, const char *);
+bool net_send_eof(THD *thd, uint server_status, uint total_warn_count);
 #ifndef EMBEDDED_LIBRARY
-static void write_eof_packet(THD *thd, NET *net,
+static bool write_eof_packet(THD *thd, NET *net,
                              uint server_status, uint total_warn_count);
 #endif
 
@@ -70,8 +70,17 @@ bool Protocol_binary::net_store_data(con
   For SIGNAL/RESIGNAL and GET DIAGNOSTICS functionality it's
   critical that every error that can be intercepted is issued in one
   place only, my_message_sql.
+
+  @param thd Thread handler
+  @param sql_errno The error code to send
+  @param err A pointer to the error message
+
+  @return
+    @retval FALSE The message was sent to the client
+    @retval TRUE An error occurred and the message wasn't sent properly
 */
-void net_send_error(THD *thd, uint sql_errno, const char *err)
+
+bool net_send_error(THD *thd, uint sql_errno, const char *err)
 {
   DBUG_ENTER("net_send_error");
 
@@ -80,6 +89,7 @@ void net_send_error(THD *thd, uint sql_e
   DBUG_ASSERT(err && err[0]);
 
   DBUG_PRINT("enter",("sql_errno: %d  err: %s", sql_errno, err));
+  bool error;
 
   /*
     It's one case when we can push an error even though there
@@ -90,11 +100,11 @@ void net_send_error(THD *thd, uint sql_e
   /* Abort multi-result sets */
   thd->server_status&= ~SERVER_MORE_RESULTS_EXISTS;
 
-  net_send_error_packet(thd, sql_errno, err);
+  error= net_send_error_packet(thd, sql_errno, err);
 
   thd->main_da.can_overwrite_status= FALSE;
 
-  DBUG_VOID_RETURN;
+  DBUG_RETURN(error);
 }
 
 /**
@@ -113,25 +123,33 @@ void net_send_error(THD *thd, uint sql_e
   Is not stored if no message.
 
   @param thd		   Thread handler
+  @param server_status     The server status
+  @param total_warn_count  Total number of warnings
   @param affected_rows	   Number of rows changed by statement
   @param id		   Auto_increment id for first row (if used)
   @param message	   Message to send to the client (Used by mysql_status)
+ 
+  @return
+    @retval FALSE The message was successfully sent
+    @retval TRUE An error occurred and the messages wasn't sent properly
+
 */
 
 #ifndef EMBEDDED_LIBRARY
-void
+bool
 net_send_ok(THD *thd,
             uint server_status, uint total_warn_count,
             ha_rows 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
@@ -162,13 +180,14 @@ 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->main_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 */
@@ -188,37 +207,54 @@ static uchar eof_buff[1]= { (uchar) 254 
   client.
 
   @param thd		Thread handler
-  @param no_flush	Set to 1 if there will be more data to the client,
-                    like in send_fields().
+  @param server_status The server status
+  @param total_warn_count Total number of warnings
+
+  @return
+    @retval FALSE The message was successfully sent
+    @retval TRUE An error occurred and the message wasn't sent properly
 */    
 
-void
+bool
 net_send_eof(THD *thd, uint server_status, uint total_warn_count)
 {
   NET *net= &thd->net;
+  bool error= FALSE;
   DBUG_ENTER("net_send_eof");
   /* Set to TRUE if no active vio, to work well in case of --init-file */
   if (net->vio != 0)
   {
     thd->main_da.can_overwrite_status= TRUE;
-    write_eof_packet(thd, net, server_status, total_warn_count);
-    VOID(net_flush(net));
+    error= write_eof_packet(thd, net, server_status, total_warn_count);
+    if (!error)
+      error= test(net_flush(net));
     thd->main_da.can_overwrite_status= FALSE;
     DBUG_PRINT("info", ("EOF sent, so no more error sending allowed"));
   }
-  DBUG_VOID_RETURN;
+  DBUG_RETURN(error);
 }
 
 
 /**
   Format EOF packet according to the current protocol and
   write it to the network output buffer.
+
+  @param thd The thread handler
+  @param net The network handler
+  @param server_status The server status
+  @param total_warn_count The number of warnings
+
+
+  @return
+    @retval FALSE The message was sent successfully
+    @retval TRUE An error occurred and the messages wasn't sent properly
 */
 
-static void write_eof_packet(THD *thd, NET *net,
+static bool write_eof_packet(THD *thd, NET *net,
                              uint server_status,
                              uint total_warn_count)
 {
+  bool error;
   if (thd->client_capabilities & CLIENT_PROTOCOL_41)
   {
     uchar buff[5];
@@ -237,10 +273,12 @@ 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));
+    error= test(my_net_write(net, buff, 5));
   }
   else
-    VOID(my_net_write(net, eof_buff, 1));
+    error= test(my_net_write(net, eof_buff, 1));
+  
+  return error;
 }
 
 /**
@@ -261,7 +299,17 @@ bool send_old_password_request(THD *thd)
 }
 
 
-void net_send_error_packet(THD *thd, uint sql_errno, const char *err)
+/**
+  @param thd Thread handler
+  @param sql_errno The error code to send
+  @param err A pointer to the error message
+
+  @return
+   @retval FALSE The message was successfully sent
+   @retval TRUE  An error occurred and the messages wasn't sent properly
+*/
+
+bool net_send_error_packet(THD *thd, uint sql_errno, const char *err)
 {
   NET *net= &thd->net;
   uint length;
@@ -279,7 +327,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);
   }
 
   if (net->return_errno)
@@ -301,9 +349,8 @@ void net_send_error_packet(THD *thd, uin
     length=(uint) strlen(err);
     set_if_smaller(length,MYSQL_ERRMSG_SIZE-1);
   }
-  VOID(net_write_command(net,(uchar) 255, (uchar*) "", 0, (uchar*) err,
-                         length));
-  DBUG_VOID_RETURN;
+  DBUG_RETURN(test(net_write_command(net,(uchar) 255, (uchar*) "", 0,
+                                     (uchar*) err, length)));
 }
 
 #endif /* EMBEDDED_LIBRARY */
@@ -389,36 +436,46 @@ void net_end_statement(THD *thd)
   if (thd->main_da.is_sent)
     return;
 
+  bool error= FALSE;
+
   switch (thd->main_da.status()) {
   case Diagnostics_area::DA_ERROR:
     /* The query failed, send error to log and abort bootstrap. */
-    net_send_error(thd,
-                   thd->main_da.sql_errno(),
-                   thd->main_da.message());
+    error= net_send_error(thd,
+                          thd->main_da.sql_errno(),
+                          thd->main_da.message());
     break;
   case Diagnostics_area::DA_EOF:
-    net_send_eof(thd,
-                 thd->main_da.server_status(),
-                 thd->main_da.total_warn_count());
+    error= net_send_eof(thd,
+                        thd->main_da.server_status(),
+                        thd->main_da.total_warn_count());
     break;
   case Diagnostics_area::DA_OK:
-    net_send_ok(thd,
-                thd->main_da.server_status(),
-                thd->main_da.total_warn_count(),
-                thd->main_da.affected_rows(),
-                thd->main_da.last_insert_id(),
-                thd->main_da.message());
+    error= net_send_ok(thd,
+                       thd->main_da.server_status(),
+                       thd->main_da.total_warn_count(),
+                       thd->main_da.affected_rows(),
+                       thd->main_da.last_insert_id(),
+                       thd->main_da.message());
     break;
   case Diagnostics_area::DA_DISABLED:
     break;
   case Diagnostics_area::DA_EMPTY:
   default:
     DBUG_ASSERT(0);
-    net_send_ok(thd, thd->server_status, thd->total_warn_count,
-                0, 0, NULL);
+    error= net_send_ok(thd, thd->server_status, thd->total_warn_count,
+                       0, 0, NULL);
     break;
   }
-  thd->main_da.is_sent= TRUE;
+  if (!error)
+    thd->main_da.is_sent= TRUE;
+  else
+  {
+    /*
+      Drop the connection if the network reports an error.
+    */
+    thd->killed= THD::KILL_CONNECTION;
+  }
 }
 
 

=== modified file 'sql/protocol.h'
--- a/sql/protocol.h	2007-12-20 21:11:37 +0000
+++ b/sql/protocol.h	2009-07-27 16:08:36 +0000
@@ -173,7 +173,7 @@ public:
 };
 
 void send_warning(THD *thd, uint sql_errno, const char *err=0);
-void net_send_error(THD *thd, uint sql_errno=0, const char *err=0);
+bool net_send_error(THD *thd, uint sql_errno=0, const char *err=0);
 void net_end_statement(THD *thd);
 bool send_old_password_request(THD *thd);
 uchar *net_store_data(uchar *to,const uchar *from, size_t length);

=== modified file 'sql/sp_head.cc'
--- a/sql/sp_head.cc	2009-05-30 13:32:28 +0000
+++ b/sql/sp_head.cc	2009-07-27 16:08:36 +0000
@@ -1249,7 +1249,7 @@ sp_head::execute(THD *thd)
     */
     if (thd->prelocked_mode == NON_PRELOCKED)
       thd->user_var_events_alloc= thd->mem_root;
-    
+
     err_status= i->execute(thd, &ip);
 
     if (i->free_list)
@@ -2865,7 +2865,7 @@ sp_instr_stmt::execute(THD *thd, uint *n
     if (!thd->is_error())
       thd->main_da.reset_diagnostics_area();
   }
-  DBUG_RETURN(res);
+  DBUG_RETURN(res || thd->is_error());
 }
 
 

=== modified file 'sql/sql_parse.cc'
--- a/sql/sql_parse.cc	2009-06-05 11:23:58 +0000
+++ b/sql/sql_parse.cc	2009-07-27 16:08:36 +0000
@@ -791,6 +791,7 @@ bool do_command(THD *thd)
   thd->clear_error();				// Clear error message
   thd->main_da.reset_diagnostics_area();
 
+
   net_new_transaction(net);
 
   packet_length= my_net_read(net);
@@ -4968,6 +4969,10 @@ static bool execute_sqlcom_select(THD *t
   LEX	*lex= thd->lex;
   select_result *result=lex->result;
   bool res;
+
+  DBUG_EXECUTE_IF("net_real_write_failure",
+    DBUG_SET("+d,net_real_write_failure2"););
+
   /* assign global limit variable if limit is not given */
   {
     SELECT_LEX *param= lex->unit.global_parameters;


Attachment: [text/bzr-bundle]
Thread
bzr commit into mysql-5.1-bugteam branch (kristofer.pettersson:2935)Bug#44521Kristofer Pettersson27 Jul
  • Re: bzr commit into mysql-5.1-bugteam branch(kristofer.pettersson:2935) Bug#44521Davi Arnaut27 Jul
    • Re: bzr commit into mysql-5.1-bugteam branch(kristofer.pettersson:2935) Bug#44521Kristofer Pettersson28 Jul