From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 7322D25274 for ; Fri, 11 May 2018 19:24:45 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id PmYwa7a-ZAYK for ; Fri, 11 May 2018 19:24:45 -0400 (EDT) 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 turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id B51D625202 for ; Fri, 11 May 2018 19:24:44 -0400 (EDT) From: Vladislav Shpilevoy Subject: [tarantool-patches] [PATCH 1/1] vinyl: fix crash in vinyl_iterator_secondary_next Date: Sat, 12 May 2018 02:24:41 +0300 Message-Id: Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: kostja@tarantool.org Vinyl_iterator_secondary_next is a method to iterate over a secondary index. It gets a tuple from the secondary index and looks up its full version in the primary index. But during lookup the tuple can be deleted from the former, and the iterator must take it in the hopper. Closes #3393 --- Branch: https://github.com/tarantool/tarantool/tree/gh-3393-vinyl-iterator-crash Issue: https://github.com/tarantool/tarantool/issues/3393 src/box/vinyl.c | 8 ++++- test/vinyl/errinj.result | 73 ++++++++++++++++++++++++++++++++++++++++++++++ test/vinyl/errinj.test.lua | 26 +++++++++++++++++ 3 files changed, 106 insertions(+), 1 deletion(-) diff --git a/src/box/vinyl.c b/src/box/vinyl.c index ff4c28314..460b9b5e7 100644 --- a/src/box/vinyl.c +++ b/src/box/vinyl.c @@ -3853,7 +3853,7 @@ vinyl_iterator_secondary_next(struct iterator *base, struct tuple **ret) goto fail; } - +next_tuple: if (vy_read_iterator_next(&it->iterator, &tuple) != 0) goto fail; @@ -3879,6 +3879,12 @@ vinyl_iterator_secondary_next(struct iterator *base, struct tuple **ret) if (vy_point_lookup(it->lsm->pk, it->tx, vy_tx_read_view(it->tx), tuple, &tuple) != 0) goto fail; + /* + * The tuple can be removed by the same transaction during + * primary index lookup. + */ + if (tuple == NULL) + goto next_tuple; *ret = tuple_bless(tuple); tuple_unref(tuple); if (*ret != NULL) diff --git a/test/vinyl/errinj.result b/test/vinyl/errinj.result index fd21f7bb4..b63d01c18 100644 --- a/test/vinyl/errinj.result +++ b/test/vinyl/errinj.result @@ -1395,3 +1395,76 @@ s:count() -- 200 s:drop() --- ... +-- +-- gh-3393: vinyl secondary index iterator must take in the hopper +-- that a tuple can disappear from the primary index after the +-- secondary index lookup. +-- +s = box.schema.create_space('test', {engine = 'vinyl'}) +--- +... +pk = s:create_index('pk') +--- +... +sk = s:create_index('sk', {parts = {{2, 'unsigned'}, {1, 'unsigned'}}}) +--- +... +s:replace{1, 2} +--- +- [1, 2] +... +s:replace{3, 4} +--- +- [3, 4] +... +box.snapshot() +--- +- ok +... +ret = nil +--- +... +function iterate(i) ret = i.next() end +--- +... +box.begin() +--- +... +s:replace{5, 6} -- Start a transaction in the engine. +--- +- [5, 6] +... +iter = create_iterator(sk, {4}, {iterator = 'GE'}) +--- +... +errinj.set("ERRINJ_VY_DELAY_PK_LOOKUP", true) +--- +- ok +... +f = fiber.create(iterate, iter) +--- +... +s:delete{3} +--- +... +errinj.set("ERRINJ_VY_DELAY_PK_LOOKUP", false) +--- +- ok +... +while not ret do fiber.sleep(0.1) end +--- +... +ret +--- +- [5, 6] +... +iter.iterate_over() +--- +- [] +... +box.commit() +--- +... +s:drop() +--- +... diff --git a/test/vinyl/errinj.test.lua b/test/vinyl/errinj.test.lua index 64d04c62d..c551de1b3 100644 --- a/test/vinyl/errinj.test.lua +++ b/test/vinyl/errinj.test.lua @@ -548,3 +548,29 @@ ch:get() s:count() -- 200 s:drop() + +-- +-- gh-3393: vinyl secondary index iterator must take in the hopper +-- that a tuple can disappear from the primary index after the +-- secondary index lookup. +-- +s = box.schema.create_space('test', {engine = 'vinyl'}) +pk = s:create_index('pk') +sk = s:create_index('sk', {parts = {{2, 'unsigned'}, {1, 'unsigned'}}}) +s:replace{1, 2} +s:replace{3, 4} +box.snapshot() +ret = nil +function iterate(i) ret = i.next() end +box.begin() +s:replace{5, 6} -- Start a transaction in the engine. +iter = create_iterator(sk, {4}, {iterator = 'GE'}) +errinj.set("ERRINJ_VY_DELAY_PK_LOOKUP", true) +f = fiber.create(iterate, iter) +s:delete{3} +errinj.set("ERRINJ_VY_DELAY_PK_LOOKUP", false) +while not ret do fiber.sleep(0.1) end +ret +iter.iterate_over() +box.commit() +s:drop() -- 2.15.1 (Apple Git-101)