[haiku-commits] haiku: hrev44610 - in src: tests/system/kernel/cache system/kernel/cache

  • From: axeld@xxxxxxxxxxxxxxxx
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Tue, 4 Sep 2012 01:00:50 +0200 (CEST)

hrev44610 adds 1 changeset to branch 'master'
old head: e85dfa6bad926733b5a1b1fc3355b2c314aa041e
new head: 098967cee129b9116b468224257234c8ef1c9707

----------------------------------------------------------------------------

098967c: 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.

                                   [ Axel Dörfler <axeld@xxxxxxxxxxxxxxxx> ]

----------------------------------------------------------------------------

Revision:    hrev44610
Commit:      098967cee129b9116b468224257234c8ef1c9707
URL:         http://cgit.haiku-os.org/haiku/commit/?id=098967c
Author:      Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>
Date:        Mon Sep  3 22:54:25 2012 UTC

Ticket:      https://dev.haiku-os.org/ticket/8910

----------------------------------------------------------------------------

2 files changed, 45 insertions(+), 4 deletions(-)
src/system/kernel/cache/block_cache.cpp            |    8 +--
src/tests/system/kernel/cache/block_cache_test.cpp |   41 +++++++++++++++-

----------------------------------------------------------------------------

diff --git a/src/system/kernel/cache/block_cache.cpp 
b/src/system/kernel/cache/block_cache.cpp
index 7cd0157..22e20cc 100644
--- a/src/system/kernel/cache/block_cache.cpp
+++ b/src/system/kernel/cache/block_cache.cpp
@@ -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;
diff --git a/src/tests/system/kernel/cache/block_cache_test.cpp 
b/src/tests/system/kernel/cache/block_cache_test.cpp
index f03f37b..d0b88f1 100644
--- a/src/tests/system/kernel/cache/block_cache_test.cpp
+++ b/src/tests/system/kernel/cache/block_cache_test.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright 2008, Axel Dörfler, axeld@xxxxxxxxxxxxxxxxx
+ * Copyright 2008-2012, Axel Dörfler, axeld@xxxxxxxxxxxxxxxxx
  * 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);
 }
 
 


Other related posts: