From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp44.i.mail.ru (smtp44.i.mail.ru [94.100.177.104]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id AE4DA4696C3 for ; Thu, 9 Apr 2020 00:37:11 +0300 (MSK) From: Nikita Pettik Date: Thu, 9 Apr 2020 00:37:08 +0300 Message-Id: <73e1f0baf18ec008312d91db4449447b3c06aa86.1586381297.git.korablev@tarantool.org> In-Reply-To: References: In-Reply-To: References: Subject: [Tarantool-patches] [PATCH 2/2] vinyl: clean-up read views if *_build_history() fails List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org Cc: v.shpilevoy@tarantool.org vy_write_iterator->read_views[i].history objects are allocated on region (see vy_write_iterator_push_rv()) during building history of the given key. However, in case of fail of vy_write_iterator_build_history() region is truncated but pointers to vy_write_history objects are not nullified. As a result, they may be accessed (for instance while finalizing write_iterator object in vy_write_iterator_stop) which in turn may lead to crash, segfaul or disk formatting. Let's nullify those objects right after function returns with rc != 0. Closes #4864 --- src/box/vy_write_iterator.c | 5 +- .../gh-4864-stmt-alloc-fail-compact.result | 51 +++++++++++++++++++ .../gh-4864-stmt-alloc-fail-compact.test.lua | 24 +++++++++ 3 files changed, 79 insertions(+), 1 deletion(-) diff --git a/src/box/vy_write_iterator.c b/src/box/vy_write_iterator.c index 7a6a20627..f6e6ed4d2 100644 --- a/src/box/vy_write_iterator.c +++ b/src/box/vy_write_iterator.c @@ -961,8 +961,11 @@ vy_write_iterator_build_read_views(struct vy_write_iterator *stream, int *count) size_t used = region_used(region); stream->rv_used_count = 0; if (vy_write_iterator_build_history(stream, &raw_count, - &is_first_insert) != 0) + &is_first_insert) != 0) { + for (int i = 0; i < stream->rv_count; ++i) + stream->read_views[i].history = NULL; goto error; + } if (raw_count == 0) { /* A key is fully optimized. */ region_truncate(region, used); diff --git a/test/vinyl/gh-4864-stmt-alloc-fail-compact.result b/test/vinyl/gh-4864-stmt-alloc-fail-compact.result index 2c03697f6..770efcca8 100644 --- a/test/vinyl/gh-4864-stmt-alloc-fail-compact.result +++ b/test/vinyl/gh-4864-stmt-alloc-fail-compact.result @@ -91,3 +91,54 @@ s.index.pk:stat().run_count s:drop() | --- | ... + +-- All the same except for delayed vy_stmt_alloc() fail. +-- Re-create space for the sake of test purity. +-- +s = box.schema.space.create('test', {engine = 'vinyl'}) + | --- + | ... +_ = s:create_index('pk', {run_count_per_level = 100, page_size = 128, range_size = 1024}) + | --- + | ... + +dump(true) + | --- + | ... +dump() + | --- + | ... + +compact() + | --- + | ... + +dump() + | --- + | ... + +errinj = box.error.injection + | --- + | ... +errinj.set('ERRINJ_VY_MAX_TUPLE_SIZE', 0) + | --- + | - ok + | ... +-- Should finish successfully despite vy_stmt_alloc() fail. +-- +compact() + | --- + | ... +-- 1 range, 1 run +s.index.pk:stat().range_count + | --- + | - 1 + | ... +s.index.pk:stat().run_count + | --- + | - 1 + | ... + +s:drop() + | --- + | ... diff --git a/test/vinyl/gh-4864-stmt-alloc-fail-compact.test.lua b/test/vinyl/gh-4864-stmt-alloc-fail-compact.test.lua index 53a050c9b..8b5c79025 100644 --- a/test/vinyl/gh-4864-stmt-alloc-fail-compact.test.lua +++ b/test/vinyl/gh-4864-stmt-alloc-fail-compact.test.lua @@ -47,3 +47,27 @@ s.index.pk:stat().range_count s.index.pk:stat().run_count s:drop() + +-- All the same except for delayed vy_stmt_alloc() fail. +-- Re-create space for the sake of test purity. +-- +s = box.schema.space.create('test', {engine = 'vinyl'}) +_ = s:create_index('pk', {run_count_per_level = 100, page_size = 128, range_size = 1024}) + +dump(true) +dump() + +compact() + +dump() + +errinj = box.error.injection +errinj.set('ERRINJ_VY_MAX_TUPLE_SIZE', 0) +-- Should finish successfully despite vy_stmt_alloc() fail. +-- +compact() +-- 1 range, 1 run +s.index.pk:stat().range_count +s.index.pk:stat().run_count + +s:drop() -- 2.17.1