bk commit into 6.0 tree (klewis:1.2610) BUG#34890

[klewis]

Below is the list of changes that have just been committed into a local
6.0 repository of klewis.  When klewis 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, 2008-03-27 01:27:57-05:00, klewis@klewis-mysql. +6 -0
  Bug#34890 - hold the SyncObject::mutex to protect thread->lockGranted and 
  send it into thread->sleep so that it will overlap the PTHREADS mutex.  
  
  This prevents a race condition where a thread is woken up inadvertently,
  and while it is looping around to go back to sleep, it somehow misses
  the event or signal by the thread holding the lock.  When this happens,
  the threads waits on that SyncObject until another thread locks and 
  releases it.  This is a thread stall.

  mysql-test/suite/falcon/r/falcon_bug_34890.result@stripped, 2008-03-27 01:27:35-05:00,
klewis@klewis-mysql. +24 -9
    Bug#34890 - Use 5 clients in this test concurrently running P1().

  mysql-test/suite/falcon/t/disabled.def@stripped, 2008-03-27 01:27:38-05:00,
klewis@klewis-mysql. +2 -2
    Bug#34890 - activate tests

  mysql-test/suite/falcon/t/falcon_bug_34890.test@stripped, 2008-03-27 01:27:41-05:00,
klewis@klewis-mysql. +54 -14
    Bug#34890 - Use 5 clients in this test concurrently running P1().

  storage/falcon/SyncObject.cpp@stripped, 2008-03-27 01:27:43-05:00, klewis@klewis-mysql. +16
-15
    Bug#34890 - hold the mutex to protect thread->lockGranted and send it 
    into thread->sleep so that it will overlap the PTHREADS mutex.  

  storage/falcon/Synchronize.cpp@stripped, 2008-03-27 01:27:47-05:00, klewis@klewis-mysql. +17
-0
    Bug#34890 - hold the mutex to protect thread->lockGranted and send it 
    into thread->sleep so that it will overlap the PTHREADS mutex.  

  storage/falcon/Synchronize.h@stripped, 2008-03-27 01:27:50-05:00, klewis@klewis-mysql. +3 -0
    Bug#34890 - hold the mutex to protect thread->lockGranted and send it 
    into thread->sleep so that it will overlap the PTHREADS mutex.  

diff -Nrup a/mysql-test/suite/falcon/r/falcon_bug_34890.result
b/mysql-test/suite/falcon/r/falcon_bug_34890.result
--- a/mysql-test/suite/falcon/r/falcon_bug_34890.result	2008-03-05 14:25:33 -06:00
+++ b/mysql-test/suite/falcon/r/falcon_bug_34890.result	2008-03-27 01:27:35 -05:00
@@ -4,6 +4,12 @@ DROP TABLE IF EXISTS t1;
 DROP PROCEDURE IF EXISTS p1;
 # Establish connection conn1 (user = root)
 SET @@autocommit = 1;
