Bug#30273 - merge tables: Can't lock file (errno: 155)

The patch for Bug 26379 (Combination of FLUSH TABLE and
REPAIR TABLE corrupts a MERGE table) fixed this bug too.
However it revealed a new bug that crashed the server.

Flushing a merge table at the moment when it is between open
and attach of children crashed the server.

The flushing thread wants to abort locks on the flushed table.
It calls ha_myisammrg::lock_count() and ha_myisammrg::store_lock()
on the TABLE object of the other thread.

Changed ha_myisammrg::lock_count() and ha_myisammrg::store_lock()
to accept non-attached children. ha_myisammrg::lock_count() returns
the number of MyISAM tables in the MERGE table so that the memory
allocation done by get_lock_data() is done correctly, even if the
children become attached before ha_myisammrg::store_lock() is
called. ha_myisammrg::store_lock() will not return any lock if the
children are not attached.

This is however a change in the handler interface. lock_count()
can now return a higher number than store_lock() stores locks.
This is more safe than the reverse implementation would be.
get_lock_data() in the SQL layer is adjusted accordingly. It sets
MYSQL_LOCK::lock_count based on the number of locks returned by
the handler::store_lock() calls, not based on the numbers returned
by the handler::lock_count() calls. The latter are only used for
allocation of memory now.

No test case. The test suite cannot reliably run FLUSH between
lock_count() and store_lock() of another thread. The bug report
contains a program that can repeat the problem with some
probability.

1 files changed, 17 insertions, 3 deletions

diff --git a/sql/lock.cc b/sql/lock.cc
index 29a07858bc1..ef4a0cc3d83 100644
--- a/sql/lock.cc
+++ b/sql/lock.cc
@@ -847,9 +847,6 @@ static MYSQL_LOCK *get_lock_data(THD *thd, TABLE **table_ptr, uint count,
   locks= locks_buf= sql_lock->locks= (THR_LOCK_DATA**) (sql_lock + 1);
   to= table_buf= sql_lock->table= (TABLE**) (locks + tables * 2);
   sql_lock->table_count=lock_count;
-  sql_lock->lock_count=tables;
-  DBUG_PRINT("info", ("sql_lock->table_count %d sql_lock->lock_count %d",
-                      sql_lock->table_count, sql_lock->lock_count));
 
   for (i=0 ; i < count ; i++)
   {
@@ -889,6 +886,23 @@ static MYSQL_LOCK *get_lock_data(THD *thd, TABLE **table_ptr, uint count,
       for ( ; org_locks != locks ; org_locks++)
 	(*org_locks)->debug_print_param= (void *) table;
   }
+  /*
+    We do not use 'tables', because there are cases where store_lock()
+    returns less locks than lock_count() claimed. This can happen when
+    a FLUSH TABLES tries to abort locks from a MERGE table of another
+    thread. When that thread has just opened the table, but not yet
+    attached its children, it cannot return the locks. lock_count()
+    always returns the number of locks that an attached table has.
+    This is done to avoid the reverse situation: If lock_count() would
+    return 0 for a non-attached MERGE table, and that table becomes
+    attached between the calls to lock_count() and store_lock(), then
+    we would have allocated too little memory for the lock data. Now
+    we may allocate too much, but better safe than memory overrun.
+    And in the FLUSH case, the memory is released quickly anyway.
+  */
+  sql_lock->lock_count= locks - locks_buf;
+  DBUG_PRINT("info", ("sql_lock->table_count %d sql_lock->lock_count %d",
+                      sql_lock->table_count, sql_lock->lock_count));
   DBUG_RETURN(sql_lock);
 }