Re: [Tarantool-patches] [PATCH v2 1/3] merger: fix NULL dereference when called via iproto

[Igor Munkin <imun@tarantool.org>]

Sasha,

This patch is also LGTM, except the single comment below.

On 17.07.20, Igor Munkin wrote:

<snipped>

> > 
> > 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 <luaL_ref> 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).
> >  |  */

<snipped>

> 
> -- 
> Best regards,
> IM

-- 
Best regards,
IM