+# Establish connection conn2 (user = root)
+SET @@autocommit = 1;
+# Establish connection conn3 (user = root)
+SET @@autocommit = 1;
+# Establish connection conn4 (user = root)
+SET @@autocommit = 1;
 # Switch to connection default
 SET @@autocommit = 1;
 CREATE TABLE t1 (
@@ -16,26 +22,35 @@ begin
 DECLARE my_count INT DEFAULT 0;
 DECLARE my_uuid CHAR(36) DEFAULT 0;
 SET FALCON_CONSISTENT_READ=OFF;
-while my_count < 10000 do
+while my_count < 5000 do
 SET my_uuid = UUID();
 INSERT INTO t1 (t1_uuid) VALUES (my_uuid);
 DELETE FROM t1 WHERE t1_uuid IN (my_uuid);
 SET my_count = my_count + 1;
 end while;
 end//
+# Switch to connection conn1
 # Send call p1() to the server but do not pull the results
 CALL p1();
-# Switch to connection conn1
+# Switch to connection conn2
+# Send call p1() to the server but do not pull the results
 CALL p1();
-# Switch to connection default
-# Pull the results of the preceeding call p1()
+# Switch to connection conn3
 # Send call p1() to the server but do not pull the results
 CALL p1();
-# Switch to connection conn1
+# Switch to connection conn4
+# Send call p1() to the server but do not pull the results
 CALL p1();
 # Switch to connection default
-# Pull the results of the preceeding call p1()
-SELECT count(*) FROM t1;
-count(*)
-0
+CALL p1();
+# Switch to connection conn1
+# Pull the results of the preceeding call p1() by conn1
+# Switch to connection conn2
+# Pull the results of the preceeding call p1() by conn2
+# Switch to connection conn3
+# Pull the results of the preceeding call p1() by conn3
+# Switch to connection conn4
+# Pull the results of the preceeding call p1() by conn4
+# Switch to connection default
 DROP PROCEDURE p1;
+DROP TABLE t1;
diff -Nrup a/mysql-test/suite/falcon/t/disabled.def
b/mysql-test/suite/falcon/t/disabled.def
--- a/mysql-test/suite/falcon/t/disabled.def	2008-03-22 14:26:50 -05:00
+++ b/mysql-test/suite/falcon/t/disabled.def	2008-03-27 01:27:38 -05:00
@@ -32,8 +32,8 @@ falcon_bug_30282    : Bug#30282 2007-08-
 falcon_bug_30480_A  : Bug#30282 2007-09-07 klewis Currently failing
 falcon_bug_30480_B  : Bug#30282 2007-09-07 klewis Currently failing
 falcon_bug_32413    : Bug#32413 2008-02-11 hakank Either works on 64-bit or on 32-bit,
but not on both platforms. This is a test case problem and not a bug.
-falcon_bug_34351_C  : Bug#35503 2008-03-22 hakank Times out from time to time
-falcon_bug_34890    : Bug#34890 2008-02-11 klewis shows a faux lock timeout
+#falcon_bug_34351_C  : Bug#35503 2008-03-22 hakank Times out from time to time
+#falcon_bug_34890    : Bug#34890 2008-02-11 klewis shows a faux lock timeout
 falcon_bug_34892    : Bug#34892 2008-03-04 hakank (Some falcon tests fail sporadically)
 falcon_deadlock     : Bug#34182 2008-01-31 hakank Bug in SELECT ... FOR UPDATE
 falcon_record_cache_memory_leak2    : Bug#34894 2008-02-27 hakank Fails on Pushbuild
Windows
diff -Nrup a/mysql-test/suite/falcon/t/falcon_bug_34890.test
b/mysql-test/suite/falcon/t/falcon_bug_34890.test
--- a/mysql-test/suite/falcon/t/falcon_bug_34890.test	2008-03-05 14:25:30 -06:00
+++ b/mysql-test/suite/falcon/t/falcon_bug_34890.test	2008-03-27 01:27:41 -05:00
@@ -22,6 +22,18 @@ DROP PROCEDURE IF EXISTS p1;
 connect (conn1,localhost,root,,);
 SET @@autocommit = 1;
 
+--echo # Establish connection conn2 (user = root)
+connect (conn2,localhost,root,,);
+SET @@autocommit = 1;
+
+--echo # Establish connection conn3 (user = root)
+connect (conn3,localhost,root,,);
+SET @@autocommit = 1;
+
+--echo # Establish connection conn4 (user = root)
+connect (conn4,localhost,root,,);
+SET @@autocommit = 1;
+
 --echo # Switch to connection default
 connection default;
 SET @@autocommit = 1;
@@ -40,7 +52,7 @@ begin
   DECLARE my_count INT DEFAULT 0;
   DECLARE my_uuid CHAR(36) DEFAULT 0;
   SET FALCON_CONSISTENT_READ=OFF;
-  while my_count < 10000 do
+  while my_count < 5000 do
     SET my_uuid = UUID();
     INSERT INTO t1 (t1_uuid) VALUES (my_uuid);
     DELETE FROM t1 WHERE t1_uuid IN (my_uuid);
@@ -53,40 +65,68 @@ delimiter ;//
 # --- Test                                          --- #
 # ----------------------------------------------------- #
 
+--echo # Switch to connection conn1
+connection conn1;
 --echo # Send call p1() to the server but do not pull the results
 --send CALL p1()
 
---echo # Switch to connection conn1
-connection conn1;
---real_sleep 1
-CALL p1();
+--echo # Switch to connection conn2
+connection conn2;
+--echo # Send call p1() to the server but do not pull the results
+--send CALL p1()
+
+--echo # Switch to connection conn3
+connection conn3;
+--echo # Send call p1() to the server but do not pull the results
+--send CALL p1()
+
+--echo # Switch to connection conn4
+connection conn4;
+--echo # Send call p1() to the server but do not pull the results
+--send CALL p1()
 
 --echo # Switch to connection default
 connection default;
---echo # Pull the results of the preceeding call p1()
---reap
---echo # Send call p1() to the server but do not pull the results
 --real_sleep 1
---send CALL p1()
+CALL p1();
 
 --echo # Switch to connection conn1
 connection conn1;
---real_sleep 1
-CALL p1();
+--echo # Pull the results of the preceeding call p1() by conn1
+--reap
+
+--echo # Switch to connection conn2
+connection conn2;
+--echo # Pull the results of the preceeding call p1() by conn2
+--reap
+
+--echo # Switch to connection conn3
+connection conn3;
+--echo # Pull the results of the preceeding call p1() by conn3
+--reap
+
+--echo # Switch to connection conn4
+connection conn4;
+--echo # Pull the results of the preceeding call p1() by conn4
+--reap
+
 
 --echo # Switch to connection default
 connection default;
---echo # Pull the results of the preceeding call p1()
---reap
 
 
 # ----------------------------------------------------- #
 # --- Check                                         --- #
 # ----------------------------------------------------- #
 # Checking row count is not applicable here.
-SELECT count(*) FROM t1;
+#SELECT count(*) FROM t1;
 
 # ----------------------------------------------------- #
 # --- Final cleanup                                 --- #
 # ----------------------------------------------------- #
+disconnect conn1;
+disconnect conn2;
+disconnect conn3;
+disconnect conn4;
 DROP PROCEDURE p1;
+DROP TABLE t1;
diff -Nrup a/storage/falcon/SyncObject.cpp b/storage/falcon/SyncObject.cpp
--- a/storage/falcon/SyncObject.cpp	2008-03-24 22:54:47 -05:00
+++ b/storage/falcon/SyncObject.cpp	2008-03-27 01:27:43 -05:00
@@ -256,6 +256,8 @@ void SyncObject::lock(Sync *sync, LockTy
 			}
 		}
 
