MDEV-12761 Error return from external_lock make the server crash

bunch of bugs when external_lock() fails on unlock: * mi_lock_database() used mi_mark_crashed() under share->intern_lock, but mi_mark_crashed() itself locks this mutex. * handler::close() required table to be unlocked, but failed external_lock didn't count as unlock * mysql_unlock_tables() ignored all unlock errors, but they still set the error status in stmt_da.

Author: vuvova

mysql-test/r/myisam_debug.result

@@ -27,3 +27,15 @@ CHECK TABLE t1;
 Table	Op	Msg_type	Msg_text
 test.t1	check	status	OK
 DROP TABLE t1,t2;
+call mtr.add_suppression('Incorrect key file for table');
+create table t1 (a int, index(a));
+lock tables t1 write;
+insert t1 values (1),(2),(1);
+set @old_dbug=@@debug_dbug;
+set debug_dbug='+d,mi_lock_database_failure';
+unlock tables;
+Warnings:
+Error	126	Incorrect key file for table './test/t1.MYI'; try to repair it
+Error	1015	Can't lock file (errno: 22 "Invalid argument")
+set debug_dbug=@old_dbug;
+drop table t1;

mysql-test/t/myisam_debug.test

@@ -59,3 +59,16 @@ KILL QUERY @thread_id;
 CHECK TABLE t1; 
 DROP TABLE t1,t2;
 DISCONNECT insertConn;
+
+#
+# MDEV-12761 Error return from external_lock make the server crash
+#
+call mtr.add_suppression('Incorrect key file for table');
+create table t1 (a int, index(a));
+lock tables t1 write;
+insert t1 values (1),(2),(1);
+set @old_dbug=@@debug_dbug;
+set debug_dbug='+d,mi_lock_database_failure';
+unlock tables;
+set debug_dbug=@old_dbug;
+drop table t1;

sql/handler.cc

@@ -5944,7 +5944,7 @@ int handler::ha_external_lock(THD *thd, int lock_type)
  MYSQL_TABLE_LOCK_WAIT(m_psi, PSI_TABLE_EXTERNAL_LOCK, lock_type,
  { error= external_lock(thd, lock_type); })
 
- if (error == 0)
+ if (error == 0 || lock_type == F_UNLCK)
  {
  m_lock_type= lock_type;
  cached_table_flags= table_flags();

sql/lock.cc

@@ -380,12 +380,15 @@ static int lock_external(THD *thd, TABLE **tables, uint count)
 void mysql_unlock_tables(THD *thd, MYSQL_LOCK *sql_lock, bool free_lock)
 {
  DBUG_ENTER("mysql_unlock_tables");
+ bool errors= thd->is_error();
  if (sql_lock->table_count)
  unlock_external(thd, sql_lock->table, sql_lock->table_count);
  if (sql_lock->lock_count)
  thr_multi_unlock(sql_lock->locks, sql_lock->lock_count, 0);
  if (free_lock)
  my_free(sql_lock);
+ if (!errors)
+ thd->clear_error();
  DBUG_VOID_RETURN;
 }
 

storage/myisam/mi_locking.c

@@ -29,7 +29,7 @@ static void mi_update_status_with_lock(MI_INFO *info);
 
 int mi_lock_database(MI_INFO *info, int lock_type)
 {
- int error;
+ int error, mark_crashed= 0;
  uint count;
  MYISAM_SHARE *share=info->s;
  DBUG_ENTER("mi_lock_database");
@@ -52,6 +52,7 @@ int mi_lock_database(MI_INFO *info, int lock_type)
  }
 
  error= 0;
+ DBUG_EXECUTE_IF ("mi_lock_database_failure", error= EINVAL;);
  mysql_mutex_lock(&share->intern_lock);
  if (share->kfile >= 0)/* May only be false on windows */
  {
@@ -75,17 +76,15 @@ int mi_lock_database(MI_INFO *info, int lock_type)
  &share->dirty_part_map,
  FLUSH_KEEP))
  {
-error=my_errno;
+mark_crashed= error=my_errno;
  mi_print_error(info->s, HA_ERR_CRASHED);
-mi_mark_crashed(info);/* Mark that table must be checked */
  }
  if (info->opt_flag & (READ_CACHE_USED | WRITE_CACHE_USED))
  {
 if (end_io_cache(&info->rec_cache))
 {
- error=my_errno;
+ mark_crashed= error=my_errno;
  mi_print_error(info->s, HA_ERR_CRASHED);
- mi_mark_crashed(info);
 }
  }
  if (!count)
@@ -110,22 +109,19 @@ int mi_lock_database(MI_INFO *info, int lock_type)
  share->state.unique= info->last_unique= info->this_unique;
  share->state.update_count= info->last_loop= ++info->this_loop;
  if (mi_state_info_write(share->kfile, &share->state, 1))
- error=my_errno;
+ mark_crashed= error=my_errno;
  share->changed=0;
  if (myisam_flush)
  {
  if (mysql_file_sync(share->kfile, MYF(0)))
- error= my_errno;
+ mark_crashed= error= my_errno;
  if (mysql_file_sync(info->dfile, MYF(0)))
- error= my_errno;
+ mark_crashed= error= my_errno;
  }
  else
  share->not_flushed=1;
  if (error)
- {
  mi_print_error(info->s, HA_ERR_CRASHED);
- mi_mark_crashed(info);
- }
 }
 if (info->lock_type != F_EXTRA_LCK)
 {
@@ -260,6 +256,8 @@ int mi_lock_database(MI_INFO *info, int lock_type)
  }
 #endif
  mysql_mutex_unlock(&share->intern_lock);
+ if (mark_crashed)
+ mi_mark_crashed(info);
  DBUG_RETURN(error);
 } /* mi_lock_database */