From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <alexander.turenko@tarantool.org>
Received: from smtp15.mail.ru (smtp15.mail.ru [94.100.176.133])
 (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 01A7D40F3AE
 for <tarantool-patches@dev.tarantool.org>;
 Wed, 17 Jun 2020 20:54:24 +0300 (MSK)
Date: Wed, 17 Jun 2020 20:53:39 +0300
From: Alexander Turenko <alexander.turenko@tarantool.org>
Message-ID: <20200617175339.aieq23vkcdqu4z7u@tkn_work_nb>
References: <cover.1591028838.git.alexander.turenko@tarantool.org>
 <0f0ad73e2fce564e22dcc8f9970d8aedd028c279.1591028838.git.alexander.turenko@tarantool.org>
 <920c35d4-150b-bae5-345f-e819ada02367@tarantool.org>
 <20200607165816.4r5s7ah4pa5uihyl@tkn_work_nb>
 <c00b3856-7938-5d8b-e51a-279f1def539d@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <c00b3856-7938-5d8b-e51a-279f1def539d@tarantool.org>
Subject: Re: [Tarantool-patches] [PATCH 2/3] merger: fix NULL dereference
 when called via iproto
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: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org

> >     Usage of the fiber-local Lua state is not quite correct now: merge
> >     source code may left garbage on a stack in case of failures (like
> 
> 1. merge -> merger.

I use term 'merge source' for sources, we agreed on it with Vladimir D.,
when the merger was designed. Removed 'code' to don't confuse a reader:
'merge source code' -> 'a merge source'.

> > +	struct lua_State *L = luaT_newthread(tarantool_L);
> > +	if (L == NULL)
> > +		return NULL;
> 
> 2. luaT_newthread() does not set a diag. That may lead to a crash,
> because as far as I see, this function may be called
> lbox_merge_source_gen() indirectly, somewhere deep in the callstack.
> And it luaT_error(), when merge_source_next() fails.

As I see, it is not so.

luaT_newthread_wrapper() may raise a Lua error (only 'not enough memory'
I guess), luaT_cpcall() calls luaT_toerror() in this case, which invokes
diag_set(LuajitError, <...>).

Isn't I miss something?