bzr commit into mysql-6.0-falcon branch (cpowers:2815) Bug#39350

[Christopher Powers]

#At file:///home/cpowers/work/dev/dev-07/mysql/

 2815 Christopher Powers	2008-09-09
      Bug#39350 Falcon: Releasing deferred indexes causes crash
      
      - Transactions now bump the reference count for deferred indexes
      - Transaction::releaseDeferredIndexes(Table *) now gets exclusive lock on
syncDeferredIndexes
modified:
  storage/falcon/DeferredIndex.cpp
  storage/falcon/DeferredIndexWalker.cpp
  storage/falcon/StorageTableShare.cpp
  storage/falcon/StorageTableShare.h
  storage/falcon/Transaction.cpp
  storage/falcon/Transaction.h

per-file messages:
  storage/falcon/DeferredIndex.cpp
    Remove releaseRef() from DeferredIndex::detachTransaction()
    Added usecount assert to ::releaseRef()
  storage/falcon/DeferredIndexWalker.cpp
    Increment deferred index reference count in DeferredIndexWalker::initialize()
    Decrement in ~DeferredIndexWalker()
  storage/falcon/StorageTableShare.cpp
    Renamed syncIndexes to syncIndexMap
  storage/falcon/StorageTableShare.h
    Renamed syncIndexes to syncIndexMap
  storage/falcon/Transaction.cpp
    Renamed Transaction::syncIndexes to syncDeferredIndexes to reduce confusion
    Transaction::add(DeferredIndex *) now bumps deferred index reference count
    Transaction::releaseDeferredIndexes() now releases deferred index reference count
    Transaction::releaseDeferredIndexes(Table *) now gets exclusive lock on
syncDeferredIndexes
  storage/falcon/Transaction.h
    Renamed syncIndexes to syncDeferredIndexes
=== modified file 'storage/falcon/DeferredIndex.cpp'
--- a/storage/falcon/DeferredIndex.cpp	2008-07-25 18:07:24 +0000
+++ b/storage/falcon/DeferredIndex.cpp	2008-09-10 04:02:07 +0000
@@ -840,7 +840,7 @@ void DeferredIndex::detachTransaction(vo
 	else
 		sync.unlock();
 
-	releaseRef();
+	//releaseRef();
 }
 
 void DeferredIndex::chill(Dbb *dbb)
@@ -884,8 +884,10 @@ void DeferredIndex::addRef()
 
 void DeferredIndex::releaseRef()
 {
+	ASSERT(useCount > 0);
+	
 	INTERLOCKED_DECREMENT(useCount);
 
 	if (useCount == 0)
 		delete this;
-}
\ No newline at end of file
+}

