Fixed "Packets out of order" warning message on stdout in clients,

compiled for debugging, when the server goes down This happens in the following scenario: - Server gets a shutdown message - Servers sends error ER_CONNECTION_KILLED to the clients connection - The client sends a query to the server, before the server has time to close the connection to the client - Client reads the ER_CONNECTION_KILLED error message In the above case, the packet number for the reply is one less than what the client expected and the client prints "Packets out of order". Fixed the following way: - The client accepts now error packages with a packet number one less than expected. - To ensure that this issue can be detected early in my_real_read(), error messages sent to the client are not compressed, even when compressed protocol is used.

Author: montywi

include/mysql.h.pp

@@ -27,7 +27,7 @@
  char save_char;
  char net_skip_rest_factor;
  my_bool thread_specific_malloc;
- my_bool compress;
+ unsigned char compress;
  my_bool unused3;
  unsigned char *unused;
  unsigned int last_errno;

include/mysql_com.h

@@ -386,7 +386,7 @@ typedef struct st_net {
  char save_char;
  char net_skip_rest_factor;
  my_bool thread_specific_malloc;
- my_bool compress;
+ unsigned char compress;
  my_bool unused3; /* Please remove with the next incompatible ABI change. */
  /*
  Pointer to query object in query cache, do not equal NULL (0) for

mysql-test/include/wait_until_connected_again.inc

@@ -14,7 +14,7 @@ while ($mysql_errno)
  # Strangely enough, the server might return "Too many connections"
  # while being shutdown, thus 1040 is an "allowed" error
  # See BUG#36228
- --error 0,1040,1053,2002,2003,2005,2006,2013
+ --error 0,1040,1053,2002,2003,2005,2006,2013,1927
  show status;
 
  dec $counter;

mysql-test/include/wait_until_disconnected.inc

@@ -12,7 +12,7 @@ while (!$mysql_errno)
  # Strangely enough, the server might return "Too many connections"
  # while being shutdown, thus 1040 is an "allowed" error.
  # See BUG#36228.
- --error 0,1040,1053,2002,2003,2005,2006,2013
+ --error 0,1040,1053,2002,2003,2005,2006,2013,1927
  show status;
 
  dec $counter;

sql/net_serv.cc

@@ -540,7 +540,7 @@ net_write_buff(NET *net, const uchar *packet, ulong len)
  left_length= (ulong) (net->buff_end - net->write_pos);
 
 #ifdef DEBUG_DATA_PACKETS
- DBUG_DUMP("data", packet, len);
+ DBUG_DUMP("data_written", packet, len);
 #endif
  if (len > left_length)
  {
@@ -629,7 +629,8 @@ net_real_write(NET *net,const uchar *packet, size_t len)
  }
  memcpy(b+header_length,packet,len);
 
- if (my_compress(b+header_length, &len, &complen))
+ /* Don't compress error packets (compress == 2) */
+ if (net->compress == 2 || my_compress(b+header_length, &len, &complen))
  complen=0;
  int3store(&b[NET_HEADER_SIZE],complen);
  int3store(b,len);
@@ -640,7 +641,7 @@ net_real_write(NET *net,const uchar *packet, size_t len)
 #endif /* HAVE_COMPRESS */
 
 #ifdef DEBUG_DATA_PACKETS
- DBUG_DUMP("data", packet, len);
+ DBUG_DUMP("data_written", packet, len);
 #endif
 
 #ifndef NO_ALARM
@@ -830,6 +831,7 @@ my_real_read(NET *net, size_t *complen,
  size_t length;
  uint i,retry_count=0;
  ulong len=packet_error;
+ my_bool expect_error_packet __attribute((unused))= 0;
  thr_alarm_t alarmed;
 #ifndef NO_ALARM
  ALARM alarm_buff;
@@ -878,6 +880,7 @@ my_real_read(NET *net, size_t *complen,
 
  if (i== 0 && thd_net_is_killed())
  {
+ DBUG_PRINT("info", ("thd is killed"));
  len= packet_error;
  net->error= 0;
  net->last_errno= ER_CONNECTION_KILLED;
@@ -947,39 +950,34 @@ my_real_read(NET *net, size_t *complen,
 pos+= length;
 update_statistics(thd_increment_bytes_received(length));
  }
+
+#ifdef DEBUG_DATA_PACKETS
+ DBUG_DUMP("data_read", net->buff+net->where_b, length);
+#endif
  if (i == 0)
  {/* First parts is packet length */
 ulong helping;
+#ifndef DEBUG_DATA_PACKETS
  DBUG_DUMP("packet_header", net->buff+net->where_b,
  NET_HEADER_SIZE);
+#endif
 if (net->buff[net->where_b + 3] != (uchar) net->pkt_nr)
-{
- if (net->buff[net->where_b] != (uchar) 255)
- {
- DBUG_PRINT("error",
- ("Packets out of order (Found: %d, expected %u)",
-(int) net->buff[net->where_b + 3],
-net->pkt_nr));
- /* 
- We don't make noise server side, since the client is expected
- to break the protocol for e.g. --send LOAD DATA .. LOCAL where
- the server expects the client to send a file, but the client
- may reply with a new command instead.
- */
+ {
 #ifndef MYSQL_SERVER
- EXTRA_DEBUG_fflush(stdout);
- EXTRA_DEBUG_fprintf(stderr,"Error: Packets out of order (Found: %d, expected %d)\n",
- (int) net->buff[net->where_b + 3],
- (uint) (uchar) net->pkt_nr);
- EXTRA_DEBUG_fflush(stderr);
+ if (net->buff[net->where_b + 3] == (uchar) (net->pkt_nr -1))
+ {
+ /*
+ If the server was killed then the server may have missed the
+ last sent client packet and the packet numbering may be one off.
+ */
+ DBUG_PRINT("warning", ("Found possible out of order packets"));
+ expect_error_packet= 1;
+ }
+ else
 #endif
- }
- len= packet_error;
- /* Not a NET error on the client. XXX: why? */
- MYSQL_SERVER_my_error(ER_NET_PACKETS_OUT_OF_ORDER, MYF(0));
- goto end;
-}
-net->compress_pkt_nr= ++net->pkt_nr;
+ goto packets_out_of_order;
+ }
+ net->compress_pkt_nr= ++net->pkt_nr;
 #ifdef HAVE_COMPRESS
 if (net->compress)
 {
@@ -1027,6 +1025,21 @@ my_real_read(NET *net, size_t *complen,
  }
 #endif
  }
+#ifndef MYSQL_SERVER
+ else if (expect_error_packet)
+ {
+ /*
+ This check is safe both for compressed and not compressed protocol
+ as for the compressed protocol errors are not compressed anymore.
+ */
+ if (net->buff[net->where_b] != (uchar) 255)
+ {
+ /* Restore pkt_nr to original value */
+ net->pkt_nr--;
+ goto packets_out_of_order;
+ }
+ }
+#endif
  }
 
 end:
@@ -1040,7 +1053,7 @@ my_real_read(NET *net, size_t *complen,
  net->reading_or_writing=0;
 #ifdef DEBUG_DATA_PACKETS
  if (len != packet_error)
- DBUG_DUMP("data", net->buff+net->where_b, len);
+ DBUG_DUMP("data_read", net->buff+net->where_b, len);
 #endif
 #ifdef MYSQL_SERVER
  if (server_extension != NULL)
@@ -1051,9 +1064,35 @@ my_real_read(NET *net, size_t *complen,
  }
 #endif
  return(len);
+
+packets_out_of_order:
+ {
+ DBUG_PRINT("error",
+ ("Packets out of order (Found: %d, expected %u)",
+ (int) net->buff[net->where_b + 3],
+ net->pkt_nr));
+ DBUG_ASSERT(0);
+ /*
+ We don't make noise server side, since the client is expected
+ to break the protocol for e.g. --send LOAD DATA .. LOCAL where
+ the server expects the client to send a file, but the client
+ may reply with a new command instead.
+ */
+#ifndef MYSQL_SERVER
+ EXTRA_DEBUG_fflush(stdout);
+ EXTRA_DEBUG_fprintf(stderr,"Error: Packets out of order (Found: %d, expected %d)\n",
+ (int) net->buff[net->where_b + 3],
+ (uint) (uchar) net->pkt_nr);
+ EXTRA_DEBUG_fflush(stderr);
+#endif
+ len= packet_error;
+ MYSQL_SERVER_my_error(ER_NET_PACKETS_OUT_OF_ORDER, MYF(0));
+ goto end;
+ }
 }
 
 
+
 /* Old interface. See my_net_read_packet() for function description */
 
 #undef my_net_read

sql/protocol.cc

@@ -373,7 +373,8 @@ bool net_send_error_packet(THD *thd, uint sql_errno, const char *err,
  uint error;
  char converted_err[MYSQL_ERRMSG_SIZE];
  char buff[2+1+SQLSTATE_LENGTH+MYSQL_ERRMSG_SIZE], *pos;
-
+ my_bool ret;
+ uint8 save_compress;
  DBUG_ENTER("send_error_packet");
 
  if (net->vio == 0)
@@ -401,8 +402,16 @@ bool net_send_error_packet(THD *thd, uint sql_errno, const char *err,
  /* Converted error message is always null-terminated. */
  length= (uint) (strmake(pos, converted_err, MYSQL_ERRMSG_SIZE - 1) - buff);
 
- DBUG_RETURN(net_write_command(net,(uchar) 255, (uchar*) "", 0, (uchar*) buff,
- length));
+ /*
+ Ensure that errors are not compressed. This is to ensure we can
+ detect out of bands error messages in the client
+ */
+ if ((save_compress= net->compress))
+ net->compress= 2;
+ ret= net_write_command(net,(uchar) 255, (uchar*) "", 0, (uchar*) buff,
+ length);
+ net->compress= save_compress;
+ DBUG_RETURN(ret);
 }
 
 #endif /* EMBEDDED_LIBRARY */