Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alexander Turenko via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Maria Khaydich <maria.khaydich@tarantool.org>
Cc: tarantool-patches <tarantool-patches@dev.tarantool.org>
Subject: Re: [Tarantool-patches] [PATCH] box: remove context from stack
Date: Thu, 1 Jul 2021 15:34:34 +0300	[thread overview]
Message-ID: <20210701123434.ezfp3bsmhny73jjf@tkn_work_nb> (raw)
In-Reply-To: <1591698918.771792066@f423.i.mail.ru>

On Tue, Jun 09, 2020 at 01:35:18PM +0300, Maria Khaydich wrote:
> I suppose no tests are needed since this one is pretty straightforward.
> ----------------------------------------------------------------------
> Lua stack was broken because we forgot to clear the context
> in case of an error when return value of called function was
> not serializable.
> Closes #4617
> ---
> Branch:
> https://github.com/tarantool/tarantool/compare/eljashm/gh-4617-broken-lua-stack  
> Issue:
> https://github.com/tarantool/tarantool/issues/4617  
> @ChangeLog
> Remove context from lua stack in case of an error.

Here we describe changes for a user (in the way a user may understand
it). For complex bugs we can briefly describe a situation, when it

It is now in the changelogs/ directory.

>  src/box/lua/call.c | 1 +
>  1 file changed, 1 insertion(+)
> diff --git a/src/box/lua/call.c b/src/box/lua/call.c
> index 6588ec2fa..7ab49983d 100644
> --- a/src/box/lua/call.c
> +++ b/src/box/lua/call.c
> @@ -436,6 +436,7 @@ port_lua_do_dump(struct port *base, struct mpstream *stream,
>      int top = lua_gettop(L);
>      if (lua_cpcall(L, handler, &ctx) != 0) {
>          luaT_toerror(port->L);
> +        lua_pop(L, 1); 

There are several doubts and thoughts around it:

1. lua_cpcall() always removes arguments from the Lua stack (in case of
   an error too). How we can observe this ctx userdata on tarantool_L
   without yield in encode_lua_call()?
2. luaT_toerror() leans on luaT_tolstring(), so it duplicates a string
   with the error message. Should not we pop two elements in the case? I
   would just call lua_settop().
3. The clash is due to tarantool_L usage (see 1.10.1-48-g18d87adf3). Is
   there any real reason why we can't use port->L here?

CCed Igor and Sergey to think around together.

>          return -1;
>      }
>      lua_settop(L, top);
> -- 
> 2.24.0

  reply	other threads:[~2021-07-01 12:35 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-09 10:35 Maria Khaydich
2021-07-01 12:34 ` Alexander Turenko via Tarantool-patches [this message]
2021-09-07 12:40   ` Sergey Kaplun via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210701123434.ezfp3bsmhny73jjf@tkn_work_nb \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=alexander.turenko@tarantool.org \
    --cc=maria.khaydich@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH] box: remove context from stack' \


* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox