From: Date: July 21 2008 4:16pm Subject: bzr push into mysql-6.0-falcon branch (vvaintroub:2749 to 2751) Bug#22165, Bug#35072 List-Archive: http://lists.mysql.com/commits/50119 X-Bug: 22165 Message-Id: <200807211416.m6LEGuY7013042@mail.mysql.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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_)