bk commit into 5.1 tree (dlenev:1.2575) BUG#21281

[dlenev]

Below is the list of changes that have just been committed into a local
5.1 repository of dlenev. When dlenev does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2007-08-02 09:56:00+04:00, dlenev@stripped +3 -0
  Proposed 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. 
  
  Questions for reviewers are marked by QQ.
  
  QQ: Should we fix this bug in 5.0 ?

  mysql-test/r/lock_multi.result@stripped, 2007-08-02 09:55:52+04:00, dlenev@stripped
+10 -0
    Added test for bug #21281 "Pending write lock is incorrectly removed when
    its statement being KILLed".

  mysql-test/t/lock_multi.test@stripped, 2007-08-02 09:55:52+04:00, dlenev@stripped
+35 -0
    Added test for bug #21281 "Pending write lock is incorrectly removed when
    its statement being KILLed".

  mysys/thr_lock.c@stripped, 2007-08-02 09:55:52+04:00, dlenev@stripped +34 -8
    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().

diff -Nrup a/mysql-test/r/lock_multi.result b/mysql-test/r/lock_multi.result
--- a/mysql-test/r/lock_multi.result	2006-10-05 02:36:01 +04:00
+++ b/mysql-test/r/lock_multi.result	2007-08-02 09:55:52 +04:00
@@ -95,3 +95,13 @@ alter table t1 auto_increment=0; alter t
 alter table t1 auto_increment=0; alter table t1 auto_increment=0; alter table t1
auto_increment=0; alter table t1 auto_increment=0; alter table t1 auto_increment=0; //
 unlock tables;
 drop table t1;
+create table t1 (i int);
+lock table t1 read;
+update t1 set i= 10;;
+select * from t1;;
+kill query ID;
+i
+ERROR 70100: Query execution was interrupted
+unlock tables;
+drop table t1;
+End of 5.1 tests
diff -Nrup a/mysql-test/t/lock_multi.test b/mysql-test/t/lock_multi.test
--- a/mysql-test/t/lock_multi.test	2007-02-27 13:39:27 +03:00
+++ b/mysql-test/t/lock_multi.test	2007-08-02 09:55:52 +04:00
@@ -270,3 +270,38 @@ drop table t1;
 
 # End of 5.0 tests
 
+
+#
+# Bug #21281 "Pending write lock is incorrectly removed when its
+#             statement being KILLed"
+#
+create table t1 (i int);
+connection locker;
+lock table t1 read;
+connection writer;
+--send update t1 set i= 10;
+connection reader;
+let $wait_condition=
+  select count(*) = 1 from information_schema.processlist
+  where state = "Locked" and info = "update t1 set i= 10";
+--source include/wait_condition.inc
+--send select * from t1;
+connection default;
+let $wait_condition=
+  select count(*) = 1 from information_schema.processlist
+  where state = "Locked" and info = "select * from t1";
+--source include/wait_condition.inc
+let $ID= `select id from information_schema.processlist where state = "Locked" and info =
"update t1 set i= 10"`;
+--replace_result $ID ID
+eval kill query $ID;
+connection reader;
+--reap
+connection writer;
+--error ER_QUERY_INTERRUPTED 
+--reap
+connection locker;
+unlock tables;
+connection default;
+drop table t1;
+
+--echo End of 5.1 tests
diff -Nrup a/mysys/thr_lock.c b/mysys/thr_lock.c
--- a/mysys/thr_lock.c	2007-05-10 13:59:26 +04:00
+++ b/mysys/thr_lock.c	2007-08-02 09:55:52 +04:00
@@ -384,6 +384,9 @@ static inline my_bool have_specific_lock
 }
 
 
+static void wake_up_waiters(THR_LOCK *lock);
+
+
 static enum enum_thr_lock_result
 wait_for_lock(struct st_lock_list *wait, THR_LOCK_DATA *data,
               my_bool in_wait_list)
@@ -445,8 +448,13 @@ wait_for_lock(struct st_lock_list *wait,
       else
 	wait->last=data->prev;
       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
   {
@@ -776,6 +784,28 @@ void thr_unlock(THR_LOCK_DATA *data)
     lock->read_no_write_count--;
   data->type=TL_UNLOCK;				/* Mark unlocked */
   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
+
+  QQ: May be there is a better name for this procedure?
+      Should this function be 'inline'?
+*/
+
+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 */
   {
@@ -827,11 +857,7 @@ void thr_unlock(THR_LOCK_DATA *data)
 	  data=lock->write_wait.data;		/* Free this too */
 	}
 	if (data->type >= TL_WRITE_LOW_PRIORITY)
-	{
-	  check_locks(lock,"giving write lock",0);
-	  pthread_mutex_unlock(&lock->mutex);
-	  DBUG_VOID_RETURN;
-	}
+          goto end;
 	/* Release possible read locks together with the write lock */
       }
       if (lock->read_wait.data)
@@ -886,8 +912,7 @@ void thr_unlock(THR_LOCK_DATA *data)
       free_all_read_locks(lock,0);
   }
 end:
-  check_locks(lock,"thr_unlock",0);
-  pthread_mutex_unlock(&lock->mutex);
+  check_locks(lock, "after waking up waiters", 0);
   DBUG_VOID_RETURN;
 }
 
@@ -1101,6 +1126,7 @@ my_bool thr_abort_locks_for_thread(THR_L
 	lock->write_wait.last= data->prev;
     }
   }
+  wake_up_waiters(lock);
   pthread_mutex_unlock(&lock->mutex);
   DBUG_RETURN(found);
 }