Commit f9264280 authored by Sergei Golubchik's avatar Sergei Golubchik

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.
parent 52aa2009
...@@ -27,3 +27,15 @@ CHECK TABLE t1; ...@@ -27,3 +27,15 @@ CHECK TABLE t1;
Table Op Msg_type Msg_text Table Op Msg_type Msg_text
test.t1 check status OK test.t1 check status OK
DROP TABLE t1,t2; 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;
...@@ -59,3 +59,16 @@ KILL QUERY @thread_id; ...@@ -59,3 +59,16 @@ KILL QUERY @thread_id;
CHECK TABLE t1; CHECK TABLE t1;
DROP TABLE t1,t2; DROP TABLE t1,t2;
DISCONNECT insertConn; 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;
...@@ -5944,7 +5944,7 @@ int handler::ha_external_lock(THD *thd, int lock_type) ...@@ -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, MYSQL_TABLE_LOCK_WAIT(m_psi, PSI_TABLE_EXTERNAL_LOCK, lock_type,
{ error= external_lock(thd, lock_type); }) { error= external_lock(thd, lock_type); })
if (error == 0) if (error == 0 || lock_type == F_UNLCK)
{ {
m_lock_type= lock_type; m_lock_type= lock_type;
cached_table_flags= table_flags(); cached_table_flags= table_flags();
......
...@@ -380,12 +380,15 @@ static int lock_external(THD *thd, TABLE **tables, uint count) ...@@ -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) void mysql_unlock_tables(THD *thd, MYSQL_LOCK *sql_lock, bool free_lock)
{ {
DBUG_ENTER("mysql_unlock_tables"); DBUG_ENTER("mysql_unlock_tables");
bool errors= thd->is_error();
if (sql_lock->table_count) if (sql_lock->table_count)
unlock_external(thd, sql_lock->table, sql_lock->table_count); unlock_external(thd, sql_lock->table, sql_lock->table_count);
if (sql_lock->lock_count) if (sql_lock->lock_count)
thr_multi_unlock(sql_lock->locks, sql_lock->lock_count, 0); thr_multi_unlock(sql_lock->locks, sql_lock->lock_count, 0);
if (free_lock) if (free_lock)
my_free(sql_lock); my_free(sql_lock);
if (!errors)
thd->clear_error();
DBUG_VOID_RETURN; DBUG_VOID_RETURN;
} }
......
...@@ -29,7 +29,7 @@ static void mi_update_status_with_lock(MI_INFO *info); ...@@ -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 mi_lock_database(MI_INFO *info, int lock_type)
{ {
int error; int error, mark_crashed= 0;
uint count; uint count;
MYISAM_SHARE *share=info->s; MYISAM_SHARE *share=info->s;
DBUG_ENTER("mi_lock_database"); DBUG_ENTER("mi_lock_database");
...@@ -52,6 +52,7 @@ int mi_lock_database(MI_INFO *info, int lock_type) ...@@ -52,6 +52,7 @@ int mi_lock_database(MI_INFO *info, int lock_type)
} }
error= 0; error= 0;
DBUG_EXECUTE_IF ("mi_lock_database_failure", error= EINVAL;);
mysql_mutex_lock(&share->intern_lock); mysql_mutex_lock(&share->intern_lock);
if (share->kfile >= 0) /* May only be false on windows */ if (share->kfile >= 0) /* May only be false on windows */
{ {
...@@ -75,17 +76,15 @@ int mi_lock_database(MI_INFO *info, int lock_type) ...@@ -75,17 +76,15 @@ int mi_lock_database(MI_INFO *info, int lock_type)
&share->dirty_part_map, &share->dirty_part_map,
FLUSH_KEEP)) FLUSH_KEEP))
{ {
error=my_errno; mark_crashed= error=my_errno;
mi_print_error(info->s, HA_ERR_CRASHED); 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 (info->opt_flag & (READ_CACHE_USED | WRITE_CACHE_USED))
{ {
if (end_io_cache(&info->rec_cache)) if (end_io_cache(&info->rec_cache))
{ {
error=my_errno; mark_crashed= error=my_errno;
mi_print_error(info->s, HA_ERR_CRASHED); mi_print_error(info->s, HA_ERR_CRASHED);
mi_mark_crashed(info);
} }
} }
if (!count) if (!count)
...@@ -110,22 +109,19 @@ int mi_lock_database(MI_INFO *info, int lock_type) ...@@ -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.unique= info->last_unique= info->this_unique;
share->state.update_count= info->last_loop= ++info->this_loop; share->state.update_count= info->last_loop= ++info->this_loop;
if (mi_state_info_write(share->kfile, &share->state, 1)) if (mi_state_info_write(share->kfile, &share->state, 1))
error=my_errno; mark_crashed= error=my_errno;
share->changed=0; share->changed=0;
if (myisam_flush) if (myisam_flush)
{ {
if (mysql_file_sync(share->kfile, MYF(0))) if (mysql_file_sync(share->kfile, MYF(0)))
error= my_errno; mark_crashed= error= my_errno;
if (mysql_file_sync(info->dfile, MYF(0))) if (mysql_file_sync(info->dfile, MYF(0)))
error= my_errno; mark_crashed= error= my_errno;
} }
else else
share->not_flushed=1; share->not_flushed=1;
if (error) if (error)
{
mi_print_error(info->s, HA_ERR_CRASHED); mi_print_error(info->s, HA_ERR_CRASHED);
mi_mark_crashed(info);
}
} }
if (info->lock_type != F_EXTRA_LCK) if (info->lock_type != F_EXTRA_LCK)
{ {
...@@ -260,6 +256,8 @@ int mi_lock_database(MI_INFO *info, int lock_type) ...@@ -260,6 +256,8 @@ int mi_lock_database(MI_INFO *info, int lock_type)
} }
#endif #endif
mysql_mutex_unlock(&share->intern_lock); mysql_mutex_unlock(&share->intern_lock);
if (mark_crashed)
mi_mark_crashed(info);
DBUG_RETURN(error); DBUG_RETURN(error);
} /* mi_lock_database */ } /* mi_lock_database */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment