[Tarantool-patches] [PATCH luajit 2/3] core: fix resources leak in memory profiler
Sergey Kaplun
skaplun at tarantool.org
Wed Dec 30 01:22:57 MSK 2020
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
More information about the Tarantool-patches
mailing list