Fixed the new issue in #8910 from r44585.
* The ASSERT() I introduced in r44585 was incorrect: when the sub transaction used block_cache_get_empty() to get the block, there is no original_data for a reason. * Added a test case that reproduces this situation. * The block must be moved to the unused list in this situation, though, or else it might contain invalid data. Since the block can only be allocated in the current transaction, this should not be a problem, though, AFAICT.
This commit is contained in:
parent
e85dfa6bad
commit
098967cee1
8
src/system/kernel/cache/block_cache.cpp
vendored
8
src/system/kernel/cache/block_cache.cpp
vendored
@ -3046,9 +3046,11 @@ cache_abort_sub_transaction(void* _cache, int32 id)
|
||||
// The parent transaction didn't change the block, but the sub
|
||||
// transaction did - we need to revert to the original data.
|
||||
// The block is no longer part of the transaction
|
||||
ASSERT(block->original_data != NULL);
|
||||
memcpy(block->current_data, block->original_data,
|
||||
cache->block_size);
|
||||
if (block->original_data != NULL) {
|
||||
// The block might not have original data if was empty
|
||||
memcpy(block->current_data, block->original_data,
|
||||
cache->block_size);
|
||||
}
|
||||
|
||||
if (last != NULL)
|
||||
last->transaction_next = next;
|
||||
|
@ -1,5 +1,5 @@
|
||||
/*
|
||||
* Copyright 2008, Axel Dörfler, axeld@pinc-software.de.
|
||||
* Copyright 2008-2012, Axel Dörfler, axeld@pinc-software.de.
|
||||
* Distributed under the terms of the MIT License.
|
||||
*/
|
||||
|
||||
@ -400,6 +400,45 @@ test_abort_sub_transaction()
|
||||
|
||||
cache_end_transaction(gCache, id, NULL, NULL);
|
||||
cache_sync_transaction(gCache, id);
|
||||
|
||||
start_test("Abort sub with empty block");
|
||||
id = cache_start_transaction(gCache);
|
||||
|
||||
gBlocks[1].present = true;
|
||||
gBlocks[1].read = true;
|
||||
|
||||
block = block_cache_get_writable(gCache, 1, id);
|
||||
or_block(block, BLOCK_CHANGED_IN_PREVIOUS);
|
||||
gBlocks[1].original = gBlocks[1].current;
|
||||
gBlocks[1].current |= BLOCK_CHANGED_IN_PREVIOUS;
|
||||
|
||||
block_cache_put(gCache, 1);
|
||||
|
||||
gBlocks[1].is_dirty = true;
|
||||
TEST_BLOCKS(1, 1);
|
||||
|
||||
cache_start_sub_transaction(gCache, id);
|
||||
|
||||
gBlocks[0].present = true;
|
||||
|
||||
block = block_cache_get_empty(gCache, 0, id);
|
||||
or_block(block, BLOCK_CHANGED_IN_SUB);
|
||||
gBlocks[0].current = BLOCK_CHANGED_IN_SUB;
|
||||
|
||||
block_cache_put(gCache, 0);
|
||||
|
||||
cache_abort_sub_transaction(gCache, id);
|
||||
|
||||
gBlocks[0].write = false;
|
||||
gBlocks[0].is_dirty = false;
|
||||
gBlocks[0].parent = 0;
|
||||
gBlocks[0].original = 0;
|
||||
TEST_BLOCKS(0, 1);
|
||||
|
||||
gBlocks[1].write = true;
|
||||
gBlocks[1].is_dirty = false;
|
||||
cache_end_transaction(gCache, id, NULL, NULL);
|
||||
cache_sync_transaction(gCache, id);
|
||||
}
|
||||
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user