Discoved while debugging in 5.1 that there was a bug where a certain crash could lead to two problems. 1) An additional share in memory that was allocated but did not have the correct use_count (so it would never be fulled deleted). Also discovered that a thread that called repair would write new rows, but would not see them. All other threads were ok, and the data was fine, but the thread doing the repair was unable to see the new rows.

sql/ha_archive.cc:
  Fix for leaked share and hidden rows.
sql/ha_archive.h:
  Change in method

1 files changed, 58 insertions, 10 deletions

diff --git a/sql/ha_archive.cc b/sql/ha_archive.cc
index cfdaba0672c..cad81926a72 100644
--- a/sql/ha_archive.cc
+++ b/sql/ha_archive.cc
@@ -366,12 +366,14 @@ int ha_archive::write_meta_file(File meta_file, ha_rows rows, bool dirty)
 
   See ha_example.cc for a longer description.
 */
-ARCHIVE_SHARE *ha_archive::get_share(const char *table_name, TABLE *table)
+ARCHIVE_SHARE *ha_archive::get_share(const char *table_name, 
+                                     TABLE *table, int *rc)
 {
   ARCHIVE_SHARE *share;
   char meta_file_name[FN_REFLEN];
   uint length;
   char *tmp_name;
+  DBUG_ENTER("ha_archive::get_share");
 
   pthread_mutex_lock(&archive_mutex);
   length=(uint) strlen(table_name);
@@ -386,7 +388,8 @@ ARCHIVE_SHARE *ha_archive::get_share(const char *table_name, TABLE *table)
                           NullS)) 
     {
       pthread_mutex_unlock(&archive_mutex);
-      return NULL;
+      *rc= HA_ERR_OUT_OF_MEM;
+      DBUG_RETURN(NULL);
     }
 
     share->use_count= 0;
@@ -424,9 +427,14 @@ ARCHIVE_SHARE *ha_archive::get_share(const char *table_name, TABLE *table)
     thr_lock_init(&share->lock);
   }
   share->use_count++;
+  DBUG_PRINT("info", ("archive table %.*s has %d open handles now", 
+                      share->table_name_length, share->table_name,
+                      share->use_count));
+  if (share->crashed)
+    *rc= HA_ERR_CRASHED_ON_USAGE;
   pthread_mutex_unlock(&archive_mutex);
 
-  return share;
+  DBUG_RETURN(share);
 }
 
 
@@ -437,13 +445,21 @@ ARCHIVE_SHARE *ha_archive::get_share(const char *table_name, TABLE *table)
 int ha_archive::free_share(ARCHIVE_SHARE *share)
 {
   int rc= 0;
+  DBUG_ENTER("ha_archive::free_share");
+  DBUG_PRINT("info", ("archive table %.*s has %d open handles on entrance", 
+                      share->table_name_length, share->table_name,
+                      share->use_count));
+
   pthread_mutex_lock(&archive_mutex);
   if (!--share->use_count)
   {
     hash_delete(&archive_open_tables, (byte*) share);
     thr_lock_delete(&share->lock);
     VOID(pthread_mutex_destroy(&share->mutex));
-    (void)write_meta_file(share->meta_file, share->rows_recorded, FALSE);
+    if (share->crashed)
+      (void)write_meta_file(share->meta_file, share->rows_recorded, TRUE);
+    else
+      (void)write_meta_file(share->meta_file, share->rows_recorded, FALSE);
     if (gzclose(share->archive_write) == Z_ERRNO)
       rc= 1;
     if (my_close(share->meta_file, MYF(0)))
@@ -452,7 +468,7 @@ int ha_archive::free_share(ARCHIVE_SHARE *share)
   }
   pthread_mutex_unlock(&archive_mutex);
 
-  return rc;
+  DBUG_RETURN(rc);
 }
 
 
@@ -479,10 +495,23 @@ const char **ha_archive::bas_ext() const
 */
 int ha_archive::open(const char *name, int mode, uint open_options)
 {
+  int rc= 0;
   DBUG_ENTER("ha_archive::open");
 
-  if (!(share= get_share(name, table)))
-    DBUG_RETURN(HA_ERR_OUT_OF_MEM); // Not handled well by calling code!
+  DBUG_PRINT("info", ("archive table was opened for crash %s", 
+                      (open_options & HA_OPEN_FOR_REPAIR) ? "yes" : "no"));
+  share= get_share(name, table, &rc);
+
+  if (rc == HA_ERR_CRASHED_ON_USAGE && !(open_options & HA_OPEN_FOR_REPAIR))
+  {
+    free_share(share);
+    DBUG_RETURN(rc);
+  }
+  else if (rc == HA_ERR_OUT_OF_MEM)
+  {
+    DBUG_RETURN(rc);
+  }
+
   thr_lock_data_init(&share->lock,&lock,NULL);
 
   if ((archive= gzopen(share->data_file_name, "rb")) == NULL)
@@ -492,10 +521,14 @@ int ha_archive::open(const char *name, int mode, uint open_options)
     DBUG_RETURN(HA_ERR_CRASHED_ON_USAGE);
   }
 
-  if (open_options & HA_OPEN_FOR_REPAIR)
+  DBUG_PRINT("info", ("archive table was crashed %s", 
+                      rc == HA_ERR_CRASHED_ON_USAGE ? "yes" : "no"));
+  if (rc == HA_ERR_CRASHED_ON_USAGE && open_options & HA_OPEN_FOR_REPAIR)
+  {
     DBUG_RETURN(0);
-
-  DBUG_RETURN(share->crashed ? HA_ERR_CRASHED_ON_USAGE : 0);
+  }
+  else
+    DBUG_RETURN(rc);
 }
 
 
@@ -684,6 +717,7 @@ int ha_archive::rnd_init(bool scan)
   if (scan)
   {
     scan_rows= share->rows_recorded;
+    DBUG_PRINT("info", ("archive will retrieve %llu rows", scan_rows));
     records= 0;
 
     /* 
@@ -695,6 +729,7 @@ int ha_archive::rnd_init(bool scan)
       pthread_mutex_lock(&share->mutex);
       if (share->dirty == TRUE)
       {
+        DBUG_PRINT("info", ("archive flushing out rows for scan"));
         gzflush(share->archive_write, Z_SYNC_FLUSH);
         share->dirty= FALSE;
       }
@@ -913,6 +948,7 @@ int ha_archive::optimize(THD* thd, HA_CHECK_OPT* check_opt)
         share->rows_recorded++;
       }
     }
+    DBUG_PRINT("info", ("recovered %llu archive rows", share->rows_recorded));
 
     my_free((char*)buf, MYF(0));
     if (rc && rc != HA_ERR_END_OF_FILE)
@@ -936,11 +972,23 @@ int ha_archive::optimize(THD* thd, HA_CHECK_OPT* check_opt)
   }
 
   gzflush(writer, Z_SYNC_FLUSH);
+  share->dirty= FALSE;
   gzclose(share->archive_write);
   share->archive_write= writer; 
 
   my_rename(writer_filename,share->data_file_name,MYF(0));
 
+  /*
+    Now we need to reopen our read descriptor since it has changed.
+  */
+  gzclose(archive);
+  if ((archive= gzopen(share->data_file_name, "rb")) == NULL)
+  {
+    rc= HA_ERR_CRASHED_ON_USAGE;
+    goto error;
+  }
+
+
   DBUG_RETURN(0); 
 
 error: