Bug#17757914 MEMORY CORRUPTION/CRASH/ASSERTION FAILED: RECORD_LENGTH == M_RECORD_LENGTH

Merge 5.6 => trunk, the re-implement.

Author: Tor Didriksen

sql/filesort.cc

@@ -342,19 +342,7 @@ ha_rows filesort(THD *thd, TABLE *table, Filesort *filesort,
       ha_rows keys= memory_available / (param.rec_length + sizeof(char*));
       // If the table is empty, allocate space for one row.
       param.max_keys_per_buffer= (uint) min(num_rows > 0 ? num_rows : 1, keys);
-      if (table_sort.sort_buffer_size() > 0)
-      {
-        // If we have already allocated a buffer, it better have same size!
-        if (std::make_pair(param.max_keys_per_buffer, param.rec_length) !=
-            table_sort.sort_buffer_properties())
-        {
-          /*
-            table->sort will still have a pointer to the same buffer,
-            but that will be overwritten by the assignment below.
-          */
-          table_sort.free_sort_buffer();
-        }
-      }
+
       table_sort.alloc_sort_buffer(param.max_keys_per_buffer, param.rec_length);
       if (table_sort.sort_buffer_size() > 0)
         break;

sql/filesort_utils.cc

@@ -93,15 +93,26 @@ uchar *Filesort_buffer::alloc_sort_buffer(uint num_records, uint record_length)
   DBUG_EXECUTE_IF("alloc_sort_buffer_fail",
                   DBUG_SET("+d,simulate_out_of_memory"););
 
+  /*
+    For subqueries we try to re-use the buffer, in order to save
+    expensive malloc/free calls. Both of the sizing parameters may change:
+    - num_records due to e.g. different statistics from the engine.
+    - record_length due to different buffer usage:
+      a heap table may be flushed to myisam, which allows us to sort by
+      <key, addon fields> rather than <key, rowid>
+    If we already have a buffer, but with wrong size, we simply delete it.
+   */
   if (m_rawmem != NULL)
   {
-    DBUG_ASSERT(num_records == m_num_records);
-    DBUG_ASSERT(record_length == m_record_length);
-    return m_rawmem;
+    if (num_records != m_num_records ||
+        record_length != m_record_length)
+      free_sort_buffer();
   }
+
   m_size_in_bytes= ALIGN_SIZE(num_records * (record_length + sizeof(uchar*)));
-  m_rawmem= (uchar*) my_malloc(key_memory_Filesort_buffer_sort_keys,
-                               m_size_in_bytes, MYF(0));
+  if (m_rawmem == NULL)
+    m_rawmem= (uchar*) my_malloc(key_memory_Filesort_buffer_sort_keys,
+                                 m_size_in_bytes, MYF(0));
   if (m_rawmem == NULL)
   {
     m_size_in_bytes= 0;

sql/filesort_utils.h

@@ -194,12 +194,6 @@ class Filesort_buffer
   */
   uchar *alloc_sort_buffer(uint num_records, uint record_length);
 
-  /// What is the <num_records, record_length> for the buffer?
-  std::pair<uint, uint> sort_buffer_properties() const
-  {
-    return std::make_pair(m_num_records, m_record_length);
-  }
-
   /// Frees the buffer.
   void free_sort_buffer()
   {

sql/sql_sort.h

@@ -491,9 +491,6 @@ class Filesort_info
   uchar *alloc_sort_buffer(uint num_records, uint record_length)
   { return filesort_buffer.alloc_sort_buffer(num_records, record_length); }
 
-  std::pair<uint, uint> sort_buffer_properties() const
-  { return filesort_buffer.sort_buffer_properties(); }
-
   void free_sort_buffer()
   { filesort_buffer.free_sort_buffer(); }
 

unittest/gunit/filesort_buffer-t.cc

@@ -30,9 +30,6 @@ class FileSortBufferTest : public ::testing::Test
   virtual void TearDown()
   {
     fs_info.free_sort_buffer();
-    std::pair<uint, uint> buffer_properties= fs_info.sort_buffer_properties();
-    EXPECT_EQ(0U, buffer_properties.first);
-    EXPECT_EQ(0U, buffer_properties.second);
     EXPECT_TRUE(NULL == fs_info.get_sort_keys());
   }
 
@@ -43,15 +40,8 @@ class FileSortBufferTest : public ::testing::Test
 TEST_F(FileSortBufferTest, FileSortBuffer)
 {
   const char letters[10]= "abcdefghi";
-  std::pair<uint, uint> buffer_properties= fs_info.sort_buffer_properties();
-  EXPECT_EQ(0U, buffer_properties.first);
-  EXPECT_EQ(0U, buffer_properties.second);
 
   uchar *sort_buff= fs_info.alloc_sort_buffer(10, sizeof(char));
-  buffer_properties= fs_info.sort_buffer_properties();
-  EXPECT_EQ(10U, buffer_properties.first);
-  EXPECT_EQ(sizeof(char), buffer_properties.second);
-
 
   const uchar *null_sort_buff= NULL;
   const uchar **null_sort_keys= NULL;