diff options
| -rw-r--r-- | mysql-test/r/lock_multi.result | 8 | ||||
| -rw-r--r-- | mysql-test/t/lock_multi.test | 33 | ||||
| -rw-r--r-- | sql/sql_db.cc | 60 | ||||
| -rw-r--r-- | storage/myisam/mi_check.c | 34 | ||||
| -rw-r--r-- | storage/myisam/mi_key.c | 14 | ||||
| -rw-r--r-- | storage/myisam/mi_update.c | 3 | ||||
| -rw-r--r-- | storage/myisam/mi_write.c | 3 | ||||
| -rw-r--r-- | storage/myisam/myisamdef.h | 2 |
8 files changed, 114 insertions, 43 deletions
diff --git a/mysql-test/r/lock_multi.result b/mysql-test/r/lock_multi.result index 180c8f41b70..6d31d01d154 100644 --- a/mysql-test/r/lock_multi.result +++ b/mysql-test/r/lock_multi.result @@ -50,6 +50,14 @@ Field Type Null Key Default Extra a int(11) YES NULL unlock tables; drop table t1; +CREATE DATABASE mysqltest_1; +FLUSH TABLES WITH READ LOCK; + DROP DATABASE mysqltest_1; +DROP DATABASE mysqltest_1; +ERROR HY000: Can't execute the query because you have a conflicting read lock +UNLOCK TABLES; +DROP DATABASE mysqltest_1; +ERROR HY000: Can't drop database 'mysqltest_1'; database doesn't exist use mysql; LOCK TABLES columns_priv WRITE, db WRITE, host WRITE, user WRITE; FLUSH TABLES; diff --git a/mysql-test/t/lock_multi.test b/mysql-test/t/lock_multi.test index a24ee8be64e..7fc65f52214 100644 --- a/mysql-test/t/lock_multi.test +++ b/mysql-test/t/lock_multi.test @@ -128,6 +128,39 @@ unlock tables; drop table t1; # +# Bug#19815 - CREATE/RENAME/DROP DATABASE can deadlock on a global read lock +# +connect (con1,localhost,root,,); +connect (con2,localhost,root,,); +# +connection con1; +CREATE DATABASE mysqltest_1; +FLUSH TABLES WITH READ LOCK; +# +# With bug in place: acquire LOCK_mysql_create_table and +# wait in wait_if_global_read_lock(). +connection con2; +send DROP DATABASE mysqltest_1; +--sleep 1 +# +# With bug in place: try to acquire LOCK_mysql_create_table... +# When fixed: Reject dropping db because of the read lock. +connection con1; +--error ER_CANT_UPDATE_WITH_READLOCK +DROP DATABASE mysqltest_1; +UNLOCK TABLES; +# +connection con2; +reap; +# +connection default; +disconnect con1; +disconnect con2; +# This must have been dropped by connection 2 already, +# which waited until the global read lock was released. +--error ER_DB_DROP_EXISTS +DROP DATABASE mysqltest_1; + # Bug#16986 - Deadlock condition with MyISAM tables # connection locker; diff --git a/sql/sql_db.cc b/sql/sql_db.cc index 6d5362c2554..11c892fee44 100644 --- a/sql/sql_db.cc +++ b/sql/sql_db.cc @@ -538,16 +538,27 @@ bool mysql_create_db(THD *thd, char *db, HA_CREATE_INFO *create_info, my_error(ER_DB_CREATE_EXISTS, MYF(0), db); DBUG_RETURN(-1); } - - VOID(pthread_mutex_lock(&LOCK_mysql_create_db)); - /* do not create database if another thread is holding read lock */ + /* + Do not create database if another thread is holding read lock. + Wait for global read lock before acquiring LOCK_mysql_create_db. + After wait_if_global_read_lock() we have protection against another + global read lock. If we would acquire LOCK_mysql_create_db first, + another thread could step in and get the global read lock before we + reach wait_if_global_read_lock(). If this thread tries the same as we + (admin a db), it would then go and wait on LOCK_mysql_create_db... + Furthermore wait_if_global_read_lock() checks if the current thread + has the global read lock and refuses the operation with + ER_CANT_UPDATE_WITH_READLOCK if applicable. + */ if (wait_if_global_read_lock(thd, 0, 1)) { error= -1; goto exit2; } + VOID(pthread_mutex_lock(&LOCK_mysql_create_db)); + /* Check directory */ path_len= build_table_filename(path, sizeof(path), db, "", ""); path[path_len-1]= 0; // Remove last '/' from path @@ -655,9 +666,9 @@ bool mysql_create_db(THD *thd, char *db, HA_CREATE_INFO *create_info, } exit: + VOID(pthread_mutex_unlock(&LOCK_mysql_create_db)); start_waiting_global_read_lock(thd); exit2: - VOID(pthread_mutex_unlock(&LOCK_mysql_create_db)); DBUG_RETURN(error); } @@ -671,12 +682,23 @@ bool mysql_alter_db(THD *thd, const char *db, HA_CREATE_INFO *create_info) int error= 0; DBUG_ENTER("mysql_alter_db"); - VOID(pthread_mutex_lock(&LOCK_mysql_create_db)); - - /* do not alter database if another thread is holding read lock */ + /* + Do not alter database if another thread is holding read lock. + Wait for global read lock before acquiring LOCK_mysql_create_db. + After wait_if_global_read_lock() we have protection against another + global read lock. If we would acquire LOCK_mysql_create_db first, + another thread could step in and get the global read lock before we + reach wait_if_global_read_lock(). If this thread tries the same as we + (admin a db), it would then go and wait on LOCK_mysql_create_db... + Furthermore wait_if_global_read_lock() checks if the current thread + has the global read lock and refuses the operation with + ER_CANT_UPDATE_WITH_READLOCK if applicable. + */ if ((error=wait_if_global_read_lock(thd,0,1))) goto exit2; + VOID(pthread_mutex_lock(&LOCK_mysql_create_db)); + /* Recreate db options file: /dbpath/.db.opt We pass MY_DB_OPT_FILE as "extension" to avoid @@ -721,9 +743,9 @@ bool mysql_alter_db(THD *thd, const char *db, HA_CREATE_INFO *create_info) send_ok(thd, result); exit: + VOID(pthread_mutex_unlock(&LOCK_mysql_create_db)); start_waiting_global_read_lock(thd); exit2: - VOID(pthread_mutex_unlock(&LOCK_mysql_create_db)); DBUG_RETURN(error); } @@ -755,15 +777,26 @@ bool mysql_rm_db(THD *thd,char *db,bool if_exists, bool silent) TABLE_LIST* dropped_tables= 0; DBUG_ENTER("mysql_rm_db"); - VOID(pthread_mutex_lock(&LOCK_mysql_create_db)); - - /* do not drop database if another thread is holding read lock */ + /* + Do not drop database if another thread is holding read lock. + Wait for global read lock before acquiring LOCK_mysql_create_db. + After wait_if_global_read_lock() we have protection against another + global read lock. If we would acquire LOCK_mysql_create_db first, + another thread could step in and get the global read lock before we + reach wait_if_global_read_lock(). If this thread tries the same as we + (admin a db), it would then go and wait on LOCK_mysql_create_db... + Furthermore wait_if_global_read_lock() checks if the current thread + has the global read lock and refuses the operation with + ER_CANT_UPDATE_WITH_READLOCK if applicable. + */ if (wait_if_global_read_lock(thd, 0, 1)) { error= -1; goto exit2; } + VOID(pthread_mutex_lock(&LOCK_mysql_create_db)); + length= build_table_filename(path, sizeof(path), db, "", ""); strmov(path+length, MY_DB_OPT_FILE); // Append db option file name del_dbopt(path); // Remove dboption hash entry @@ -872,7 +905,6 @@ bool mysql_rm_db(THD *thd,char *db,bool if_exists, bool silent) exit: (void)sp_drop_db_routines(thd, db); /* QQ Ignore errors for now */ error= Events::drop_schema_events(thd, db); - start_waiting_global_read_lock(thd); /* If this database was the client's selected database, we silently change the client's selected database to nothing (to have an empty SELECT DATABASE() @@ -901,9 +933,9 @@ exit: thd->db= 0; thd->db_length= 0; } -exit2: VOID(pthread_mutex_unlock(&LOCK_mysql_create_db)); - + start_waiting_global_read_lock(thd); +exit2: DBUG_RETURN(error); } diff --git a/storage/myisam/mi_check.c b/storage/myisam/mi_check.c index ef7b1e6fde3..d91597e9138 100644 --- a/storage/myisam/mi_check.c +++ b/storage/myisam/mi_check.c @@ -453,25 +453,24 @@ int chk_key(MI_CHECK *param, register MI_INFO *info) if ((uint) share->base.auto_key -1 == key) { /* Check that auto_increment key is bigger than max key value */ - ulonglong save_auto_value=info->s->state.auto_increment; - info->s->state.auto_increment=0; + ulonglong auto_increment; info->lastinx=key; _mi_read_key_record(info, 0L, info->rec_buff); - update_auto_increment(info, info->rec_buff); - if (info->s->state.auto_increment > save_auto_value) + auto_increment= retrieve_auto_increment(info, info->rec_buff); + if (auto_increment > info->s->state.auto_increment) { - mi_check_print_warning(param, - "Auto-increment value: %s is smaller than max used value: %s", - llstr(save_auto_value,buff2), - llstr(info->s->state.auto_increment, buff)); + mi_check_print_warning(param, "Auto-increment value: %s is smaller " + "than max used value: %s", + llstr(info->s->state.auto_increment,buff2), + llstr(auto_increment, buff)); } if (param->testflag & T_AUTO_INC) { - set_if_bigger(info->s->state.auto_increment, - param->auto_increment_value); + set_if_bigger(info->s->state.auto_increment, + auto_increment); + set_if_bigger(info->s->state.auto_increment, + param->auto_increment_value); } - else - info->s->state.auto_increment=save_auto_value; /* Check that there isn't a row with auto_increment = 0 in the table */ mi_extra(info,HA_EXTRA_KEYREAD,0); @@ -481,8 +480,8 @@ int chk_key(MI_CHECK *param, register MI_INFO *info) { /* Don't count this as a real warning, as myisamchk can't correct it */ uint save=param->warning_printed; - mi_check_print_warning(param, - "Found row where the auto_increment column has the value 0"); + mi_check_print_warning(param, "Found row where the auto_increment " + "column has the value 0"); param->warning_printed=save; } mi_extra(info,HA_EXTRA_NO_KEYREAD,0); @@ -4124,11 +4123,10 @@ void update_auto_increment_key(MI_CHECK *param, MI_INFO *info, } else { - ulonglong auto_increment= (repair_only ? info->s->state.auto_increment : - param->auto_increment_value); - info->s->state.auto_increment=0; - update_auto_increment(info, record); + ulonglong auto_increment= retrieve_auto_increment(info, record); set_if_bigger(info->s->state.auto_increment,auto_increment); + if (!repair_only) + set_if_bigger(info->s->state.auto_increment, param->auto_increment_value); } mi_extra(info,HA_EXTRA_NO_KEYREAD,0); my_free((char*) record, MYF(0)); diff --git a/storage/myisam/mi_key.c b/storage/myisam/mi_key.c index 526a733e724..01bd0c43119 100644 --- a/storage/myisam/mi_key.c +++ b/storage/myisam/mi_key.c @@ -507,22 +507,21 @@ int _mi_read_key_record(MI_INFO *info, my_off_t filepos, byte *buf) return(-1); /* Wrong data to read */ } - + /* - Update auto_increment info + Retrieve auto_increment info SYNOPSIS - update_auto_increment() + retrieve_auto_increment() info MyISAM handler record Row to update IMPLEMENTATION - Only replace the auto_increment value if it is higher than the previous - one. For signed columns we don't update the auto increment value if it's + For signed columns we don't retrieve the auto increment value if it's less than zero. */ -void update_auto_increment(MI_INFO *info,const byte *record) +ulonglong retrieve_auto_increment(MI_INFO *info,const byte *record) { ulonglong value= 0; /* Store unsigned values here */ longlong s_value= 0; /* Store signed values here */ @@ -587,6 +586,5 @@ void update_auto_increment(MI_INFO *info,const byte *record) and if s_value == 0 then value will contain either s_value or the correct value. */ - set_if_bigger(info->s->state.auto_increment, - (s_value > 0) ? (ulonglong) s_value : value); + return (s_value > 0) ? (ulonglong) s_value : value; } diff --git a/storage/myisam/mi_update.c b/storage/myisam/mi_update.c index 937c9983b45..f8b5cf55406 100644 --- a/storage/myisam/mi_update.c +++ b/storage/myisam/mi_update.c @@ -164,7 +164,8 @@ int mi_update(register MI_INFO *info, const byte *oldrec, byte *newrec) key_changed|= HA_STATE_CHANGED; /* Must update index file */ } if (auto_key_changed) - update_auto_increment(info,newrec); + set_if_bigger(info->s->state.auto_increment, + retrieve_auto_increment(info, newrec)); if (share->calc_checksum) info->state->checksum+=(info->checksum - old_checksum); diff --git a/storage/myisam/mi_write.c b/storage/myisam/mi_write.c index 5e79b2937cc..9ab8753f6d7 100644 --- a/storage/myisam/mi_write.c +++ b/storage/myisam/mi_write.c @@ -149,7 +149,8 @@ int mi_write(MI_INFO *info, byte *record) info->state->checksum+=info->checksum; } if (share->base.auto_key) - update_auto_increment(info,record); + set_if_bigger(info->s->state.auto_increment, + retrieve_auto_increment(info, record)); info->update= (HA_STATE_CHANGED | HA_STATE_AKTIV | HA_STATE_WRITTEN | HA_STATE_ROW_CHANGED); info->state->records++; diff --git a/storage/myisam/myisamdef.h b/storage/myisam/myisamdef.h index a83fbe8f759..baab34b4b67 100644 --- a/storage/myisam/myisamdef.h +++ b/storage/myisam/myisamdef.h @@ -593,7 +593,7 @@ extern uint _mi_pack_key(MI_INFO *info,uint keynr,uchar *key,uchar *old, extern int _mi_read_key_record(MI_INFO *info,my_off_t filepos,byte *buf); extern int _mi_read_cache(IO_CACHE *info,byte *buff,my_off_t pos, uint length,int re_read_if_possibly); -extern void update_auto_increment(MI_INFO *info,const byte *record); +extern ulonglong retrieve_auto_increment(MI_INFO *info,const byte *record); extern byte *mi_alloc_rec_buff(MI_INFO *,ulong, byte**); #define mi_get_rec_buff_ptr(info,buf) \ |