=== modified file 'storage/falcon/DeferredIndexWalker.cpp'
--- a/storage/falcon/DeferredIndexWalker.cpp	2008-05-07 22:37:18 +0000
+++ b/storage/falcon/DeferredIndexWalker.cpp	2008-09-10 04:02:07 +0000
@@ -31,6 +31,7 @@ DeferredIndexWalker::DeferredIndexWalker
 void DeferredIndexWalker::initialize(DeferredIndex *deferredIdx, IndexKey *indexKey, int
searchFlags)
 {
 	deferredIndex = deferredIdx;
+	deferredIndex->addRef();
 	currentNode = NULL;
 	DIBucket *bucket = (DIBucket*) deferredIndex->root;
 	nodePending = true;
@@ -94,6 +95,8 @@ void DeferredIndexWalker::initialize(Def
 
 DeferredIndexWalker::~DeferredIndexWalker(void)
 {
+	if (deferredIndex)
+		deferredIndex->releaseRef();
 }
 
 DINode* DeferredIndexWalker::next(void)

=== modified file 'storage/falcon/StorageTableShare.cpp'
--- a/storage/falcon/StorageTableShare.cpp	2008-09-09 23:15:17 +0000
+++ b/storage/falcon/StorageTableShare.cpp	2008-09-10 04:02:07 +0000
@@ -129,8 +129,8 @@ StorageTableShare::StorageTableShare(Sto
 	format = NULL;
 	syncObject = new SyncObject;
 	syncObject->setName("StorageTableShare::syncObject");
-	syncIndexes = new SyncObject;
-	syncIndexes->setName("StorageTableShare::syncIndexes");
+	syncIndexMap = new SyncObject;
+	syncIndexMap->setName("StorageTableShare::syncIndexMap");
 	sequence = NULL;
 	tempTable = tempTbl;
 	setPath(path);
@@ -147,7 +147,7 @@ StorageTableShare::StorageTableShare(Sto
 StorageTableShare::~StorageTableShare(void)
 {
 	delete syncObject;
-	delete syncIndexes;
+	delete syncIndexMap;
 	delete [] impure;
 	
 	if (storageDatabase)
@@ -172,12 +172,12 @@ void StorageTableShare::unlock(void)
 
 void StorageTableShare::lockIndexes(bool exclusiveLock)
 {
-	syncIndexes->lock(NULL, (exclusiveLock) ? Exclusive : Shared);
+	syncIndexMap->lock(NULL, (exclusiveLock) ? Exclusive : Shared);
 }
 
 void StorageTableShare::unlockIndexes(void)
 {
-	syncIndexes->unlock();
+	syncIndexMap->unlock();
 }
 
 int StorageTableShare::open(void)
@@ -335,7 +335,7 @@ int StorageTableShare::createIndex(Stora
 
 	// Lock out other clients before locking the table
 	
-	Sync syncIndex(syncIndexes, "StorageTableShare::createIndex(1)");
+	Sync syncIndex(syncIndexMap, "StorageTableShare::createIndex(1)");
 	syncIndex.lock(Exclusive);
 	
 	Sync syncObj(syncObject, "StorageTableShare::createIndex(2)");
@@ -389,7 +389,7 @@ int StorageTableShare::dropIndex(Storage
 
 	// Lock out other clients before locking the table
 
-	Sync syncIndex(syncIndexes, "StorageTableShare::dropIndex(1)");
+	Sync syncIndex(syncIndexMap, "StorageTableShare::dropIndex(1)");
 	syncIndex.lock(Exclusive);
 	
 	Sync syncObj(syncObject, "StorageTableShare::dropIndex(2)");
@@ -514,7 +514,7 @@ StorageIndexDesc* StorageTableShare::get
 	if (!indexes)
 		return NULL;
 	
-	Sync sync(syncIndexes, "StorageTableShare::getIndex");
+	Sync sync(syncIndexMap, "StorageTableShare::getIndex");
 	sync.lock(Shared);
 	
 	StorageIndexDesc *index = getIndex(indexId);

=== modified file 'storage/falcon/StorageTableShare.h'
--- a/storage/falcon/StorageTableShare.h	2008-09-09 23:15:17 +0000
+++ b/storage/falcon/StorageTableShare.h	2008-09-10 04:02:07 +0000
@@ -163,7 +163,7 @@ public:
 	unsigned char		*impure;
 	int					initialized;
 	SyncObject			*syncObject;
-	SyncObject			*syncIndexes;
+	SyncObject			*syncIndexMap;
 	StorageDatabase		*storageDatabase;
 	StorageHandler		*storageHandler;
 	Table				*table;

=== modified file 'storage/falcon/Transaction.cpp'
--- a/storage/falcon/Transaction.cpp	2008-09-08 21:56:03 +0000
+++ b/storage/falcon/Transaction.cpp	2008-09-10 04:02:07 +0000
@@ -85,7 +85,7 @@ Transaction::Transaction(Connection *cnc
 	useCount = 1;
 	syncObject.setName("Transaction::syncObject");
 	syncIsActive.setName("Transaction::syncActive");
-	syncIndexes.setName("Transaction::syncIndexes");
+	syncDeferredIndexes.setName("Transaction::syncDeferredIndexes");
 	syncRecords.setName("Transaction::syncRecords");
 	syncSavepoints.setName("Transaction::syncSavepoints");
 	firstRecord = NULL;
@@ -254,7 +254,7 @@ void Transaction::commit()
 
 	if (state == Active)
 		{
-		Sync sync(&syncIndexes, "Transaction::commit(1)");
+		Sync sync(&syncDeferredIndexes, "Transaction::commit(1)");
 		sync.lock(Shared);
 		
 		for (DeferredIndex *deferredIndex= deferredIndexes; deferredIndex;  
@@ -502,7 +502,7 @@ void Transaction::prepare(int xidLen, co
 	state = Limbo;
 	database->dbb->prepareTransaction(transactionId, xidLength, xid);
 
-	Sync sync(&syncIndexes, "Transaction::prepare");
+	Sync sync(&syncDeferredIndexes, "Transaction::prepare");
 	sync.lock(Shared);
 	
 	for (DeferredIndex *deferredIndex= deferredIndexes; deferredIndex;  
@@ -1314,9 +1314,10 @@ void Transaction::rollbackSavepoint(int 
 
 void Transaction::add(DeferredIndex* deferredIndex)
 {
-	Sync sync(&syncIndexes, "Transaction::add");
+	Sync sync(&syncDeferredIndexes, "Transaction::add");
 	sync.lock(Exclusive);
 
+	deferredIndex->addRef();
 	deferredIndex->nextInTransaction = deferredIndexes;
 	deferredIndexes = deferredIndex;
 	deferredIndexCount++;
@@ -1495,25 +1496,37 @@ void Transaction::printBlockage(void)
 
 void Transaction::releaseDeferredIndexes(void)
 {
-	Sync sync(&syncIndexes, "Transaction::releaseDeferredIndexes");
+	Sync sync(&syncDeferredIndexes, "Transaction::releaseDeferredIndexes");
 	sync.lock(Exclusive);
+	
 	for (DeferredIndex *deferredIndex; (deferredIndex = deferredIndexes);)
 		{
-		ASSERT(deferredIndex->transaction == this);
 		deferredIndexes = deferredIndex->nextInTransaction;
-		deferredIndex->detachTransaction();
+
+		if (deferredIndex->transaction)
+			{
+			ASSERT(deferredIndex->transaction == this);
+			deferredIndex->detachTransaction();
+			}
+
+		deferredIndex->releaseRef();
 		deferredIndexCount--;
 		}
 }
 
 void Transaction::releaseDeferredIndexes(Table* table)
 {
+	Sync sync(&syncDeferredIndexes, "Transaction::releaseDeferredIndexes(Table *)");
+	sync.lock(Exclusive);
+
 	for (DeferredIndex **ptr = &deferredIndexes, *deferredIndex; (deferredIndex =
*ptr);)
 		{
 		if (deferredIndex->index && (deferredIndex->index->table == table))
 			{
 			*ptr = deferredIndex->nextInTransaction;
-			deferredIndex->detachTransaction();
+			if (deferredIndex->transaction)
+				deferredIndex->detachTransaction();
+			deferredIndex->releaseRef();
 			--deferredIndexCount;
 			}
 		else

=== modified file 'storage/falcon/Transaction.h'
--- a/storage/falcon/Transaction.h	2008-09-03 22:17:54 +0000
+++ b/storage/falcon/Transaction.h	2008-09-10 04:02:07 +0000
@@ -170,8 +170,8 @@ public:
 	bool			pendingPageWrites;
 	bool			hasLocks;
 	SyncObject		syncObject;
-	SyncObject		syncIsActive;  // locked while transaction is active
-	SyncObject		syncIndexes;
+	SyncObject		syncIsActive;		// locked while transaction is active
+	SyncObject		syncDeferredIndexes;
 	SyncObject		syncRecords;
 	SyncObject		syncSavepoints;
 	uint64			totalRecordData;	// total bytes of record data for this transaction (unchilled
+ thawed)