From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <alyapunov@tarantool.org>
Received: from smtp3.mail.ru (smtp3.mail.ru [94.100.179.58])
 (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 311ED430407
 for <tarantool-patches@dev.tarantool.org>;
 Tue, 25 Aug 2020 18:23:05 +0300 (MSK)
References: <20200819172324.6188-1-i.kosarev@tarantool.org>
From: Aleksandr Lyapunov <alyapunov@tarantool.org>
Message-ID: <fe6eab48-93a9-afeb-1798-6b58d4aa330e@tarantool.org>
Date: Tue, 25 Aug 2020 18:23:03 +0300
MIME-Version: 1.0
In-Reply-To: <20200819172324.6188-1-i.kosarev@tarantool.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Subject: Re: [Tarantool-patches] [PATCH] test: concurrent tuple update
 segfault on bitset index iteration
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Ilya Kosarev <i.kosarev@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org

Hi, thanks for the patch, LGTM.

On 8/19/20 8:23 PM, Ilya Kosarev wrote:
> Concurrent tuple update could segfault on BITSET_ALL_NOT_SET iterator
> usage. Fixed in 850054b2dbca257076c3f7c22e00564ac55b70d5.
>
> Closes #1088
> ---
> Branch: https://github.com/tarantool/tarantool/tree/i.kosarev/gh-1088-concurrent-tuple-update-segfault-with-bitset-index
> Issue: https://github.com/tarantool/tarantool/issues/1088
>
>   test/box/bitset.result   | 86 ++++++++++++++++++++++++++++++++++++++++
>   test/box/bitset.test.lua | 42 ++++++++++++++++++++
>   2 files changed, 128 insertions(+)
>
> diff --git a/test/box/bitset.result b/test/box/bitset.result
> index bf44773ef3..5da068385c 100644
> --- a/test/box/bitset.result
> +++ b/test/box/bitset.result
> @@ -2020,3 +2020,89 @@ s:drop()
>   box.schema.func.drop('s')
>   ---
>   ...
> +-- gh-1088 concurrent tuple update segfaults on BITSET_ALL_NOT_SET iteration
> +test_run = require('test_run').new()
> +---
> +...
> +fiber = require('fiber')
> +---
> +...
> +s = box.schema.space.create('gh-1088')
> +---
> +...
> +_ = s:create_index('primary', {type = 'hash', parts = {1, 'num'}})
> +---
> +...
> +_ = s:create_index('bitset', {unique = false, type = 'BITSET', parts = {2, 'num'}})
> +---
> +...
> +for i = 1, 100 do s:insert{i, 0, i - 1} end
> +---
> +...
> +counter = 0
> +---
> +...
> +test_run:cmd("setopt delimiter ';'")
> +---
> +- true
> +...
> +function update()
> +    for _, t in s.index.bitset:pairs(1, {iterator = box.index.BITS_ALL_NOT_SET}) do
> +        counter = counter + 1
> +        s:update(t[1], {{'+', 3, 11}})
> +        fiber.sleep(0)
> +    end
> +    fiber.self():cancel()
> +end;
> +---
> +...
> +test_run:cmd("setopt delimiter ''");
> +---
> +- true
> +...
> +fibers = {}
> +---
> +...
> +for _ = 1, 100 do table.insert(fibers, fiber.create(update)) end
> +---
> +...
> +updating = true
> +---
> +...
> +test_run:cmd("setopt delimiter ';'")
> +---
> +- true
> +...
> +while updating do
> +    updating = false
> +    for _, f in pairs(fibers) do
> +        if f:status() ~= 'dead' then updating = true end
> +    end
> +    fiber.sleep(0.001)
> +end;
> +---
> +...
> +test_run:cmd("setopt delimiter ''");
> +---
> +- true
> +...
> +s:get(1)
> +---
> +- [1, 0, 1100]
> +...
> +s:get(2)
> +---
> +- [2, 0, 1101]
> +...
> +s:get(3)
> +---
> +- [3, 0, 1102]
> +...
> +s:get(4)
> +---
> +- [4, 0, 1103]
> +...
> +counter -- total updates counter
> +---
> +- 10000
> +...
> diff --git a/test/box/bitset.test.lua b/test/box/bitset.test.lua
> index d644d34e0b..dd432edeb0 100644
> --- a/test/box/bitset.test.lua
> +++ b/test/box/bitset.test.lua
> @@ -162,3 +162,45 @@ _ = s:create_index('pk')
>   _ = s:create_index('idx', {type = 'bitset', func = box.func.s.id, parts = {{1, 'unsigned'}}})
>   s:drop()
>   box.schema.func.drop('s')
> +
> +-- gh-1088 concurrent tuple update segfaults on BITSET_ALL_NOT_SET iteration
> +test_run = require('test_run').new()
> +fiber = require('fiber')
> +
> +s = box.schema.space.create('gh-1088')
> +_ = s:create_index('primary', {type = 'hash', parts = {1, 'num'}})
> +_ = s:create_index('bitset', {unique = false, type = 'BITSET', parts = {2, 'num'}})
> +for i = 1, 100 do s:insert{i, 0, i - 1} end
> +
> +counter = 0
> +test_run:cmd("setopt delimiter ';'")
> +function update()
> +    for _, t in s.index.bitset:pairs(1, {iterator = box.index.BITS_ALL_NOT_SET}) do
> +        counter = counter + 1
> +        s:update(t[1], {{'+', 3, 11}})
> +        fiber.sleep(0)
> +    end
> +    fiber.self():cancel()
> +end;
> +test_run:cmd("setopt delimiter ''");
> +
> +fibers = {}
> +for _ = 1, 100 do table.insert(fibers, fiber.create(update)) end
> +
> +updating = true
> +test_run:cmd("setopt delimiter ';'")
> +while updating do
> +    updating = false
> +    for _, f in pairs(fibers) do
> +        if f:status() ~= 'dead' then updating = true end
> +    end
> +    fiber.sleep(0.001)
> +end;
> +test_run:cmd("setopt delimiter ''");
> +
> +s:get(1)
> +s:get(2)
> +s:get(3)
> +s:get(4)
> +
> +counter -- total updates counter