From: Igor Munkin <imun@tarantool.org> To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Cc: tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH 1/1] swim: don't call swim_quit via FFI Date: Mon, 23 Nov 2020 16:40:30 +0300 [thread overview] Message-ID: <20201123134030.GB14086@tarantool.org> (raw) In-Reply-To: <8f87d28e7b1dc259fa0b1f230c0c6a5b25573bf1.1605912044.git.v.shpilevoy@tarantool.org> Vlad, Thanks for the patch! LGTM, with a couple nits below. On 20.11.20, 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. Minor: I wouldn't call this UB, let's reword it "where a yield might lead to the platform panic". > > 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); Minor: I don't know whether it's common practice in Tarantool, but assert that <ctypeid> equals to <ctid_swim_ptr> would be nice here. Feel free to ignore. > + swim_quit(s); > + return 0; > +} > + > void > tarantool_lua_swim_init(struct lua_State *L) > { <snipped> > -- > 2.24.3 (Apple Git-128) > -- Best regards, IM
next prev parent reply other threads:[~2020-11-23 13:40 UTC|newest] Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-20 22:42 Vladislav Shpilevoy 2020-11-23 13:40 ` Igor Munkin [this message] 2020-11-23 22:10 ` Vladislav Shpilevoy 2020-12-18 17:28 ` Alexander V. Tikhonov 2020-12-20 17:21 ` Vladislav Shpilevoy
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: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20201123134030.GB14086@tarantool.org \ --to=imun@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH 1/1] swim: don'\''t call swim_quit via FFI' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * 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