List:Commits« Previous MessageNext Message »
From:Kristofer Pettersson Date:July 16 2009 10:31pm
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-17
      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 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_class.cc
        * If always reset is_sent if a new error is written over current state in the DA.

    modified:
      sql/protocol.cc
      sql/protocol.h
      sql/sp_head.cc
      sql/sql_class.cc
=== modified file 'sql/protocol.cc'
--- a/sql/protocol.cc	2009-03-24 13:58:52 +0000
+++ b/sql/protocol.cc	2009-07-16 22:31:53 +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));
+  int 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;
+  int 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,55 @@ 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");
+  DBUG_PRINT("huggla",("server_status= %x",server_status));
   /* 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= 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)
 {
+  int error;
   if (thd->client_capabilities & CLIENT_PROTOCOL_41)
   {
     uchar buff[5];
@@ -237,10 +274,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= my_net_write(net, buff, 5);
   }
   else
-    VOID(my_net_write(net, eof_buff, 1));
+    error= my_net_write(net, eof_buff, 1);
+  
+  return error;
 }
 
 /**
@@ -261,7 +300,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 +328,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 +350,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,
+  DBUG_RETURN(net_write_command(net,(uchar) 255, (uchar*) "", 0, (uchar*) err,
                          length));
-  DBUG_VOID_RETURN;
 }
 
 #endif /* EMBEDDED_LIBRARY */
@@ -389,36 +437,39 @@ 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;
 }
 
 

=== modified file 'sql/protocol.h'
--- a/sql/protocol.h	2007-12-20 21:11:37 +0000
+++ b/sql/protocol.h	2009-07-16 22:31:53 +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-16 22:31:53 +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_class.cc'
--- a/sql/sql_class.cc	2009-06-08 22:05:24 +0000
+++ b/sql/sql_class.cc	2009-07-16 22:31:53 +0000
@@ -511,6 +511,14 @@ Diagnostics_area::set_error_status(THD *
 
   m_sql_errno= sql_errno_arg;
   strmake(m_message, message_arg, sizeof(m_message) - 1);
+  /*
+    If we're overwriting the status, unset the 'is_sent' flag,
+    since effectively we get a new message into the DA
+    on top of the one that is being sent. This can happen
+    when we're inside net_end_statement().
+  */
+  if (is_set() && is_sent)
+    is_sent= FALSE;
 
   m_status= DA_ERROR;
 }


Attachment: [text/bzr-bundle]
Thread
bzr commit into mysql-5.1-bugteam branch (kristofer.pettersson:2935)Bug#44521Kristofer Pettersson17 Jul