From: Sergey Kaplun <skaplun@tarantool.org> To: Sergey Ostanevich <sergos@tarantool.org>, Igor Munkin <imun@tarantool.org> Cc: tarantool-patches@dev.tarantool.org Subject: [Tarantool-patches] [PATCH luajit 2/3] core: fix resources leak in memory profiler Date: Wed, 30 Dec 2020 01:22:57 +0300 [thread overview] Message-ID: <061599ac978b162d42ce80ac65106ba67c6d5fcb.1609278043.git.skaplun@tarantool.org> (raw) In-Reply-To: <cover.1609278043.git.skaplun@tarantool.org> When the profiler is failing to start with error different from PROFILE_ERRIO neither a file stream is closed nor ctx is freed in case of incorrect return status checking. To avoid this behaviour on_stop callback is called manually inside the profiler when error on start is occurring. Checks in misc.memprof.start() are omitted. Follows up tarantool/tarantool#5442 --- * How patch was checked: Before patch you can occur the error like: | $ src/luajit -e ' | local f, msg, errno = misc.memprof.start("/tmp/tmp_memprofile.bin") | misc.memprof.start("/tmp/tmp_memprofile.bin") print(f,msg,errno) | ' | true nil nil | luajit: lj_state.c:178: close_state: Assertion `g->gc.total == sizeof(GG_State)' failed. This patch fixes it. * Why this assertion is not failed in tests (we have the test with same functionality)? This assertion failed inside close_state. Tarantool in some reason doesn't call lua_close on stop. It's weird to me. I'll try to find an explanation and will create a ticket. * Why I don't create a test case. The best idea is to do something like this and waiting for OOM: | for _ = 1, 10000 do | misc.memprof.start("/tmp/tmp_memprofile.bin") | end But it's disgusting, so as I've discussed with Igor offline test case will be ommited. src/lib_misc.c | 4 ---- src/lj_memprof.c | 8 ++++++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/lib_misc.c b/src/lib_misc.c index f89827e..1dab08c 100644 --- a/src/lib_misc.c +++ b/src/lib_misc.c @@ -177,10 +177,6 @@ LJLIB_CF(misc_memprof_start) memprof_status = lj_memprof_start(L, &opt); if (LJ_UNLIKELY(memprof_status != PROFILE_SUCCESS)) { - if (memprof_status == PROFILE_ERRIO) { - fclose(ctx->stream); - lj_mem_free(ctx->g, ctx, sizeof(*ctx)); - } switch (memprof_status) { case PROFILE_ERRUSE: lua_pushnil(L); diff --git a/src/lj_memprof.c b/src/lj_memprof.c index 4994de5..c4d2645 100644 --- a/src/lj_memprof.c +++ b/src/lj_memprof.c @@ -228,8 +228,11 @@ int lj_memprof_start(struct lua_State *L, const struct lj_memprof_options *opt) lua_assert(opt->buf != NULL); lua_assert(opt->len != 0); - if (mp->state != MPS_IDLE) + if (mp->state != MPS_IDLE) { + /* Clean up resourses. Ignore possible errors. */ + opt->on_stop(opt->ctx, opt->buf); return PROFILE_ERRRUN; + } /* Discard possible old errno. */ mp->saved_errno = 0; @@ -331,7 +334,8 @@ errio: int lj_memprof_start(struct lua_State *L, const struct lj_memprof_options *opt) { UNUSED(L); - UNUSED(opt); + /* Clean up resourses. Ignore possible errors. */ + opt->on_stop(opt->ctx, opt->buf); return PROFILE_ERRUSE; } -- 2.28.0
next prev parent reply other threads:[~2020-12-29 22:23 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-12-29 22:22 [Tarantool-patches] [PATCH luajit 0/3] LuaJIT memory profiler bug fixes Sergey Kaplun 2020-12-29 22:22 ` [Tarantool-patches] [PATCH luajit 1/3] misc: fix build with disabled memory profiler Sergey Kaplun 2020-12-30 8:49 ` Igor Munkin 2020-12-30 8:52 ` Sergey Kaplun 2020-12-30 9:42 ` Sergey Ostanevich 2020-12-29 22:22 ` Sergey Kaplun [this message] 2020-12-30 9:06 ` [Tarantool-patches] [PATCH luajit 2/3] core: fix resources leak in " Igor Munkin 2020-12-30 9:31 ` Sergey Ostanevich 2020-12-30 9:33 ` Sergey Kaplun 2020-12-30 9:32 ` Sergey Kaplun 2020-12-30 9:53 ` Sergey Ostanevich 2020-12-29 22:22 ` [Tarantool-patches] [PATCH luajit 3/3] core: remove excess assertion inside memprof Sergey Kaplun 2020-12-30 9:39 ` Igor Munkin 2020-12-30 9:50 ` Sergey Kaplun 2020-12-30 10:50 ` Sergey Ostanevich 2020-12-30 11:06 ` Sergey Kaplun 2020-12-30 8:24 ` [Tarantool-patches] [PATCH luajit 0/3] LuaJIT memory profiler bug fixes Alexander V. Tikhonov 2020-12-30 11:20 ` Igor Munkin
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=061599ac978b162d42ce80ac65106ba67c6d5fcb.1609278043.git.skaplun@tarantool.org \ --to=skaplun@tarantool.org \ --cc=imun@tarantool.org \ --cc=sergos@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH luajit 2/3] core: fix resources leak in memory profiler' \ /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