Commit af2d0f87 authored by unknown's avatar unknown

Fix for bug #21281 "Pending write lock is incorrectly removed when its

statement being KILLed".

When statement which was trying to obtain write lock on then table and
which was blocked by existing read lock was killed, concurrent statements
that were trying to obtain read locks on the same table and that were
blocked by the presence of this pending write lock were not woken up and
had to wait until this first read lock goes away.

This problem was caused by the fact that we forgot to wake up threads
which pending requests could have been satisfied after removing lock
request for the killed thread.

The patch solves the problem by waking up those threads in such situation.

Test for this bug will be added to 5.1 only as it has much better
facilities for its implementation. Particularly, by using I_S.PROCESSLIST
and wait_condition.inc script we can wait until thread will be blocked on
certain table lock without relying on unconditional sleep (which usage
increases time needed for test runs and might cause spurious test
failures on slower platforms).


mysys/thr_lock.c:
  After removing lock request from the list of waiting lock requests
  (e.g. when we discover that current thread was killed) we should
  wake up other threads waiting for the same lock which pending
  requests now can be satisfied. To implement this behavior we
  move code responsible for waking up threads which pending requests
  can be satisfied from thr_unlock() to new wake_up_waiters() procedure
  and use it in wait_for_lock() and hr_abort_locks_for_thread().
parent 586304c8
...@@ -383,6 +383,9 @@ static inline my_bool have_specific_lock(THR_LOCK_DATA *data, ...@@ -383,6 +383,9 @@ static inline my_bool have_specific_lock(THR_LOCK_DATA *data,
} }
static void wake_up_waiters(THR_LOCK *lock);
static enum enum_thr_lock_result static enum enum_thr_lock_result
wait_for_lock(struct st_lock_list *wait, THR_LOCK_DATA *data, wait_for_lock(struct st_lock_list *wait, THR_LOCK_DATA *data,
my_bool in_wait_list) my_bool in_wait_list)
...@@ -444,8 +447,13 @@ wait_for_lock(struct st_lock_list *wait, THR_LOCK_DATA *data, ...@@ -444,8 +447,13 @@ wait_for_lock(struct st_lock_list *wait, THR_LOCK_DATA *data,
else else
wait->last=data->prev; wait->last=data->prev;
data->type= TL_UNLOCK; /* No lock */ data->type= TL_UNLOCK; /* No lock */
check_locks(data->lock, "killed or timed out wait_for_lock", 1);
wake_up_waiters(data->lock);
}
else
{
check_locks(data->lock, "aborted wait_for_lock", 0);
} }
check_locks(data->lock,"failed wait_for_lock",0);
} }
else else
{ {
...@@ -771,6 +779,26 @@ void thr_unlock(THR_LOCK_DATA *data) ...@@ -771,6 +779,26 @@ void thr_unlock(THR_LOCK_DATA *data)
lock->read_no_write_count--; lock->read_no_write_count--;
data->type=TL_UNLOCK; /* Mark unlocked */ data->type=TL_UNLOCK; /* Mark unlocked */
check_locks(lock,"after releasing lock",1); check_locks(lock,"after releasing lock",1);
wake_up_waiters(lock);
pthread_mutex_unlock(&lock->mutex);
DBUG_VOID_RETURN;
}
/**
@brief Wake up all threads which pending requests for the lock
can be satisfied.
@param lock Lock for which threads should be woken up
*/
static void wake_up_waiters(THR_LOCK *lock)
{
THR_LOCK_DATA *data;
enum thr_lock_type lock_type;
DBUG_ENTER("wake_up_waiters");
if (!lock->write.data) /* If no active write locks */ if (!lock->write.data) /* If no active write locks */
{ {
...@@ -820,11 +848,7 @@ void thr_unlock(THR_LOCK_DATA *data) ...@@ -820,11 +848,7 @@ void thr_unlock(THR_LOCK_DATA *data)
data=lock->write_wait.data; /* Free this too */ data=lock->write_wait.data; /* Free this too */
} }
if (data->type >= TL_WRITE_LOW_PRIORITY) if (data->type >= TL_WRITE_LOW_PRIORITY)
{ goto end;
check_locks(lock,"giving write lock",0);
pthread_mutex_unlock(&lock->mutex);
DBUG_VOID_RETURN;
}
/* Release possible read locks together with the write lock */ /* Release possible read locks together with the write lock */
} }
if (lock->read_wait.data) if (lock->read_wait.data)
...@@ -879,8 +903,7 @@ void thr_unlock(THR_LOCK_DATA *data) ...@@ -879,8 +903,7 @@ void thr_unlock(THR_LOCK_DATA *data)
free_all_read_locks(lock,0); free_all_read_locks(lock,0);
} }
end: end:
check_locks(lock,"thr_unlock",0); check_locks(lock, "after waking up waiters", 0);
pthread_mutex_unlock(&lock->mutex);
DBUG_VOID_RETURN; DBUG_VOID_RETURN;
} }
...@@ -1094,6 +1117,7 @@ my_bool thr_abort_locks_for_thread(THR_LOCK *lock, pthread_t thread) ...@@ -1094,6 +1117,7 @@ my_bool thr_abort_locks_for_thread(THR_LOCK *lock, pthread_t thread)
lock->write_wait.last= data->prev; lock->write_wait.last= data->prev;
} }
} }
wake_up_waiters(lock);
pthread_mutex_unlock(&lock->mutex); pthread_mutex_unlock(&lock->mutex);
DBUG_RETURN(found); DBUG_RETURN(found);
} }
......
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