+    // mutex is held going into wait() It is released before coming out.
+
 	wait(type, thread, sync, timeout);
 	DEBUG_FREEZE;
 }
@@ -319,6 +321,8 @@ void SyncObject::downGrade(LockType type
 
 void SyncObject::wait(LockType type, Thread *thread, Sync *sync, int timeout)
 {
+	// mutex is currently held
+
 	++stalls;
 	BUMP(waitCount);
 
@@ -351,23 +355,16 @@ void SyncObject::wait(LockType type, Thr
 	thread->lockGranted = false;
 	thread->lockPending = sync;
 	++thread->activeLocks;
-	mutex.release();
-	bool wakeup = 0;
+	bool wokeup = 0;
 
 	if (timeout)
 		while (!thread->lockGranted)
 			{
-			wakeup = thread->sleep (timeout);
-			
-			if (thread->lockGranted)
-				return;
-			
-			mutex.lock();
+			wokeup = thread->sleep (timeout, &mutex);
 			
 			if (thread->lockGranted)
 				{
-				mutex.unlock();
-				
+				mutex.release();
 				return;
 				}
 			
@@ -379,30 +376,34 @@ void SyncObject::wait(LockType type, Thr
 					break;
 					}
 			
-			mutex.unlock();
-			if (!wakeup)
+			if (!wokeup)
+				{
+				mutex.release();
 				timedout(timeout);
+				}
 			}
 		
 		
 	while (!thread->lockGranted)
 		{
-		wakeup = thread->sleep (10000);
+		wokeup = thread->sleep (10000, &mutex);
 		
 		if (thread->lockGranted)
 			{
-			if (!wakeup)
+			if (!wokeup)
 				Log::debug("Apparent lost thread wakeup\n");
 
 			break;
 			}
 			
-		if (!wakeup)
+		if (!wokeup)
 			{
 			stalled (thread);
 			break;
 			}
 		}
+
+	mutex.release();
 
 	while (!thread->lockGranted)
 		thread->sleep();
diff -Nrup a/storage/falcon/Synchronize.cpp b/storage/falcon/Synchronize.cpp
--- a/storage/falcon/Synchronize.cpp	2008-03-11 10:16:31 -05:00
+++ b/storage/falcon/Synchronize.cpp	2008-03-27 01:27:47 -05:00
@@ -34,6 +34,7 @@
 #include "Engine.h"
 #include "Synchronize.h"
 #include "Interlock.h"
+#include "Mutex.h"
 
 #ifdef ENGINE
 #include "Log.h"
@@ -106,7 +107,9 @@ void Synchronize::sleep()
 			}
 		}
 #else
+	sleeping = true;
 	sleep (INFINITE);
+	sleeping = false;
 #endif
 #endif
 
@@ -129,10 +132,19 @@ void Synchronize::sleep()
 
 bool Synchronize::sleep(int milliseconds)
 {
+	return sleep(milliseconds, NULL);
+}
+
+bool Synchronize::sleep(int milliseconds, Mutex *callersMutex)
+{
 	sleeping = true;
 
 #ifdef _WIN32
+	if (callersMutex)
+		callersMutex->release();
 	int n = WaitForSingleObject(event, milliseconds);
+	if (callersMutex)
+		callersMutex->lock();
 	sleeping = false;
 	DEBUG_FREEZE;
 
@@ -142,7 +154,10 @@ bool Synchronize::sleep(int milliseconds
 #ifdef _PTHREADS
 	int ret = pthread_mutex_lock (&mutex);
 	CHECK_RET("pthread_mutex_lock failed, errno %d", errno);
+	if (callersMutex)
+		callersMutex->release();
 	struct timespec nanoTime;
+
 #if _POSIX_TIMERS > 0
 	ret = clock_gettime(CLOCK_REALTIME, &nanoTime);
 	CHECK_RET("clock_gettime failed, errno %d", errno);
@@ -189,6 +204,8 @@ bool Synchronize::sleep(int milliseconds
 
 	sleeping = false;
 	wakeup = false;
+	if (callersMutex)
+		callersMutex->lock();
 	pthread_mutex_unlock(&mutex);
 	DEBUG_FREEZE;
 
diff -Nrup a/storage/falcon/Synchronize.h b/storage/falcon/Synchronize.h
--- a/storage/falcon/Synchronize.h	2007-10-12 15:52:31 -05:00
+++ b/storage/falcon/Synchronize.h	2008-03-27 01:27:50 -05:00
@@ -24,6 +24,8 @@
 #pragma once
 #endif // _MSC_VER >= 1000
 
+#include <Mutex.h>
+
 #ifdef _PTHREADS
 #include <pthread.h>
 #endif
@@ -51,6 +53,7 @@ public:
 	virtual void shutdown();
 	virtual void wake();
 	virtual bool sleep (int milliseconds);
+	virtual bool sleep (int milliseconds, Mutex *callersMutex);
 	virtual void sleep();
 	Synchronize();
 	virtual ~Synchronize();