[tarantool-patches] [PATCH 1/1] vinyl: fix crash in vinyl_iterator_secondary_next

[Vladislav Shpilevoy]

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)