#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)
