Bug#19604 - CHECK TABLE with concurrent INSERT can reset auto_increment

CHECK TABLE did temporarily clear the auto_increment value.
It runs with a read lock, allowing other readers and
conurrent INSERTs. The latter could grab the wrong value
in this moment.

CHECK TABLE does no longer modify the auto_increment value.
Not even for a short moment.


myisam/mi_check.c:
  Bug#19604 - CHECK TABLE with concurrent INSERT can reset auto_increment
  In chk_key() and update_auto_increment_key() in the repair_only
  case, do not touch info->s->state.auto_increment. Especially
  chk_key() can be called from CHECK TABLE with a read lock.
  Concurrent inserts could grab a temporarily changed value.
  Added minor style fixes.
myisam/mi_key.c:
  Bug#19604 - CHECK TABLE with concurrent INSERT can reset auto_increment
  Changed update_auto_increment() to retrieve_auto_increment()
  to reflect that it does not change the auto_increment by
  itself any more. This must now be done externally if needed.
myisam/mi_update.c:
  Bug#19604 - CHECK TABLE with concurrent INSERT can reset auto_increment
  Added explicit update of info->s->state.auto_increment
  after the change from update_auto_increment() to
  retrieve_auto_increment().
myisam/mi_write.c:
  Bug#19604 - CHECK TABLE with concurrent INSERT can reset auto_increment
  Added explicit update of info->s->state.auto_increment
  after the change from update_auto_increment() to
  retrieve_auto_increment().
myisam/myisamdef.h:
  Bug#19604 - CHECK TABLE with concurrent INSERT can reset auto_increment
  Changed update_auto_increment() to retrieve_auto_increment()
  to reflect that it does not change the auto_increment by
  itself any more. This must now be done externally if needed.

5 files changed, 27 insertions, 29 deletions

diff --git a/myisam/mi_check.c b/myisam/mi_check.c
index 7d87ecd8595..e0f04965650 100644
--- a/myisam/mi_check.c
+++ b/myisam/mi_check.c
@@ -453,25 +453,24 @@ int chk_key(MI_CHECK *param, register MI_INFO *info)
     if ((uint) share->base.auto_key -1 == key)
     {
       /* Check that auto_increment key is bigger than max key value */
-      ulonglong save_auto_value=info->s->state.auto_increment;
-      info->s->state.auto_increment=0;
+      ulonglong auto_increment;
       info->lastinx=key;
       _mi_read_key_record(info, 0L, info->rec_buff);
-      update_auto_increment(info, info->rec_buff);
-      if (info->s->state.auto_increment > save_auto_value)
+      auto_increment= retrieve_auto_increment(info, info->rec_buff);
+      if (auto_increment > info->s->state.auto_increment)
       {
-	mi_check_print_warning(param,
-			       "Auto-increment value: %s is smaller than max used value: %s",
-			       llstr(save_auto_value,buff2),
-			       llstr(info->s->state.auto_increment, buff));
+        mi_check_print_warning(param, "Auto-increment value: %s is smaller "
+                               "than max used value: %s",
+                               llstr(info->s->state.auto_increment,buff2),
+                               llstr(auto_increment, buff));
       }
       if (param->testflag & T_AUTO_INC)
       {
-	set_if_bigger(info->s->state.auto_increment,
-		      param->auto_increment_value);
+        set_if_bigger(info->s->state.auto_increment,
+                      auto_increment);
+        set_if_bigger(info->s->state.auto_increment,
+                      param->auto_increment_value);
       }
-      else
-	info->s->state.auto_increment=save_auto_value;
 
       /* Check that there isn't a row with auto_increment = 0 in the table */
       mi_extra(info,HA_EXTRA_KEYREAD,0);
@@ -481,8 +480,8 @@ int chk_key(MI_CHECK *param, register MI_INFO *info)
       {
 	/* Don't count this as a real warning, as myisamchk can't correct it */
 	uint save=param->warning_printed;
-	mi_check_print_warning(param,
-			       "Found row where the auto_increment column has the value 0");
+        mi_check_print_warning(param, "Found row where the auto_increment "
+                               "column has the value 0");
 	param->warning_printed=save;
       }
       mi_extra(info,HA_EXTRA_NO_KEYREAD,0);
@@ -4099,11 +4098,10 @@ void update_auto_increment_key(MI_CHECK *param, MI_INFO *info,
   }
   else
   {
-    ulonglong auto_increment= (repair_only ? info->s->state.auto_increment :
-			       param->auto_increment_value);
-    info->s->state.auto_increment=0;
-    update_auto_increment(info, record);
+    ulonglong auto_increment= retrieve_auto_increment(info, record);
     set_if_bigger(info->s->state.auto_increment,auto_increment);
+    if (!repair_only)
+      set_if_bigger(info->s->state.auto_increment, param->auto_increment_value);
   }
   mi_extra(info,HA_EXTRA_NO_KEYREAD,0);
   my_free((char*) record, MYF(0));
diff --git a/myisam/mi_key.c b/myisam/mi_key.c
index 717a5dbd56e..17ea56f0210 100644
--- a/myisam/mi_key.c
+++ b/myisam/mi_key.c
@@ -507,22 +507,21 @@ int _mi_read_key_record(MI_INFO *info, my_off_t filepos, byte *buf)
   return(-1);				/* Wrong data to read */
 }
 
