List:Commits« Previous MessageNext Message »
From:lars-erik.bjork Date:March 10 2009 8:33am
Subject:bzr commit into mysql-6.0-falcon-team branch (lars-erik.bjork:3059) Bug#43488
View as plain text  
#At file:///home/lb200670/mysql/bugfest/ based on revid:christopher.powers@stripped

 3059 lars-erik.bjork@stripped	2009-03-10
      This is a patch for Bug #43488
      Crash in Table::fetchForUpdate (update conflict) when using LIMIT query
      
      This crash was due to missing exception handling when calling fetchForUpdate
      when fetching records through the IndexWalker. The exception handling
      introduced in this patch is split in two parts. The first part does the 
      necessary cleanup in IndexWalker::getValidatedRecord and re-throws the 
      exception. The exception is re-caught in StorageDatabase::nextIndexed, 
      where we return the correct error.
      
      I have not included any test files for this patch, because it is not
      easily reproducible without a larger framework as described in the bug
      report.
      
      Modified file 'storage/falcon/IndexWalker.cpp'
      ----------------------------------------------
      Added try - catch logic, doing the necessary cleanup in the case
      of an exception. This implementation is based on the similar cases
      in StorageDatabase.cpp. After the cleanup has been done, we re-throw
      the exception, so that we can return the appropriate error in 
      StorageDatabase::nextIndexed
       
      Modified file 'storage/falcon/StorageDatabase.cpp'
      --------------------------------------------------
      Recatch the exception and return the appropriate error.
      modified:
        storage/falcon/IndexWalker.cpp
        storage/falcon/StorageDatabase.cpp

=== modified file 'storage/falcon/IndexWalker.cpp'
--- a/storage/falcon/IndexWalker.cpp	2009-03-07 07:51:29 +0000
+++ b/storage/falcon/IndexWalker.cpp	2009-03-10 08:33:34 +0000
@@ -142,31 +142,51 @@ Record* IndexWalker::getValidatedRecord(
 
 	// Fetch record.  If it doesn't exist, that's ok.
 
-	Record *candidate = table->fetch(recordId);
-	if (!candidate)
-		return NULL;
-	RECORD_HISTORY(candidate);
+	Record *candidate = NULL;
+	Record *record = NULL;
+
+	try
+		{
+		candidate = table->fetch(recordId);
 
-	// Get the correct version.  If this is select for update, get a lock record
+		if (!candidate)
+			return NULL;
 
-	Record *record = (lockForUpdate) 
-				    ? table->fetchForUpdate(transaction, candidate, true)
-				    : candidate->fetchVersion(transaction);
+		RECORD_HISTORY(candidate);
+
+		// Get the correct version.  If this is select for update, get a lock record
+		record = (lockForUpdate) 
+			? table->fetchForUpdate(transaction, candidate, true)
+			: candidate->fetchVersion(transaction);
 	
-	if (!record)
-		{
-		if (!lockForUpdate)
-			candidate->release(REC_HISTORY);
+		if (!record)
+			{
+			if (!lockForUpdate)
+				candidate->release(REC_HISTORY);
 		
-		return NULL;
-		}
+			return NULL;
+			}
 	
-	// If we have a different record version, release the original
+		// If we have a different record version, release the original
 	
-	if (!lockForUpdate && candidate != record)
+		if (!lockForUpdate && candidate != record)
+			{
+			record->addRef(REC_HISTORY);
+			candidate->release(REC_HISTORY);
+			}
+		}
+	catch (SQLException& exception)
 		{
-		record->addRef(REC_HISTORY);
-		candidate->release(REC_HISTORY);
+
+		if (record && record != candidate)
+			record->release(REC_HISTORY);
+
+		if (candidate && !lockForUpdate)
+			candidate->release(REC_HISTORY);
+
+		// Re-throw the exception, catch it further up to return the correct
+		// error
+		throw;
 		}
 	
 	// Compute record key and compare against index key.  If there' different, punt

=== modified file 'storage/falcon/StorageDatabase.cpp'
--- a/storage/falcon/StorageDatabase.cpp	2009-02-26 20:04:31 +0000
+++ b/storage/falcon/StorageDatabase.cpp	2009-03-10 08:33:34 +0000
@@ -484,14 +484,45 @@ int StorageDatabase::nextIndexed(Storage
 
 int StorageDatabase::nextIndexed(StorageTable* storageTable, IndexWalker* indexWalker, bool lockForUpdate)
 {
-	Record *record = indexWalker->getNext(lockForUpdate);
 
-	if (!record)
-		return StorageErrorRecordNotFound;
+	try
+		{
+		Record *record = indexWalker->getNext(lockForUpdate);
+
+		if (!record)
+			return StorageErrorRecordNotFound;
+
+		storageTable->setRecord(record, lockForUpdate);
+		return record->recordNumber;
+		}
+	catch (SQLException& exception)
+		{
+		StorageConnection *storageConnection = storageTable->storageConnection;
+		storageConnection->setErrorText(&exception);
+		int errorCode = exception.getSqlcode();
 		
-	storageTable->setRecord(record, lockForUpdate);
-	
-	return record->recordNumber;
+		switch (errorCode)
+			{
+			case UPDATE_CONFLICT:
+				return StorageErrorUpdateConflict;
+
+			case OUT_OF_MEMORY_ERROR:
+				return StorageErrorOutOfMemory;
+
+			case OUT_OF_RECORD_MEMORY_ERROR:
+				return StorageErrorOutOfRecordMemory;
+
+			case DEADLOCK:
+				return StorageErrorDeadlock;
+
+			case LOCK_TIMEOUT:
+				return StorageErrorLockTimeout;
+				
+			default:
+				ASSERT(false);
+			}
+
+		}
 }
 
 int StorageDatabase::savepointSet(Connection* connection)

Thread
bzr commit into mysql-6.0-falcon-team branch (lars-erik.bjork:3059) Bug#43488lars-erik.bjork10 Mar
  • Re: bzr commit into mysql-6.0-falcon-team branch(lars-erik.bjork:3059) Bug#43488Kevin Lewis11 Mar
    • Re: bzr commit into mysql-6.0-falcon-team branch(lars-erik.bjork:3059) Bug#43488Lars-Erik Bjørk13 Mar
      • Re: bzr commit into mysql-6.0-falcon-team branch(lars-erik.bjork:3059) Bug#43488Kevin Lewis13 Mar
        • Re: bzr commit into mysql-6.0-falcon-team branch(lars-erik.bjork:3059) Bug#43488Lars-Erik Bjørk15 Mar