Re: [Tarantool-patches] [PATCH 1/1] swim: don't call swim_quit via FFI

["Alexander V. Tikhonov" <avtikhon@tarantool.org>]

Hi Vlad, thanks for the patch, as I see no new degradation found in
gitlab-ci testing commit criteria pipeline [1], patch LGTM.

[1] - https://gitlab.com/tarantool/tarantool/-/pipelines/220237031

On Fri, Nov 20, 2020 at 11:42:07PM +0100, Vladislav Shpilevoy wrote:
> swim_quit yields, because it joins the event handler fiber. Hence
> it can't be called via FFI, where a yield leads to undefined
> behaviour.
> 
> Closes #4570
> ---
> Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-4570-swim-crash
> Issue: https://github.com/tarantool/tarantool/issues/4570
> 
> @ChangeLog
> * Fixed a crash in swim.quit() (gh-4570).
> 
>  src/lua/swim.c   | 14 ++++++++++++++
>  src/lua/swim.lua |  5 +----
>  2 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/src/lua/swim.c b/src/lua/swim.c
> index ae916bf78..b9c9dd635 100644
> --- a/src/lua/swim.c
> +++ b/src/lua/swim.c
> @@ -88,6 +88,19 @@ lua_swim_delete(struct lua_State *L)
>  	return 0;
>  }
>  
> +/**
> + * Gracefully leave the cluster, broadcast a notification, and delete the SWIM
> + * instance. It is not FFI, because this operation yields.
> + */
> +static int
> +lua_swim_quit(struct lua_State *L)
> +{
> +	uint32_t ctypeid;
> +	struct swim *s = *(struct swim **) luaL_checkcdata(L, 1, &ctypeid);
> +	swim_quit(s);
> +	return 0;
> +}
> +
>  void
>  tarantool_lua_swim_init(struct lua_State *L)
>  {
> @@ -98,6 +111,7 @@ tarantool_lua_swim_init(struct lua_State *L)
>  	static const struct luaL_Reg lua_swim_internal_methods [] = {
>  		{"swim_new", lua_swim_new},
>  		{"swim_delete", lua_swim_delete},
> +		{"swim_quit", lua_swim_quit},
>  		{"swim_on_member_event", lua_swim_on_member_event},
>  		{NULL, NULL}
>  	};
> diff --git a/src/lua/swim.lua b/src/lua/swim.lua
> index c1ab1c5c3..1da55337a 100644
> --- a/src/lua/swim.lua
> +++ b/src/lua/swim.lua
> @@ -74,9 +74,6 @@ ffi.cdef[[
>      int
>      swim_size(const struct swim *swim);
>  
> -    void
> -    swim_quit(struct swim *swim);
> -
>      struct swim_member *
>      swim_self(struct swim *swim);
>  
> @@ -519,7 +516,7 @@ end
>  --
>  local function swim_quit(s)
>      local ptr = swim_check_instance(s, 'swim:quit')
> -    capi.swim_quit(ffi.gc(ptr, nil))
> +    internal.swim_quit(ffi.gc(ptr, nil))
>      s.ptr = nil
>      setmetatable(s, swim_mt_deleted)
>  end
> -- 
> 2.24.3 (Apple Git-128)
>