List:Commits« Previous MessageNext Message »
From:Vladislav Vaintroub Date:July 21 2008 4:16pm
Subject:bzr push into mysql-6.0-falcon branch (vvaintroub:2749 to 2751) Bug#22165,
Bug#35072
View as plain text  
 2751 Vladislav Vaintroub	2008-07-21
      Accoding to Kevin, Bug#22165 could have been fixed with his fix to 36438.
      This change reactivates the test case
renamed:
  mysql-test/suite/falcon_team/r/falcon_bug_22165.result =>
mysql-test/suite/falcon/r/falcon_bug_22165.result
  mysql-test/suite/falcon_team/t/falcon_bug_22165.test =>
mysql-test/suite/falcon/t/falcon_bug_22165.test

 2750 Vladislav Vaintroub	2008-07-18
      Bug#35072: Crash Falcon crash in RecoveryObjects::findRecoveryObject
      Problem: The functiion crashed while traversing single linked list 
      that contained a dangling pointer (deleted memory, filled with 0xe bytes).
      Access and modification of the hash table was not synchronized.
      When one thread adds a new entry to the hashtable while another
      is delets from the same hash bucket,  hashtable can become corrupted 
      (i.e still have pointer to the deleted object)
      
      Solution is to synchronize hashtable access. Here, a reader/writer
      lock is used, one per hash bucket.
modified:
  storage/falcon/RecoveryObjects.cpp
  storage/falcon/RecoveryObjects.h

 2749 Kevin Lewis	2008-07-15
      Cleanup code.  Some white space cleanup, but mostly making the 
      text identifiers for Sync constructors accurate and unique.
      Alo comment out debug calls to validate in WalkIndex.cpp

=== renamed file 'mysql-test/suite/falcon_team/r/falcon_bug_22165.result' =>
'mysql-test/suite/falcon/r/falcon_bug_22165.result'
=== renamed file 'mysql-test/suite/falcon_team/t/falcon_bug_22165.test' =>
'mysql-test/suite/falcon/t/falcon_bug_22165.test'
=== modified file 'storage/falcon/RecoveryObjects.cpp'
--- a/storage/falcon/RecoveryObjects.cpp	2007-09-20 15:44:25 +0000
+++ b/storage/falcon/RecoveryObjects.cpp	2008-07-18 21:24:52 +0000
@@ -23,6 +23,8 @@
 #include "RecoveryObjects.h"
 #include "RecoveryPage.h"
 #include "SerialLog.h"
+#include "SyncObject.h"
+#include "Sync.h"
 
 #ifdef _DEBUG
 #undef THIS_FILE
@@ -95,17 +97,24 @@ bool RecoveryObjects::isObjectActive(int
 	return object->pass1Count == object->currentCount;
 }
 
-RecoveryPage* RecoveryObjects::findRecoveryObject(int objectNumber, int tableSpaceId)
-{
-	int slot = objectNumber % RPG_HASH_SIZE;
 
-	for (RecoveryPage *object = recoveryObjects[slot]; object; object =
object->collision)
+static inline RecoveryPage * findInHashBucket(RecoveryPage *head, int objectNumber, int
tableSpaceId)
+{
+	for (RecoveryPage *object = head ; object; object = object->collision)
 		if (object->objectNumber == objectNumber && object->tableSpaceId ==
tableSpaceId)
 			return object;
-
 	return NULL;
 }
 
+
+RecoveryPage* RecoveryObjects::findRecoveryObject(int objectNumber, int tableSpaceId)
+{
+	int slot = objectNumber % RPG_HASH_SIZE;
+	Sync sync(&syncArray[slot], "RecoveryObjects::findRecoveryObject");
+	sync.lock(Shared);
+	return findInHashBucket(recoveryObjects[slot], objectNumber, tableSpaceId);
+}
+
 void RecoveryObjects::setActive(int objectNumber, int tableSpaceId)
 {
 	RecoveryPage *object = findRecoveryObject(objectNumber, tableSpaceId);
@@ -129,10 +138,25 @@ RecoveryPage* RecoveryObjects::getRecove
 	int slot = objectNumber % RPG_HASH_SIZE;
 	RecoveryPage *object;
 
-	for (object = recoveryObjects[slot]; object; object = object->collision)
-		if (object->objectNumber == objectNumber && object->tableSpaceId ==
tableSpaceId)
-			return object;
+	Sync sync(&syncArray[slot], "RecoveryObjects::getRecoveryObject");
+	sync.lock(Shared);
+	object = findInHashBucket(recoveryObjects[slot], objectNumber, tableSpaceId);
+
+	if(object)
+		return object;
+
+	// Object not found, insert (need exlusive lock for this)
+	sync.unlock();
+	sync.lock(Exclusive);
+
+	// We need to traverse the collision list once again. Another thread 
+	// may have inserted the entry while current thread was waiting 
+	// for exclusive lock.
+	object = findInHashBucket(recoveryObjects[slot], objectNumber, tableSpaceId);
+	if (object)
+		return object;
 
+	// Add object to the start of the collision list
 	object = new RecoveryPage(objectNumber, tableSpaceId);
 	object->collision = recoveryObjects[slot];
 	recoveryObjects[slot] = object;
@@ -143,7 +167,9 @@ RecoveryPage* RecoveryObjects::getRecove
 void RecoveryObjects::deleteObject(int objectNumber, int tableSpaceId)
 {
 	int slot = objectNumber % RPG_HASH_SIZE;
-	
+	Sync sync(&syncArray[slot], "RecoveryObjects::deleteObject");
+	sync.lock(Exclusive);
+
 	for (RecoveryPage **ptr = recoveryObjects + slot, *object; (object = *ptr); ptr =
&object->collision)
 		if (object->objectNumber == objectNumber && object->tableSpaceId ==
tableSpaceId)
 			{

=== modified file 'storage/falcon/RecoveryObjects.h'
--- a/storage/falcon/RecoveryObjects.h	2007-09-20 15:44:25 +0000
+++ b/storage/falcon/RecoveryObjects.h	2008-07-18 21:24:52 +0000
@@ -26,6 +26,7 @@
 
 static const int RPG_HASH_SIZE			= 101;
 
+#include "SyncObject.h"
 class RecoveryPage;
 class SerialLog;
 
@@ -48,6 +49,7 @@ public:
 
 	SerialLog			*serialLog;
 	RecoveryPage		*recoveryObjects[RPG_HASH_SIZE];
+	SyncObject			syncArray[RPG_HASH_SIZE];
 };
 
 #endif //
!defined(AFX_RECOVERYOBJECTS_H__00C7CE5F_3C33_435C_9521_9C274CAB0581__INCLUDED_)

Thread
bzr push into mysql-6.0-falcon branch (vvaintroub:2749 to 2751) Bug#22165,Bug#35072Vladislav Vaintroub21 Jul