From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 92BB2445320 for ; Fri, 17 Jul 2020 01:55:09 +0300 (MSK) Date: Fri, 17 Jul 2020 01:44:51 +0300 From: Igor Munkin Message-ID: <20200716224451.GD18920@tarantool.org> References: <20200701203633.GA5559@tarantool.org> <20200716201057.67ity4dkaozagvjd@tkn_work_nb> <20200716214249.GA18920@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20200716214249.GA18920@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v2 1/3] merger: fix NULL dereference when called via iproto List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Turenko Cc: tarantool-patches@dev.tarantool.org, Vladislav Shpilevoy Sasha, This patch is also LGTM, except the single comment below. On 17.07.20, Igor Munkin wrote: > > > > I reformatted the comment a bit to make it more clear: > > > > | /* > > | * Unlike lua_newthread(), luaT_newthread() does not leave > > | * the new Lua state on tarantool_L. > > I was around to it today and unfortunately it does[1]. So you need to > explicitly pop a newly created coroutine from the guest stack right > after anchoring it to the registry. As you mentioned in offline discussion there is an implicit pop underneath routine, so the implementation is fine and only this misleading comment need to be fixed. > > > | * > > | * It is desired behaviour here, because of two reasons. > > | * > > | * First, if we'll push something to tarantool_L and > > | * yield, then another fiber will not know that a stack > > | * top is changed and may operate on a wrong slot. > > | * > > | * Second, many requests that push a value to tarantool_L > > | * and yield may exhaust available slots on the stack. It > > | * is limited by LUAI_MAXSTACK build time constant (~65K). > > | */ > > -- > Best regards, > IM -- Best regards, IM