Skip to content

Commit 46cc98b

Browse files
nshysergepetrenko
authored andcommitted
salad: fix crash on insertion into bps tree with OOM conditions
On insertion into bps tree we first reserve blocks for the case we need to insert new leaf during rebalancing. Next there can be a free block on last extent in matras but no more free extents in tuple arena. So `bps_tree_reserve_blocks()` is able to allocate a block but then `bps_tree_garbage_push()` is called that touches a block. As there no free extents touch is failed which is unexpected and causes a crash. We don't need to touch the block on this path as block is newly allocated and is not shared with any read view. We don't need to touch block on other paths too (disposing a leaf or a inner node) because we move data from this node before so it is already should be touched. So just don't touch the block in `bps_tree_garbage_push()`. The issue is introduced in the commit 51c56d9 ("salad: reserve extents for matras_touch on BPS tree operations") and unlocked in the commit 32b0678 ("memtx: reserve extents in the RTREE index itself") where we removed crutches where we heuristically reserve memory for index operations. Closes tarantool#11326 NO_DOC=bugfix
1 parent 061d4bb commit 46cc98b

File tree

3 files changed

+47
-4
lines changed

3 files changed

+47
-4
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
## bugfix/memtx
2+
3+
* Fixed a crash on OOM on insertion in tree index of memtx engine (gh-11326).

src/lib/salad/bps_tree.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3156,7 +3156,6 @@ static inline void
31563156
bps_tree_garbage_push(struct bps_tree_common *tree, struct bps_block *block,
31573157
bps_tree_block_id_t id)
31583158
{
3159-
assert(block); (void) block;
31603159
bps_tree_block_id_t next_leaf_id = (bps_tree_block_id_t)(-1);
31613160
bps_tree_block_id_t prev_leaf_id = (bps_tree_block_id_t)(-1);
31623161
if (block->type == BPS_TREE_BT_LEAF) {
@@ -3165,8 +3164,7 @@ bps_tree_garbage_push(struct bps_tree_common *tree, struct bps_block *block,
31653164
prev_leaf_id = leaf->prev_id;
31663165
}
31673166

3168-
struct bps_garbage *garbage = (struct bps_garbage *)
3169-
bps_tree_touch_block(tree, id);
3167+
struct bps_garbage *garbage = (struct bps_garbage *)block;
31703168
garbage->header.type = BPS_TREE_BT_GARBAGE;
31713169
garbage->next_id = tree->garbage_head_id;
31723170
garbage->next_leaf_id = next_leaf_id;

test/unit/bps_tree.cc

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,11 +174,14 @@ compare(type_t a, type_t b)
174174
return a < b ? -1 : a > b ? 1 : 0;
175175
}
176176
static int extents_count = 0;
177+
static bool extent_alloc_failure = false;
177178

178179
static void *
179180
extent_alloc(struct matras_allocator *allocator)
180181
{
181182
(void)allocator;
183+
if (extent_alloc_failure)
184+
return NULL;
182185
++extents_count;
183186
return xmalloc(BPS_TREE_EXTENT_SIZE);
184187
}
@@ -920,10 +923,44 @@ insert_successor_test()
920923
ok(true, "successor test");
921924
}
922925

926+
static void
927+
gh_11326_oom_on_insertion_test()
928+
{
929+
plan(1);
930+
header();
931+
932+
test tree;
933+
test_view view;
934+
type_t replaced;
935+
struct test_iterator iterator;
936+
937+
test_create(&tree, 0, &allocator, NULL);
938+
test_insert(&tree, 0, &replaced, NULL);
939+
test_view_create(&view, &tree);
940+
941+
extent_alloc_failure = true;
942+
fail_unless(test_insert(&tree, 1, &replaced, NULL) != 0);
943+
debug_check(&tree);
944+
fail_unless(test_size(&tree) == 1);
945+
iterator = test_first(&tree);
946+
type_t *v = test_iterator_get_elem(&tree, &iterator);
947+
fail_unless(v != NULL && *v == 0);
948+
fail_unless(test_iterator_next(&tree, &iterator) == false);
949+
extent_alloc_failure = false;
950+
951+
test_view_destroy(&view);
952+
test_destroy(&tree);
953+
954+
ok(true, "gh-11326: OOM on insertion test");
955+
956+
footer();
957+
check_plan();
958+
}
959+
923960
int
924961
main(void)
925962
{
926-
plan(12);
963+
plan(13);
927964
header();
928965

929966
matras_allocator_create(&allocator, BPS_TREE_EXTENT_SIZE,
@@ -944,6 +981,11 @@ main(void)
944981

945982
matras_allocator_destroy(&allocator);
946983

984+
matras_allocator_create(&allocator, BPS_TREE_EXTENT_SIZE,
985+
extent_alloc, extent_free);
986+
gh_11326_oom_on_insertion_test();
987+
matras_allocator_destroy(&allocator);
988+
947989
footer();
948990
return check_plan();
949991
}

0 commit comments

Comments
 (0)