-  
+
 /*
-  Update auto_increment info
+  Retrieve auto_increment info
 
   SYNOPSIS
-    update_auto_increment()
+    retrieve_auto_increment()
     info			MyISAM handler
     record			Row to update
 
   IMPLEMENTATION
-    Only replace the auto_increment value if it is higher than the previous
-    one. For signed columns we don't update the auto increment value if it's
+    For signed columns we don't retrieve the auto increment value if it's
     less than zero.
 */
 
-void update_auto_increment(MI_INFO *info,const byte *record)
+ulonglong retrieve_auto_increment(MI_INFO *info,const byte *record)
 {
   ulonglong value= 0;			/* Store unsigned values here */
   longlong s_value= 0;			/* Store signed values here */
@@ -587,6 +586,5 @@ void update_auto_increment(MI_INFO *info,const byte *record)
     and if s_value == 0 then value will contain either s_value or the
     correct value.
   */
-  set_if_bigger(info->s->state.auto_increment,
-                (s_value > 0) ? (ulonglong) s_value : value);
+  return (s_value > 0) ? (ulonglong) s_value : value;
 }
diff --git a/myisam/mi_update.c b/myisam/mi_update.c
index 937c9983b45..f8b5cf55406 100644
--- a/myisam/mi_update.c
+++ b/myisam/mi_update.c
@@ -164,7 +164,8 @@ int mi_update(register MI_INFO *info, const byte *oldrec, byte *newrec)
       key_changed|= HA_STATE_CHANGED;		/* Must update index file */
   }
   if (auto_key_changed)
-    update_auto_increment(info,newrec);
+    set_if_bigger(info->s->state.auto_increment,
+                  retrieve_auto_increment(info, newrec));
   if (share->calc_checksum)
     info->state->checksum+=(info->checksum - old_checksum);
 
diff --git a/myisam/mi_write.c b/myisam/mi_write.c
index 5e79b2937cc..9ab8753f6d7 100644
--- a/myisam/mi_write.c
+++ b/myisam/mi_write.c
@@ -149,7 +149,8 @@ int mi_write(MI_INFO *info, byte *record)
     info->state->checksum+=info->checksum;
   }
   if (share->base.auto_key)
-    update_auto_increment(info,record);
+    set_if_bigger(info->s->state.auto_increment,
+                  retrieve_auto_increment(info, record));
   info->update= (HA_STATE_CHANGED | HA_STATE_AKTIV | HA_STATE_WRITTEN |
 		 HA_STATE_ROW_CHANGED);
   info->state->records++;
diff --git a/myisam/myisamdef.h b/myisam/myisamdef.h
index 6ccb52aff22..d589173f0e7 100644
--- a/myisam/myisamdef.h
+++ b/myisam/myisamdef.h
@@ -582,7 +582,7 @@ extern uint _mi_pack_key(MI_INFO *info,uint keynr,uchar *key,uchar *old,
 extern int _mi_read_key_record(MI_INFO *info,my_off_t filepos,byte *buf);
 extern int _mi_read_cache(IO_CACHE *info,byte *buff,my_off_t pos,
 			  uint length,int re_read_if_possibly);
-extern void update_auto_increment(MI_INFO *info,const byte *record);
+extern ulonglong retrieve_auto_increment(MI_INFO *info,const byte *record);
 
 extern byte *mi_alloc_rec_buff(MI_INFO *,ulong, byte**);
 #define mi_get_rec_buff_ptr(info,buf)                              \