Description:
Background
----------
We are using a thread to open and lock tables. This lock thread (as I call it) owns its own THD object and has the necessary mutex and condition variable for use. The use of the locking thread allows us to have several drivers running at the same time who require the use of open_and_lock_tables(). But there is a problem -- we cannot kill the locking thread. :|
Problem
-------
We need a way to ensure the thread can be killed if we desire -- either via the backup kernel or when the thread that is running the kernel is killed. As it is now, the thread will not die when awakened with a KILL option. But thankfully behaves well in a normal system shutdown.
How to repeat:
Brief
-----
There are two ways to reproduce this bug.
1) Create a worker thread and try to kill it with awake(THD::KILL_CONNECTION).
2) Attempt to kill the worker thread from the SQL KILL <thread_id> command.
Neither method kills the thread.
More Information
----------------
This problem was found while working on BUG#31383. The online backup kernel has a design feature that allows for cancelling the backup process. A worker thread is used in a portion of the online backup code to open and lock tables.
The code to cancel the worker thread is:
lock_thd->awake(THD::KILL_CONNECTION);
The thread is in a sleeping state then this occurs. However, the locking thread is never killed and still shows up in the processlist.
I have included a sample of the locking thread code below.
/**
@brief Lock tables in driver.
This method creates a new THD for use in the new thread. It calls
the method to open and lock the tables.
*/
pthread_handler_t backup_thread_for_locking(void *arg) {
Backup_thread_driver *drv= static_cast<Backup_thread_driver *>(arg);
pthread_mutex_t THR_LOCK_driver_thread= drv->THR_LOCK_driver_thread;
pthread_cond_t COND_driver_thread_wait= drv->COND_driver_thread_wait;
DBUG_PRINT("info", ("Default_backup - lock_tables_in_separate_thread"));
/*
Turn off condition variable check for lock.
*/
pthread_mutex_lock(&THR_LOCK_driver_thread);
drv->lock_called= FALSE;
pthread_mutex_unlock(&THR_LOCK_driver_thread);
my_thread_init();
pthread_detach_this_thread();
/*
First, create a new THD object.
*/
DBUG_PRINT("info",("Online backup creating THD struct for thread"));
THD *thd= new THD;
if (unlikely(!thd))
{
delete thd;
return (0);
}
drv->lock_thd= thd;
thd->thread_stack = (char*)&thd; // remember where our stack is
pthread_mutex_lock(&LOCK_thread_count);
thd->thread_id= thread_id++;
pthread_mutex_unlock(&LOCK_thread_count);
if (unlikely(thd->store_globals())) // for a proper MEM_ROOT
{
delete thd;
return (0);
}
thd->init_for_queries(); // opening tables needs a proper LEX
thd->command= COM_DAEMON;
thd->system_thread= SYSTEM_THREAD_BACKUP;
thd->version= refresh_version;
thd->set_time();
thd->main_security_ctx.host_or_ip= "";
thd->client_capabilities= 0;
my_net_init(&thd->net, 0);
thd->main_security_ctx.master_access= ~0;
thd->main_security_ctx.priv_user= 0;
thd->real_id= pthread_self();
/*
Making this thread visible to SHOW PROCESSLIST is useful for
troubleshooting a backup job (why does it stall etc).
*/
pthread_mutex_lock(&LOCK_thread_count);
threads.append(thd);
pthread_mutex_unlock(&LOCK_thread_count);
lex_start(thd);
mysql_reset_thd_for_next_command(thd);
if (thd->killed)
goto end;
/*
Now open and lock the tables.
*/
DBUG_PRINT("info",("Online backup open tables in thread"));
if (!drv->tables_in_backup)
{
DBUG_PRINT("info",("Online backup locking error no tables to lock"));
return (0);
}
/*
As locking tables can be a long operation, we need to support
cancellability during that time. So we publish our THD to the thread which
created us.
*/
if (!thd->killed && open_and_lock_tables(thd, drv->tables_in_backup))
{
DBUG_PRINT("info",("Online backup locking thread dying"));
close_thread_tables(thd);
return (0);
}
if (thd->killed)
goto end;
/*
Turn on condition variable check for lock.
*/
pthread_mutex_lock(&THR_LOCK_driver_thread);
drv->lock_called= TRUE;
pthread_mutex_unlock(&THR_LOCK_driver_thread);
/*
Part of work is done. Rest until woken up.
*/
pthread_mutex_lock(&THR_LOCK_driver_thread);
thd->enter_cond(&COND_driver_thread_wait, &THR_LOCK_driver_thread,
"Online backup driver thread: holding table locks");
while (!thd->killed)
pthread_cond_wait(&COND_driver_thread_wait, &THR_LOCK_driver_thread);
thd->exit_cond("Online backup driver thread: terminating");
DBUG_PRINT("info",("Online backup driver thread locking thread terminating"));
end:
/*
Cleanup and return.
*/
close_thread_tables(thd);
net_end(&thd->net);
my_thread_end();
delete thd;
pthread_exit(0);
return (0);
}
Suggested fix:
Guilhem has traced the problem to the method THD::awake() which has the condition:
if (!system_thread) // Don't abort locks
mysys_var->abort=1;
However, we set the locking thread to have system_thread!=0. This means that mysys_var->abort won't be set to 1 and the locking thread will not die. We have a condition wait loop that will eventually fire leaving the thread waiting perpetually for the condition broadcast (which will never come if the backup is cancelled). So if online backup driver or the kernel is killed or if the backup process is killed, the locking thread may not die.
Proposed Solution
-----------------
Guilhem has proposed changing the above condition to:
if (system_thread != SYSTEM_THREAD_DELAYED_INSERT) // Don't abort locks
mysys_var->abort=1;
Which should allow all but the delayed insert threads to abort.
