From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp52.i.mail.ru (smtp52.i.mail.ru [94.100.177.112]) (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 B01C44696C3 for ; Mon, 27 Apr 2020 03:52:19 +0300 (MSK) From: Nikita Pettik Date: Mon, 27 Apr 2020 03:52:14 +0300 Message-Id: <1094177fe18b811b8b6f49f03e2ced30e798b95b.1587948306.git.korablev@tarantool.org> In-Reply-To: References: In-Reply-To: References: Subject: [Tarantool-patches] [PATCH v3 1/3] vinyl: init all vars before cleanup in vy_lsm_split_range() 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 If vy_key_from_msgpack() fails in vy_lsm_split_range(), clean-up procedure is called. However, at this moment struct vy_range *parts[2] is not initialized ergo contains garbage and access to this structure may result in crash, segfault or disk formatting. Let's move initialization of mentioned variables before call of vy_lsm_split_range(). Part of #4864 --- src/box/vy_lsm.c | 4 +- src/box/vy_stmt.c | 10 ++ src/errinj.h | 1 + test/box/errinj.result | 1 + .../gh-4864-stmt-alloc-fail-compact.result | 123 ++++++++++++++++++ .../gh-4864-stmt-alloc-fail-compact.test.lua | 55 ++++++++ test/vinyl/suite.ini | 2 +- 7 files changed, 193 insertions(+), 3 deletions(-) create mode 100644 test/vinyl/gh-4864-stmt-alloc-fail-compact.result create mode 100644 test/vinyl/gh-4864-stmt-alloc-fail-compact.test.lua diff --git a/src/box/vy_lsm.c b/src/box/vy_lsm.c index 3d3f41b7a..04c9926a8 100644 --- a/src/box/vy_lsm.c +++ b/src/box/vy_lsm.c @@ -1069,7 +1069,7 @@ vy_lsm_split_range(struct vy_lsm *lsm, struct vy_range *range) /* Split a range in two parts. */ const int n_parts = 2; - + struct vy_range *parts[2] = { NULL, NULL }; /* * Determine new ranges' boundaries. */ @@ -1088,7 +1088,7 @@ vy_lsm_split_range(struct vy_lsm *lsm, struct vy_range *range) * the old range's runs for them. */ struct vy_slice *slice, *new_slice; - struct vy_range *part, *parts[2] = {NULL, }; + struct vy_range *part = NULL; for (int i = 0; i < n_parts; i++) { part = vy_range_new(vy_log_next_id(), keys[i], keys[i + 1], lsm->cmp_def); diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c index 9b7c55516..fafe6e56f 100644 --- a/src/box/vy_stmt.c +++ b/src/box/vy_stmt.c @@ -140,6 +140,16 @@ vy_stmt_alloc(struct tuple_format *format, uint32_t bsize) error_log(diag_last_error(diag_get())); return NULL; } +#ifndef NDEBUG + struct errinj *inj = errinj(ERRINJ_VY_STMT_ALLOC, ERRINJ_INT); + if (inj != NULL && inj->iparam >= 0) { + if (inj->iparam-- == 0) { + diag_set(OutOfMemory, total_size, "malloc", + "struct vy_stmt"); + return NULL; + } + } +#endif struct tuple *tuple = malloc(total_size); if (unlikely(tuple == NULL)) { diag_set(OutOfMemory, total_size, "malloc", "struct vy_stmt"); diff --git a/src/errinj.h b/src/errinj.h index 2cb090b68..383dafcb5 100644 --- a/src/errinj.h +++ b/src/errinj.h @@ -127,6 +127,7 @@ struct errinj { _(ERRINJ_VY_COMPACTION_DELAY, ERRINJ_BOOL, {.bparam = false}) \ _(ERRINJ_DYN_MODULE_COUNT, ERRINJ_INT, {.iparam = 0}) \ _(ERRINJ_INDEX_RESERVE, ERRINJ_BOOL, {.bparam = false})\ + _(ERRINJ_VY_STMT_ALLOC, ERRINJ_INT, {.iparam = -1})\ ENUM0(errinj_id, ERRINJ_LIST); extern struct errinj errinjs[]; diff --git a/test/box/errinj.result b/test/box/errinj.result index 8090deedc..efbb4e85e 100644 --- a/test/box/errinj.result +++ b/test/box/errinj.result @@ -75,6 +75,7 @@ evals - ERRINJ_VY_RUN_WRITE_STMT_TIMEOUT: 0 - ERRINJ_VY_SCHED_TIMEOUT: 0 - ERRINJ_VY_SQUASH_TIMEOUT: 0 + - ERRINJ_VY_STMT_ALLOC: -1 - ERRINJ_VY_TASK_COMPLETE: false - ERRINJ_WAL_BREAK_LSN: -1 - ERRINJ_WAL_DELAY: false diff --git a/test/vinyl/gh-4864-stmt-alloc-fail-compact.result b/test/vinyl/gh-4864-stmt-alloc-fail-compact.result new file mode 100644 index 000000000..1afc02bef --- /dev/null +++ b/test/vinyl/gh-4864-stmt-alloc-fail-compact.result @@ -0,0 +1,123 @@ +-- test-run result file version 2 +test_run = require('test_run').new() + | --- + | ... +fiber = require('fiber') + | --- + | ... +digest = require('digest') + | --- + | ... + +s = box.schema.space.create('test', {engine = 'vinyl'}) + | --- + | ... +_ = s:create_index('pk', {run_count_per_level = 100, page_size = 128, range_size = 1024}) + | --- + | ... + +test_run:cmd("setopt delimiter ';'") + | --- + | - true + | ... +function dump(big) + local step = big and 1 or 5 + for i = 1, 20, step do + s:replace{i, digest.urandom(1000)} + end + box.snapshot() +end; + | --- + | ... + +function compact() + s.index.pk:compact() + repeat + fiber.sleep(0.001) + local info = s.index.pk:stat() + until info.range_count == info.run_count +end; + | --- + | ... +test_run:cmd("setopt delimiter ''"); + | --- + | - true + | ... + +-- The first run should be big enough to prevent major compaction +-- on the next dump, because run_count_per_level is ignored on the +-- last level. +-- +dump(true) + | --- + | ... +dump() + | --- + | ... +assert(s.index.pk:stat().range_count == 1) + | --- + | - true + | ... +assert(s.index.pk:stat().run_count == 2) + | --- + | - true + | ... + +compact() + | --- + | ... +assert(s.index.pk:stat().range_count == 1) + | --- + | - true + | ... +assert(s.index.pk:stat().run_count == 1) + | --- + | - true + | ... + +dump() + | --- + | ... +assert(s.index.pk:stat().range_count == 1) + | --- + | - true + | ... +assert(s.index.pk:stat().run_count == 2) + | --- + | - true + | ... + +errinj = box.error.injection + | --- + | ... +errinj.set('ERRINJ_VY_STMT_ALLOC', 0) + | --- + | - ok + | ... +-- Should finish successfully despite vy_stmt_alloc() failure. +-- Still split_range() fails, as a result we get one range +-- instead two. +-- +compact() + | --- + | ... +assert(s.index.pk:stat().range_count == 1) + | --- + | - true + | ... +assert(s.index.pk:stat().run_count == 1) + | --- + | - true + | ... +assert(errinj.get('ERRINJ_VY_STMT_ALLOC') == -1) + | --- + | - true + | ... +errinj.set('ERRINJ_VY_STMT_ALLOC', -1) + | --- + | - ok + | ... + +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 new file mode 100644 index 000000000..bf70bdf75 --- /dev/null +++ b/test/vinyl/gh-4864-stmt-alloc-fail-compact.test.lua @@ -0,0 +1,55 @@ +test_run = require('test_run').new() +fiber = require('fiber') +digest = require('digest') + +s = box.schema.space.create('test', {engine = 'vinyl'}) +_ = s:create_index('pk', {run_count_per_level = 100, page_size = 128, range_size = 1024}) + +test_run:cmd("setopt delimiter ';'") +function dump(big) + local step = big and 1 or 5 + for i = 1, 20, step do + s:replace{i, digest.urandom(1000)} + end + box.snapshot() +end; + +function compact() + s.index.pk:compact() + repeat + fiber.sleep(0.001) + local info = s.index.pk:stat() + until info.range_count == info.run_count +end; +test_run:cmd("setopt delimiter ''"); + +-- The first run should be big enough to prevent major compaction +-- on the next dump, because run_count_per_level is ignored on the +-- last level. +-- +dump(true) +dump() +assert(s.index.pk:stat().range_count == 1) +assert(s.index.pk:stat().run_count == 2) + +compact() +assert(s.index.pk:stat().range_count == 1) +assert(s.index.pk:stat().run_count == 1) + +dump() +assert(s.index.pk:stat().range_count == 1) +assert(s.index.pk:stat().run_count == 2) + +errinj = box.error.injection +errinj.set('ERRINJ_VY_STMT_ALLOC', 0) +-- Should finish successfully despite vy_stmt_alloc() failure. +-- Still split_range() fails, as a result we get one range +-- instead two. +-- +compact() +assert(s.index.pk:stat().range_count == 1) +assert(s.index.pk:stat().run_count == 1) +assert(errinj.get('ERRINJ_VY_STMT_ALLOC') == -1) +errinj.set('ERRINJ_VY_STMT_ALLOC', -1) + +s:drop() diff --git a/test/vinyl/suite.ini b/test/vinyl/suite.ini index 1417d7156..ed602bb64 100644 --- a/test/vinyl/suite.ini +++ b/test/vinyl/suite.ini @@ -2,7 +2,7 @@ core = tarantool description = vinyl integration tests script = vinyl.lua -release_disabled = errinj.test.lua errinj_ddl.test.lua errinj_gc.test.lua errinj_stat.test.lua errinj_tx.test.lua errinj_vylog.test.lua partial_dump.test.lua quota_timeout.test.lua recovery_quota.test.lua replica_rejoin.test.lua +release_disabled = errinj.test.lua errinj_ddl.test.lua errinj_gc.test.lua errinj_stat.test.lua errinj_tx.test.lua errinj_vylog.test.lua partial_dump.test.lua quota_timeout.test.lua recovery_quota.test.lua replica_rejoin.test.lua gh-4864-stmt-alloc-fail-compact.test.lua config = suite.cfg lua_libs = suite.lua stress.lua large.lua txn_proxy.lua ../box/lua/utils.lua use_unix_sockets = True -- 2.17.1