From b51c8cab3e0afccd6b44fea70e59243b24844158 Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Fri, 20 Aug 2010 13:58:28 +0400 Subject: BUG#54989 - With null_audit installed, server hangs on an attempt to install a plugin twice Server crashes when [UN]INSTALL PLUGIN fails (returns an error) and general log is disabled and there are audit plugins interested in MYSQL_AUDIT_GENERAL_CLASS. When audit event is triggered, audit subsystem acquires interested plugins by walking through plugin list. Evidently plugin list iterator protects plugin list by acquiring LOCK_plugin, see plugin_foreach_with_mask(). On the other hand [UN]INSTALL PLUGIN is acquiring LOCK_plugin rather for a long time. When audit event is triggered during [UN]INSTALL PLUGIN, plugin list iterator acquires the same lock (within the same thread) second time. Repeatable only with general_log disabled, because general_log triggers MYSQL_AUDIT_GENERAL_LOG event, which acquires audit plugins before [UN]INSTALL PLUGIN acquired LOCK_plugin. With this fix we pre-acquire audit plugins for events that may potentially occur during [UN]INSTALL PLUGIN. This hack should be removed when LOCK_plugin is fixed so it protects only what it supposed to protect. No test case for this fix - we do not have facility to test audit plugins yet. sql/sql_audit.cc: Move "acquire audit plugin" logics to a separate function. sql/sql_audit.h: Move "acquire audit plugin" logics to a separate function. sql/sql_plugin.cc: Pre-acquire audit plugins for events that may potentially occur during [UN]INSTALL PLUGIN. --- sql/sql_audit.cc | 44 ++++++++++++++++++++++++++++++-------------- sql/sql_audit.h | 1 + sql/sql_plugin.cc | 44 +++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 74 insertions(+), 15 deletions(-) (limited to 'sql') diff --git a/sql/sql_audit.cc b/sql/sql_audit.cc index 7d269d455a9..b7d363dc09a 100644 --- a/sql/sql_audit.cc +++ b/sql/sql_audit.cc @@ -137,6 +137,30 @@ static my_bool acquire_plugins(THD *thd, plugin_ref plugin, void *arg) } +/** + @brief Acquire audit plugins + + @param[in] thd MySQL thread handle + @param[in] event_class Audit event class + + @details Ensure that audit plugins interested in given event + class are locked by current thread. +*/ +void mysql_audit_acquire_plugins(THD *thd, uint event_class) +{ + unsigned long event_class_mask[MYSQL_AUDIT_CLASS_MASK_SIZE]; + DBUG_ENTER("mysql_audit_acquire_plugins"); + set_audit_mask(event_class_mask, event_class); + if (thd && !check_audit_mask(mysql_global_audit_mask, event_class_mask) && + check_audit_mask(thd->audit_class_mask, event_class_mask)) + { + plugin_foreach(thd, acquire_plugins, MYSQL_AUDIT_PLUGIN, &event_class); + add_audit_mask(thd->audit_class_mask, event_class_mask); + } + DBUG_VOID_RETURN; +} + + /** Notify the audit system of an event @@ -151,21 +175,8 @@ void mysql_audit_notify(THD *thd, uint event_class, uint event_subtype, ...) { va_list ap; audit_handler_t *handlers= audit_handlers + event_class; - unsigned long event_class_mask[MYSQL_AUDIT_CLASS_MASK_SIZE]; - DBUG_ASSERT(event_class < audit_handlers_count); - - set_audit_mask(event_class_mask, event_class); - /* - Check to see if we have acquired the audit plugins for the - required audit event classes. - */ - if (thd && check_audit_mask(thd->audit_class_mask, event_class_mask)) - { - plugin_foreach(thd, acquire_plugins, MYSQL_AUDIT_PLUGIN, &event_class); - add_audit_mask(thd->audit_class_mask, event_class_mask); - } - + mysql_audit_acquire_plugins(thd, event_class); va_start(ap, event_subtype); (*handlers)(thd, event_subtype, ap); va_end(ap); @@ -448,6 +459,11 @@ static void event_class_dispatch(THD *thd, const struct mysql_event *event) #else /* EMBEDDED_LIBRARY */ +void mysql_audit_acquire_plugins(THD *thd, uint event_class) +{ +} + + void mysql_audit_initialize() { } diff --git a/sql/sql_audit.h b/sql/sql_audit.h index 5b6962b9ecb..953e41f1f06 100644 --- a/sql/sql_audit.h +++ b/sql/sql_audit.h @@ -29,6 +29,7 @@ extern void mysql_audit_finalize(); extern void mysql_audit_init_thd(THD *thd); extern void mysql_audit_free_thd(THD *thd); +extern void mysql_audit_acquire_plugins(THD *thd, uint event_class); extern void mysql_audit_notify(THD *thd, uint event_class, diff --git a/sql/sql_plugin.cc b/sql/sql_plugin.cc index e3323260373..2d317eb56ef 100644 --- a/sql/sql_plugin.cc +++ b/sql/sql_plugin.cc @@ -29,7 +29,7 @@ #include "records.h" // init_read_record, end_read_record #include #include -#include +#include "sql_audit.h" #include "lock.h" // MYSQL_LOCK_IGNORE_TIMEOUT #define REPORT_TO_LOG 1 #define REPORT_TO_USER 2 @@ -1703,6 +1703,27 @@ bool mysql_install_plugin(THD *thd, const LEX_STRING *name, const LEX_STRING *dl MYSQL_LOCK_IGNORE_TIMEOUT))) DBUG_RETURN(TRUE); + /* + Pre-acquire audit plugins for events that may potentially occur + during [UN]INSTALL PLUGIN. + + When audit event is triggered, audit subsystem acquires interested + plugins by walking through plugin list. Evidently plugin list + iterator protects plugin list by acquiring LOCK_plugin, see + plugin_foreach_with_mask(). + + On the other hand [UN]INSTALL PLUGIN is acquiring LOCK_plugin + rather for a long time. + + When audit event is triggered during [UN]INSTALL PLUGIN, plugin + list iterator acquires the same lock (within the same thread) + second time. + + This hack should be removed when LOCK_plugin is fixed so it + protects only what it supposed to protect. + */ + mysql_audit_acquire_plugins(thd, MYSQL_AUDIT_GENERAL_CLASS); + mysql_mutex_lock(&LOCK_plugin); mysql_rwlock_wrlock(&LOCK_system_variables_hash); @@ -1783,6 +1804,27 @@ bool mysql_uninstall_plugin(THD *thd, const LEX_STRING *name) if (! (table= open_ltable(thd, &tables, TL_WRITE, MYSQL_LOCK_IGNORE_TIMEOUT))) DBUG_RETURN(TRUE); + /* + Pre-acquire audit plugins for events that may potentially occur + during [UN]INSTALL PLUGIN. + + When audit event is triggered, audit subsystem acquires interested + plugins by walking through plugin list. Evidently plugin list + iterator protects plugin list by acquiring LOCK_plugin, see + plugin_foreach_with_mask(). + + On the other hand [UN]INSTALL PLUGIN is acquiring LOCK_plugin + rather for a long time. + + When audit event is triggered during [UN]INSTALL PLUGIN, plugin + list iterator acquires the same lock (within the same thread) + second time. + + This hack should be removed when LOCK_plugin is fixed so it + protects only what it supposed to protect. + */ + mysql_audit_acquire_plugins(thd, MYSQL_AUDIT_GENERAL_CLASS); + mysql_mutex_lock(&LOCK_plugin); if (!(plugin= plugin_find_internal(name, MYSQL_ANY_PLUGIN))) { -- cgit v1.2.1 From 8df0bf13ab5930333ae1148856533e7bff156296 Mon Sep 17 00:00:00 2001 From: Mattias Jonsson Date: Fri, 20 Aug 2010 19:15:48 +0200 Subject: Bug#54747: Deadlock between REORGANIZE PARTITION and SELECT is not detected The ALTER PARTITION and SELECT seemed to be deadlocked when having innodb_thread_concurrency = 1. Problem was that there was unreleased latches in the ALTER PARTITION thread which was needed by the SELECT thread to be able to continue. Solution was to release the latches by commit before requesting upgrade to exclusive MDL lock. Updated according to reviewers comments (3). mysql-test/r/partition_innodb.result: updated test result mysql-test/t/partition_innodb.test: added test sql/sql_partition.cc: Moved implicit commit into mysql_change_partition so that if latches are taken, they are always released before waiting on exclusive lock. sql/sql_table.cc: refactored the code to prepare and commit around copy_data_between_tables, to be able to reuse it in mysql_change_partitions sql/sql_table.h: exporting mysql_trans_prepare/commit_alter_copy_data --- sql/sql_partition.cc | 21 ++++++++-------- sql/sql_table.cc | 71 ++++++++++++++++++++++++++++++++++++---------------- sql/sql_table.h | 2 ++ 3 files changed, 63 insertions(+), 31 deletions(-) (limited to 'sql') diff --git a/sql/sql_partition.cc b/sql/sql_partition.cc index 5bbe946179e..b72816f8ce3 100644 --- a/sql/sql_partition.cc +++ b/sql/sql_partition.cc @@ -63,6 +63,7 @@ #include "sql_table.h" // build_table_filename, // build_table_shadow_filename, // table_to_filename + // mysql_*_alter_copy_data #include "opt_range.h" // store_key_image_to_rec #include "sql_analyse.h" // append_escaped @@ -4377,7 +4378,6 @@ static int fast_end_partition(THD *thd, ulonglong copied, ALTER_PARTITION_PARAM_TYPE *lpt, bool written_bin_log) { - int error; char tmp_name[80]; DBUG_ENTER("fast_end_partition"); @@ -4386,13 +4386,6 @@ static int fast_end_partition(THD *thd, ulonglong copied, if (!is_empty) query_cache_invalidate3(thd, table_list, 0); - error= trans_commit_stmt(thd); - if (trans_commit_implicit(thd)) - error= 1; - - if (error) - DBUG_RETURN(TRUE); /* The error has been reported */ - if ((!is_empty) && (!written_bin_log) && (!thd->lex->no_write_to_binlog) && write_bin_log(thd, FALSE, thd->query(), thd->query_length())) @@ -5535,17 +5528,25 @@ static bool mysql_change_partitions(ALTER_PARTITION_PARAM_TYPE *lpt) char path[FN_REFLEN+1]; int error; handler *file= lpt->table->file; + THD *thd= lpt->thd; DBUG_ENTER("mysql_change_partitions"); build_table_filename(path, sizeof(path) - 1, lpt->db, lpt->table_name, "", 0); + + if(mysql_trans_prepare_alter_copy_data(thd)) + DBUG_RETURN(TRUE); + if ((error= file->ha_change_partitions(lpt->create_info, path, &lpt->copied, &lpt->deleted, lpt->pack_frm_data, lpt->pack_frm_len))) { file->print_error(error, MYF(error != ER_OUTOFMEMORY ? 0 : ME_FATALERROR)); - DBUG_RETURN(TRUE); } - DBUG_RETURN(FALSE); + + if (mysql_trans_commit_alter_copy_data(thd)) + DBUG_RETURN(TRUE); /* The error has been reported */ + + DBUG_RETURN(test(error)); } diff --git a/sql/sql_table.cc b/sql/sql_table.cc index bd5f381b8b8..3bb7f7667ea 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -6740,6 +6740,54 @@ err_with_mdl: } /* mysql_alter_table */ + + +/** + Prepare the transaction for the alter table's copy phase. +*/ + +bool mysql_trans_prepare_alter_copy_data(THD *thd) +{ + DBUG_ENTER("mysql_prepare_alter_copy_data"); + /* + Turn off recovery logging since rollback of an alter table is to + delete the new table so there is no need to log the changes to it. + + This needs to be done before external_lock. + */ + if (ha_enable_transaction(thd, FALSE)) + DBUG_RETURN(TRUE); + DBUG_RETURN(FALSE); +} + + +/** + Commit the copy phase of the alter table. +*/ + +bool mysql_trans_commit_alter_copy_data(THD *thd) +{ + bool error= FALSE; + DBUG_ENTER("mysql_commit_alter_copy_data"); + + if (ha_enable_transaction(thd, TRUE)) + DBUG_RETURN(TRUE); + + /* + Ensure that the new table is saved properly to disk before installing + the new .frm. + And that InnoDB's internal latches are released, to avoid deadlock + when waiting on other instances of the table before rename (Bug#54747). + */ + if (trans_commit_stmt(thd)) + error= TRUE; + if (trans_commit_implicit(thd)) + error= TRUE; + + DBUG_RETURN(error); +} + + static int copy_data_between_tables(TABLE *from,TABLE *to, List &create, @@ -6766,14 +6814,7 @@ copy_data_between_tables(TABLE *from,TABLE *to, ulonglong prev_insert_id; DBUG_ENTER("copy_data_between_tables"); - /* - Turn off recovery logging since rollback of an alter table is to - delete the new table so there is no need to log the changes to it. - - This needs to be done before external_lock - */ - error= ha_enable_transaction(thd, FALSE); - if (error) + if (mysql_trans_prepare_alter_copy_data(thd)) DBUG_RETURN(-1); if (!(copy= new Copy_field[to->s->fields])) @@ -6932,20 +6973,8 @@ copy_data_between_tables(TABLE *from,TABLE *to, } to->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY); - if (ha_enable_transaction(thd, TRUE)) - { + if (mysql_trans_commit_alter_copy_data(thd)) error= 1; - goto err; - } - - /* - Ensure that the new table is saved properly to disk so that we - can do a rename - */ - if (trans_commit_stmt(thd)) - error=1; - if (trans_commit_implicit(thd)) - error=1; err: thd->variables.sql_mode= save_sql_mode; diff --git a/sql/sql_table.h b/sql/sql_table.h index ae5beefea37..eb0b1aa94dd 100644 --- a/sql/sql_table.h +++ b/sql/sql_table.h @@ -143,6 +143,8 @@ bool mysql_create_table_no_lock(THD *thd, const char *db, bool mysql_prepare_alter_table(THD *thd, TABLE *table, HA_CREATE_INFO *create_info, Alter_info *alter_info); +bool mysql_trans_prepare_alter_copy_data(THD *thd); +bool mysql_trans_commit_alter_copy_data(THD *thd); bool mysql_alter_table(THD *thd, char *new_db, char *new_name, HA_CREATE_INFO *create_info, TABLE_LIST *table_list, -- cgit v1.2.1 From 1ed02deea0986ee2aefd7b8bc61390f3675c2e94 Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Mon, 23 Aug 2010 13:56:21 +0400 Subject: Bug#52121 partition by key on utf32 enum field cause debug assertion: (length % 4) == 0 Problem: ENUM columns are sorted and distributed according to their numeric value, but Field::hash() incorrectly passed string character set (utf32) in combination with numeric value to the hash function, which made assertion fail. Fix: pass "binary" character set in combination with numeric value to the hash function. mysql-test/suite/parts/r/part_ctype_utf32.result Adding tests mysql-test/suite/parts/t/part_ctype_utf32.test Adding test sql/field.cc Pass correct character set pointer to the hash function. --- sql/field.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'sql') diff --git a/sql/field.cc b/sql/field.cc index 3c93ffadac5..fc55426b177 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -1329,7 +1329,7 @@ void Field::hash(ulong *nr, ulong *nr2) else { uint len= pack_length(); - CHARSET_INFO *cs= charset(); + CHARSET_INFO *cs= sort_charset(); cs->coll->hash_sort(cs, ptr, len, nr, nr2); } } -- cgit v1.2.1 From 3592489cae0115c245ef3e692bf0ec276a3e010d Mon Sep 17 00:00:00 2001 From: Jon Olav Hauglid Date: Mon, 23 Aug 2010 17:42:53 +0200 Subject: Bug #54332 Deadlock with two connections doing LOCK TABLE+INSERT DELAYED The problem was that deadlocks involving INSERT DELAYED were not detected. The reason for this is that two threads are involved in INSERT DELAYED: the connection thread and the handler thread. The connection thread would wait while the handler thread acquired locks and opened the table. In essence, this adds an edge to the wait-for-graph between the connection thread and the handler thread that the deadlock detector is unaware of. Therefore many deadlocks involving INSERT DELAYED were not detected. This patch fixes the problem by having the connection thread acquire the metadata lock the table before starting the handler thread. This allows the deadlock detector to detect any possible deadlocks resulting from trying to acquire a metadata lock the table. If a metadata lock is successfully acquired, the handler thread is started and given a copy of the ticket representing the metadata lock. When the handler thread then tries to lock and open the table, it will find that it already has the metadata lock and therefore not acquire any new metadata locks. Test cases added to delayed.test. --- sql/sql_insert.cc | 73 +++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 63 insertions(+), 10 deletions(-) (limited to 'sql') diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index 6c23b47997a..d9340cf843a 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -548,10 +548,34 @@ bool open_and_lock_for_insert_delayed(THD *thd, TABLE_LIST *table_list) DBUG_RETURN(TRUE); } - if (delayed_get_table(thd, table_list)) + /* + In order for the deadlock detector to be able to find any deadlocks + caused by the handler thread locking this table, we take the metadata + lock inside the connection thread. If this goes ok, the ticket is cloned + and added to the list of granted locks held by the handler thread. + */ + MDL_ticket *mdl_savepoint= thd->mdl_context.mdl_savepoint(); + if (thd->mdl_context.acquire_lock(&table_list->mdl_request, + thd->variables.lock_wait_timeout)) + /* + If a lock can't be acquired, it makes no sense to try normal insert. + Therefore we just abort the statement. + */ DBUG_RETURN(TRUE); - if (table_list->table) + /* + If a lock was acquired above, we should release it after delayed_get_table() + has cloned the ticket for the handler thread. Note that acquire_lock() can + succeed because of a lock already held by the connection. In this case we + should not release it here. + */ + MDL_ticket *table_ticket = mdl_savepoint == thd->mdl_context.mdl_savepoint() ? + NULL: thd->mdl_context.mdl_savepoint(); + + bool error= FALSE; + if (delayed_get_table(thd, table_list)) + error= TRUE; + else if (table_list->table) { /* Open tables used for sub-selects or in stored functions, will also @@ -560,16 +584,30 @@ bool open_and_lock_for_insert_delayed(THD *thd, TABLE_LIST *table_list) if (open_and_lock_tables(thd, table_list->next_global, TRUE, 0)) { end_delayed_insert(thd); - DBUG_RETURN(TRUE); + error= TRUE; + } + else + { + /* + First table was not processed by open_and_lock_tables(), + we need to set updatability flag "by hand". + */ + if (!table_list->derived && !table_list->view) + table_list->updatable= 1; // usual table } - /* - First table was not processed by open_and_lock_tables(), - we need to set updatability flag "by hand". - */ - if (!table_list->derived && !table_list->view) - table_list->updatable= 1; // usual table - DBUG_RETURN(FALSE); } + + if (table_ticket) + thd->mdl_context.release_lock(table_ticket); + /* + Clone_ticket() in delayed_get_table() causes TABLE_LIST::MDL_REQUEST::ticket + to be overwritten with the cloned ticket. Reset the ticket here in case + we end up having to use normal insert. + */ + table_list->mdl_request.ticket= NULL; + + if (error || table_list->table) + DBUG_RETURN(error); #endif /* * This is embedded library and we don't have auxiliary @@ -2025,6 +2063,20 @@ bool delayed_get_table(THD *thd, TABLE_LIST *table_list) /* Replace volatile strings with local copies */ di->table_list.alias= di->table_list.table_name= di->thd.query(); di->table_list.db= di->thd.db; + + /* + Clone the ticket representing the lock on the target table for + the insert and add it to the list of granted metadata locks held by + the handler thread. This is safe since the handler thread is + not holding nor waiting on any metadata locks. + */ + if (di->thd.mdl_context.clone_ticket(&table_list->mdl_request)) + { + delete di; + my_error(ER_OUT_OF_RESOURCES, MYF(ME_FATALERROR)); + goto end_create; + } + di->lock(); mysql_mutex_lock(&di->mutex); if ((error= mysql_thread_create(key_thread_delayed_insert, @@ -2036,6 +2088,7 @@ bool delayed_get_table(THD *thd, TABLE_LIST *table_list) error)); mysql_mutex_unlock(&di->mutex); di->unlock(); + di->thd.mdl_context.release_lock(table_list->mdl_request.ticket); delete di; my_error(ER_CANT_CREATE_THREAD, MYF(ME_FATALERROR), error); goto end_create; -- cgit v1.2.1 From ee07ed22868f002701fd0a3b729caae220aafd18 Mon Sep 17 00:00:00 2001 From: Alfranio Correia Date: Mon, 23 Aug 2010 23:31:12 +0100 Subject: Post-fix push for BUG#53452. --- sql/log_event.cc | 3 ++- sql/sql_table.cc | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) (limited to 'sql') diff --git a/sql/log_event.cc b/sql/log_event.cc index c1f7836e08a..16290c58685 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -674,7 +674,8 @@ const char* Log_event::get_type_str() #ifndef MYSQL_CLIENT Log_event::Log_event(THD* thd_arg, uint16 flags_arg, bool using_trans) - :log_pos(0), temp_buf(0), exec_time(0), flags(flags_arg), thd(thd_arg) + :log_pos(0), temp_buf(0), exec_time(0), flags(flags_arg), + cache_type(Log_event::EVENT_INVALID_CACHE), thd(thd_arg) { server_id= thd->server_id; when= thd->start_time; diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 3bb7f7667ea..3f0a0326c84 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -4268,6 +4268,7 @@ bool mysql_create_table(THD *thd, TABLE_LIST *create_table, Alter_info *alter_info) { bool result; + bool is_trans= FALSE; DBUG_ENTER("mysql_create_table"); /* @@ -4282,7 +4283,6 @@ bool mysql_create_table(THD *thd, TABLE_LIST *create_table, /* Got lock. */ DEBUG_SYNC(thd, "locked_table_name"); - bool is_trans; result= mysql_create_table_no_lock(thd, create_table->db, create_table->table_name, create_info, alter_info, FALSE, 0, &is_trans); @@ -4454,6 +4454,7 @@ bool mysql_create_like_table(THD* thd, TABLE_LIST* table, TABLE_LIST* src_table, HA_CREATE_INFO local_create_info; Alter_info local_alter_info; bool res= TRUE; + bool is_trans= FALSE; uint not_used; DBUG_ENTER("mysql_create_like_table"); @@ -4503,7 +4504,6 @@ bool mysql_create_like_table(THD* thd, TABLE_LIST* table, TABLE_LIST* src_table, /* Reset auto-increment counter for the new table. */ local_create_info.auto_increment_value= 0; - bool is_trans; if ((res= mysql_create_table_no_lock(thd, table->db, table->table_name, &local_create_info, &local_alter_info, FALSE, 0, &is_trans))) -- cgit v1.2.1 From 1c09847b5f252baa59a3af78d003b4146473a974 Mon Sep 17 00:00:00 2001 From: Jon Olav Hauglid Date: Tue, 24 Aug 2010 16:00:17 +0200 Subject: Follow-up to Bug #54332 Deadlock with two connections doing LOCK TABLE+INSERT DELAYED The problem was that the server could crash if the insert delayed handler thread was killed due to a conflicting shared metadata lock. This could happen because the metadata lock ticket was added to the handler thread before it was properly initialized. This patch moves the cloning of the acquired metadata lock ticket until after the handler thread has been properly initialized. --- sql/sql_insert.cc | 88 ++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 55 insertions(+), 33 deletions(-) (limited to 'sql') diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index d9340cf843a..5f6ab43e2ea 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -563,15 +563,6 @@ bool open_and_lock_for_insert_delayed(THD *thd, TABLE_LIST *table_list) */ DBUG_RETURN(TRUE); - /* - If a lock was acquired above, we should release it after delayed_get_table() - has cloned the ticket for the handler thread. Note that acquire_lock() can - succeed because of a lock already held by the connection. In this case we - should not release it here. - */ - MDL_ticket *table_ticket = mdl_savepoint == thd->mdl_context.mdl_savepoint() ? - NULL: thd->mdl_context.mdl_savepoint(); - bool error= FALSE; if (delayed_get_table(thd, table_list)) error= TRUE; @@ -597,12 +588,18 @@ bool open_and_lock_for_insert_delayed(THD *thd, TABLE_LIST *table_list) } } - if (table_ticket) - thd->mdl_context.release_lock(table_ticket); /* - Clone_ticket() in delayed_get_table() causes TABLE_LIST::MDL_REQUEST::ticket - to be overwritten with the cloned ticket. Reset the ticket here in case - we end up having to use normal insert. + If a lock was acquired above, we should release it after + handle_delayed_insert() has cloned the ticket. Note that acquire_lock() can + succeed because the connection already has the lock. In this case the ticket + will be before the mdl_savepoint and we should not release it here. + */ + if (!thd->mdl_context.has_lock(mdl_savepoint, table_list->mdl_request.ticket)) + thd->mdl_context.release_lock(table_list->mdl_request.ticket); + + /* + Reset the ticket in case we end up having to use normal insert and + therefore will reopen the table and reacquire the metadata lock. */ table_list->mdl_request.ticket= NULL; @@ -1839,14 +1836,25 @@ public: mysql_cond_t cond, cond_client; volatile uint tables_in_use,stacked_inserts; volatile bool status; + /* + When the handler thread starts, it clones a metadata lock ticket + for the table to be inserted. This is done to allow the deadlock + detector to detect deadlocks resulting from this lock. + Before this is done, the connection thread cannot safely exit + without causing problems for clone_ticket(). + Once handler_thread_initialized has been set, it is safe for the + connection thread to exit. + Access to handler_thread_initialized is protected by di->mutex. + */ + bool handler_thread_initialized; COPY_INFO info; I_List rows; ulong group_count; TABLE_LIST table_list; // Argument Delayed_insert() - :locks_in_memory(0), - table(0),tables_in_use(0),stacked_inserts(0), status(0), group_count(0) + :locks_in_memory(0), table(0),tables_in_use(0),stacked_inserts(0), + status(0), handler_thread_initialized(FALSE), group_count(0) { DBUG_ENTER("Delayed_insert constructor"); thd.security_ctx->user=thd.security_ctx->priv_user=(char*) delayed_user; @@ -2063,19 +2071,9 @@ bool delayed_get_table(THD *thd, TABLE_LIST *table_list) /* Replace volatile strings with local copies */ di->table_list.alias= di->table_list.table_name= di->thd.query(); di->table_list.db= di->thd.db; - - /* - Clone the ticket representing the lock on the target table for - the insert and add it to the list of granted metadata locks held by - the handler thread. This is safe since the handler thread is - not holding nor waiting on any metadata locks. - */ - if (di->thd.mdl_context.clone_ticket(&table_list->mdl_request)) - { - delete di; - my_error(ER_OUT_OF_RESOURCES, MYF(ME_FATALERROR)); - goto end_create; - } + /* We need the ticket so that it can be cloned in handle_delayed_insert */ + init_mdl_requests(&di->table_list); + di->table_list.mdl_request.ticket= table_list->mdl_request.ticket; di->lock(); mysql_mutex_lock(&di->mutex); @@ -2088,15 +2086,20 @@ bool delayed_get_table(THD *thd, TABLE_LIST *table_list) error)); mysql_mutex_unlock(&di->mutex); di->unlock(); - di->thd.mdl_context.release_lock(table_list->mdl_request.ticket); delete di; my_error(ER_CANT_CREATE_THREAD, MYF(ME_FATALERROR), error); goto end_create; } - /* Wait until table is open */ + /* + Wait until table is open unless the handler thread or the connection + thread has been killed. Note that we in all cases must wait until the + handler thread has been properly initialized before exiting. Otherwise + we risk doing clone_ticket() on a ticket that is no longer valid. + */ thd_proc_info(thd, "waiting for handler open"); - while (!di->thd.killed && !di->table && !thd->killed) + while (!di->handler_thread_initialized || + (!di->thd.killed && !di->table && !thd->killed)) { mysql_cond_wait(&di->cond_client, &di->mutex); } @@ -2524,6 +2527,7 @@ pthread_handler_t handle_delayed_insert(void *arg) /* Can't use my_error since store_globals has not yet been called */ thd->stmt_da->set_error_status(thd, ER_OUT_OF_RESOURCES, ER(ER_OUT_OF_RESOURCES), NULL); + di->handler_thread_initialized= TRUE; } else { @@ -2534,6 +2538,7 @@ pthread_handler_t handle_delayed_insert(void *arg) /* Can't use my_error since store_globals has perhaps failed */ thd->stmt_da->set_error_status(thd, ER_OUT_OF_RESOURCES, ER(ER_OUT_OF_RESOURCES), NULL); + di->handler_thread_initialized= TRUE; thd->fatal_error(); goto err; } @@ -2546,7 +2551,24 @@ pthread_handler_t handle_delayed_insert(void *arg) thd->lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_INSERT_DELAYED); thd->set_current_stmt_binlog_format_row_if_mixed(); - init_mdl_requests(&di->table_list); + /* + Clone the ticket representing the lock on the target table for + the insert and add it to the list of granted metadata locks held by + the handler thread. This is safe since the handler thread is + not holding nor waiting on any metadata locks. + */ + if (thd->mdl_context.clone_ticket(&di->table_list.mdl_request)) + { + di->handler_thread_initialized= TRUE; + goto err; + } + + /* + Now that the ticket has been cloned, it is safe for the connection + thread to exit. + */ + di->handler_thread_initialized= TRUE; + di->table_list.mdl_request.ticket= NULL; if (di->open_and_lock_table()) goto err; -- cgit v1.2.1