Commit 664833fa authored by Jon Olav Hauglid's avatar Jon Olav Hauglid

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.
parent 12668c14
...@@ -563,15 +563,6 @@ bool open_and_lock_for_insert_delayed(THD *thd, TABLE_LIST *table_list) ...@@ -563,15 +563,6 @@ bool open_and_lock_for_insert_delayed(THD *thd, TABLE_LIST *table_list)
*/ */
DBUG_RETURN(TRUE); 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; bool error= FALSE;
if (delayed_get_table(thd, table_list)) if (delayed_get_table(thd, table_list))
error= TRUE; error= TRUE;
...@@ -597,12 +588,18 @@ bool open_and_lock_for_insert_delayed(THD *thd, TABLE_LIST *table_list) ...@@ -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 If a lock was acquired above, we should release it after
to be overwritten with the cloned ticket. Reset the ticket here in case handle_delayed_insert() has cloned the ticket. Note that acquire_lock() can
we end up having to use normal insert. 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; table_list->mdl_request.ticket= NULL;
...@@ -1839,14 +1836,25 @@ public: ...@@ -1839,14 +1836,25 @@ public:
mysql_cond_t cond, cond_client; mysql_cond_t cond, cond_client;
volatile uint tables_in_use,stacked_inserts; volatile uint tables_in_use,stacked_inserts;
volatile bool status; 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; COPY_INFO info;
I_List<delayed_row> rows; I_List<delayed_row> rows;
ulong group_count; ulong group_count;
TABLE_LIST table_list; // Argument TABLE_LIST table_list; // Argument
Delayed_insert() Delayed_insert()
:locks_in_memory(0), :locks_in_memory(0), table(0),tables_in_use(0),stacked_inserts(0),
table(0),tables_in_use(0),stacked_inserts(0), status(0), group_count(0) status(0), handler_thread_initialized(FALSE), group_count(0)
{ {
DBUG_ENTER("Delayed_insert constructor"); DBUG_ENTER("Delayed_insert constructor");
thd.security_ctx->user=thd.security_ctx->priv_user=(char*) delayed_user; 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) ...@@ -2063,19 +2071,9 @@ bool delayed_get_table(THD *thd, TABLE_LIST *table_list)
/* Replace volatile strings with local copies */ /* Replace volatile strings with local copies */
di->table_list.alias= di->table_list.table_name= di->thd.query(); di->table_list.alias= di->table_list.table_name= di->thd.query();
di->table_list.db= di->thd.db; di->table_list.db= di->thd.db;
/* We need the ticket so that it can be cloned in handle_delayed_insert */
/* init_mdl_requests(&di->table_list);
Clone the ticket representing the lock on the target table for di->table_list.mdl_request.ticket= table_list->mdl_request.ticket;
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(); di->lock();
mysql_mutex_lock(&di->mutex); mysql_mutex_lock(&di->mutex);
...@@ -2088,15 +2086,20 @@ bool delayed_get_table(THD *thd, TABLE_LIST *table_list) ...@@ -2088,15 +2086,20 @@ bool delayed_get_table(THD *thd, TABLE_LIST *table_list)
error)); error));
mysql_mutex_unlock(&di->mutex); mysql_mutex_unlock(&di->mutex);
di->unlock(); di->unlock();
di->thd.mdl_context.release_lock(table_list->mdl_request.ticket);
delete di; delete di;
my_error(ER_CANT_CREATE_THREAD, MYF(ME_FATALERROR), error); my_error(ER_CANT_CREATE_THREAD, MYF(ME_FATALERROR), error);
goto end_create; 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"); 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); mysql_cond_wait(&di->cond_client, &di->mutex);
} }
...@@ -2524,6 +2527,7 @@ pthread_handler_t handle_delayed_insert(void *arg) ...@@ -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 */ /* Can't use my_error since store_globals has not yet been called */
thd->stmt_da->set_error_status(thd, ER_OUT_OF_RESOURCES, thd->stmt_da->set_error_status(thd, ER_OUT_OF_RESOURCES,
ER(ER_OUT_OF_RESOURCES), NULL); ER(ER_OUT_OF_RESOURCES), NULL);
di->handler_thread_initialized= TRUE;
} }
else else
{ {
...@@ -2534,6 +2538,7 @@ pthread_handler_t handle_delayed_insert(void *arg) ...@@ -2534,6 +2538,7 @@ pthread_handler_t handle_delayed_insert(void *arg)
/* Can't use my_error since store_globals has perhaps failed */ /* Can't use my_error since store_globals has perhaps failed */
thd->stmt_da->set_error_status(thd, ER_OUT_OF_RESOURCES, thd->stmt_da->set_error_status(thd, ER_OUT_OF_RESOURCES,
ER(ER_OUT_OF_RESOURCES), NULL); ER(ER_OUT_OF_RESOURCES), NULL);
di->handler_thread_initialized= TRUE;
thd->fatal_error(); thd->fatal_error();
goto err; goto err;
} }
...@@ -2546,7 +2551,24 @@ pthread_handler_t handle_delayed_insert(void *arg) ...@@ -2546,7 +2551,24 @@ pthread_handler_t handle_delayed_insert(void *arg)
thd->lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_INSERT_DELAYED); thd->lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_INSERT_DELAYED);
thd->set_current_stmt_binlog_format_row_if_mixed(); 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()) if (di->open_and_lock_table())
goto err; goto err;
......
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