[Tarantool-patches] [PATCH luajit 0/3] LuaJIT memory profiler bug fixes

[Tarantool-patches] [PATCH luajit 0/3] LuaJIT memory profiler bug fixes

[Sergey Kaplun]

This patchset accumulate different fixes for memory profiler bugs
introduced in scope of [1].

Branch: https://github.com/tarantool/luajit/tree/skaplun/gh-noticket-misc-memprof-fixes
Branch CI: https://github.com/tarantool/tarantool/tree/skaplun/gh-noticket-misc-memprof-fixes

CI is red [2], [3] but it isn't related to the patch, AFAICS. It has failed
before my commit (see [4], [5]).
CI :https://gitlab.com/tarantool/tarantool/-/pipelines/235622340

[1]: https://github.com/tarantool/tarantool/issues/5442
[2]: https://gitlab.com/tarantool/tarantool/-/jobs/936802176
[3]: https://gitlab.com/tarantool/tarantool/-/jobs/936802176
[4]: https://gitlab.com/tarantool/tarantool/-/jobs/936547205#L4118
[5]: https://gitlab.com/tarantool/tarantool/-/pipelines/235383279

Sergey Kaplun (3):
  misc: fix build with disabled memory profiler
  core: fix resources leak in memory profiler
  core: remove excess assertion inside memprof

 src/lib_misc.c   |  8 ++++----
 src/lj_errmsg.h  |  2 +-
 src/lj_memprof.c | 18 +++++++++++-------
 3 files changed, 16 insertions(+), 12 deletions(-)

-- 
2.28.0

[Tarantool-patches] [PATCH luajit 1/3] misc: fix build with disabled memory profiler

[Sergey Kaplun]

This patch fixes the regression introduced in scope of
b4e6bf0d82100049d29af1aa3196e781270f39d1 ('misc: add Lua API for memory
profiler'). Build is failed with disabled memory profiler because
related error messages are not defined.

This patch fixes build by making LJ_ERR_PROF_MISUSE visible and avoiding
usage of other profiler-related errors when the profiler is disabled.

Follows up tarantool/tarantool#5442
---
 src/lib_misc.c  | 4 ++++
 src/lj_errmsg.h | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/lib_misc.c b/src/lib_misc.c
index 958c189..f89827e 100644
--- a/src/lib_misc.c
+++ b/src/lib_misc.c
@@ -187,6 +187,7 @@ LJLIB_CF(misc_memprof_start)
       lua_pushstring(L, err2msg(LJ_ERR_PROF_MISUSE));
       lua_pushinteger(L, EINVAL);
       return 3;
+#if LJ_HASMEMPROF
     case PROFILE_ERRRUN:
       lua_pushnil(L);
       lua_pushstring(L, err2msg(LJ_ERR_PROF_ISRUNNING));
@@ -194,6 +195,7 @@ LJLIB_CF(misc_memprof_start)
       return 3;
     case PROFILE_ERRIO:
       return luaL_fileresult(L, 0, fname);
+#endif
     default:
       lua_assert(0);
       return 0;
@@ -214,6 +216,7 @@ LJLIB_CF(misc_memprof_stop)
       lua_pushstring(L, err2msg(LJ_ERR_PROF_MISUSE));
       lua_pushinteger(L, EINVAL);
       return 3;
+#if LJ_HASMEMPROF
     case PROFILE_ERRRUN:
       lua_pushnil(L);
       lua_pushstring(L, err2msg(LJ_ERR_PROF_NOTRUNNING));
@@ -221,6 +224,7 @@ LJLIB_CF(misc_memprof_stop)
       return 3;
     case PROFILE_ERRIO:
       return luaL_fileresult(L, 0, NULL);
+#endif
     default:
       lua_assert(0);
       return 0;
diff --git a/src/lj_errmsg.h b/src/lj_errmsg.h
index bebe804..ae0a18c 100644
--- a/src/lj_errmsg.h
+++ b/src/lj_errmsg.h
@@ -185,9 +185,9 @@ ERRDEF(FFI_NYIPACKBIT,	"NYI: packed bit fields")
 ERRDEF(FFI_NYICALL,	"NYI: cannot call this C function (yet)")
 #endif
 
-#if LJ_HASMEMPROF
 /* Profiler errors. */
 ERRDEF(PROF_MISUSE,	"profiler misuse")
+#if LJ_HASMEMPROF
 ERRDEF(PROF_ISRUNNING,	"profiler is running already")
 ERRDEF(PROF_NOTRUNNING,	"profiler is not running")
 #endif
-- 
2.28.0

Re: [Tarantool-patches] [PATCH luajit 1/3] misc: fix build with disabled memory profiler

[Igor Munkin]

Sergey,

Thanks for the patch! LGTM with a single nit below.

On 30.12.20, Sergey Kaplun wrote:
> This patch fixes the regression introduced in scope of
> b4e6bf0d82100049d29af1aa3196e781270f39d1 ('misc: add Lua API for memory
> profiler'). Build is failed with disabled memory profiler because

Typo: s/is failed/fails/.

> related error messages are not defined.
> 
> This patch fixes build by making LJ_ERR_PROF_MISUSE visible and avoiding
> usage of other profiler-related errors when the profiler is disabled.
> 
> Follows up tarantool/tarantool#5442
> ---
>  src/lib_misc.c  | 4 ++++
>  src/lj_errmsg.h | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 

<snipped>

> -- 
> 2.28.0
> 

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit 1/3] misc: fix build with disabled memory profiler

[Sergey Kaplun]

Hi, Igor!

Thanks for the review!

On 30.12.20, Igor Munkin wrote:
> Sergey,
> 
> Thanks for the patch! LGTM with a single nit below.
> 
> On 30.12.20, Sergey Kaplun wrote:
> > This patch fixes the regression introduced in scope of
> > b4e6bf0d82100049d29af1aa3196e781270f39d1 ('misc: add Lua API for memory
> > profiler'). Build is failed with disabled memory profiler because
> 
> Typo: s/is failed/fails/.

Fixed. Branch is force-pushed.

> 
> > related error messages are not defined.
> > 
> > This patch fixes build by making LJ_ERR_PROF_MISUSE visible and avoiding
> > usage of other profiler-related errors when the profiler is disabled.
> > 
> > Follows up tarantool/tarantool#5442
> > ---
> >  src/lib_misc.c  | 4 ++++
> >  src/lj_errmsg.h | 2 +-
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> > 
> 
> <snipped>
> 
> > -- 
> > 2.28.0
> > 
> 
> -- 
> Best regards,
> IM

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 1/3] misc: fix build with disabled memory profiler

[Sergey Ostanevich]

[-- Attachment #1: Type: text/plain, Size: 1047 bytes --]

LGTM.

Sergos

> On 30 Dec 2020, at 11:52, Sergey Kaplun <skaplun@tarantool.org> wrote:
> 
> Hi, Igor!
> 
> Thanks for the review!
> 
> On 30.12.20, Igor Munkin wrote:
>> Sergey,
>> 
>> Thanks for the patch! LGTM with a single nit below.
>> 
>> On 30.12.20, Sergey Kaplun wrote:
>>> This patch fixes the regression introduced in scope of
>>> b4e6bf0d82100049d29af1aa3196e781270f39d1 ('misc: add Lua API for memory
>>> profiler'). Build is failed with disabled memory profiler because
>> 
>> Typo: s/is failed/fails/.
> 
> Fixed. Branch is force-pushed.
> 
>> 
>>> related error messages are not defined.
>>> 
>>> This patch fixes build by making LJ_ERR_PROF_MISUSE visible and avoiding
>>> usage of other profiler-related errors when the profiler is disabled.
>>> 
>>> Follows up tarantool/tarantool#5442
>>> ---
>>> src/lib_misc.c  | 4 ++++
>>> src/lj_errmsg.h | 2 +-
>>> 2 files changed, 5 insertions(+), 1 deletion(-)
>>> 
>> 
>> <snipped>
>> 
>>> -- 
>>> 2.28.0
>>> 
>> 
>> -- 
>> Best regards,
>> IM
> 
> -- 
> Best regards,
> Sergey Kaplun


[-- Attachment #2: Type: text/html, Size: 9319 bytes --]

[Tarantool-patches] [PATCH luajit 2/3] core: fix resources leak in memory profiler

[Sergey Kaplun]

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

Re: [Tarantool-patches] [PATCH luajit 2/3] core: fix resources leak in memory profiler

[Igor Munkin]

Sergey,

Thanks for the patch! LGTM except the wording in commit message
(consider the comments below).

Side note: I want to notice that with on_start callback things would be
clearer. Let's return to this again later.

On 30.12.20, Sergey Kaplun wrote:
> When the profiler is failing to start with error different from

Typo: s/is failing/fails/.
Typo: s/with error/with the error/.

> PROFILE_ERRIO neither a file stream is closed nor ctx is freed
> in case of incorrect return status checking.

Strictly saying there are two problems:
* Possible leakage for PROFILE_ERRRUN
* Double free for PROFILE_ERRIO

So I propose the following wording:

| When memory profiler fails to start with PROFILE_ERRRUN status both
| stream and ctx are not released. At the same time when memory profiler
| fails to start with the PROFILE_ERRIO status both stream and ctx are
| released twice. Both cases occur due to invalid return status checking.

> 
> To avoid this behaviour on_stop callback is called manually inside

Minor: s/To avoid this behaviour/To fix the leakage/.

> the profiler when error on start is occurring. Checks in

Typo: s/is occuring/occurs/.

> 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.

Side note: The test aims to hit LUA_ERRMEM and need to be run for a long
time. The exact reason we didn't face this failure is the omitted
<lua_close> in Tarantool.

> 
>  src/lib_misc.c   | 4 ----
>  src/lj_memprof.c | 8 ++++++--
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 

<snipped>

>  
> -- 
> 2.28.0
> 

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit 2/3] core: fix resources leak in memory profiler

[Sergey Ostanevich]

[-- Attachment #1: Type: text/plain, Size: 2853 bytes --]

Serge, can you point me to a branch please?

> On 30 Dec 2020, at 12:06, Igor Munkin <imun@tarantool.org> wrote:
> 
> Sergey,
> 
> Thanks for the patch! LGTM except the wording in commit message
> (consider the comments below).
> 
> Side note: I want to notice that with on_start callback things would be
> clearer. Let's return to this again later.
> 
> On 30.12.20, Sergey Kaplun wrote:
>> When the profiler is failing to start with error different from
> 
> Typo: s/is failing/fails/.
> Typo: s/with error/with the error/.
> 
>> PROFILE_ERRIO neither a file stream is closed nor ctx is freed
>> in case of incorrect return status checking.
> 
> Strictly saying there are two problems:
> * Possible leakage for PROFILE_ERRRUN
> * Double free for PROFILE_ERRIO
> 
> So I propose the following wording:
> 
> | When memory profiler fails to start with PROFILE_ERRRUN status both
> | stream and ctx are not released. At the same time when memory profiler
> | fails to start with the PROFILE_ERRIO status both stream and ctx are
> | released twice. Both cases occur due to invalid return status checking.
> 
>> 
>> To avoid this behaviour on_stop callback is called manually inside
> 
> Minor: s/To avoid this behaviour/To fix the leakage/.
> 
>> the profiler when error on start is occurring. Checks in
> 
> Typo: s/is occuring/occurs/.
> 
>> 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.
> 
> Side note: The test aims to hit LUA_ERRMEM and need to be run for a long
> time. The exact reason we didn't face this failure is the omitted
> <lua_close> in Tarantool.
> 
>> 
>> src/lib_misc.c   | 4 ----
>> src/lj_memprof.c | 8 ++++++--
>> 2 files changed, 6 insertions(+), 6 deletions(-)
>> 
> 
> <snipped>
> 
>> 
>> -- 
>> 2.28.0
>> 
> 
> -- 
> Best regards,
> IM


[-- Attachment #2: Type: text/html, Size: 30417 bytes --]

Re: [Tarantool-patches] [PATCH luajit 2/3] core: fix resources leak in memory profiler

[Sergey Kaplun]

Hi!

On 30.12.20, Sergey Ostanevich wrote:
> Serge, can you point me to a branch please?

https://github.com/tarantool/luajit/tree/skaplun/gh-noticket-misc-memprof-fixes

> 
> > On 30 Dec 2020, at 12:06, Igor Munkin <imun@tarantool.org> wrote:
> > 
> > Sergey,
> > 
> > Thanks for the patch! LGTM except the wording in commit message
> > (consider the comments below).
> > 
> > Side note: I want to notice that with on_start callback things would be
> > clearer. Let's return to this again later.
> > 
> > On 30.12.20, Sergey Kaplun wrote:
> >> When the profiler is failing to start with error different from
> > 
> > Typo: s/is failing/fails/.
> > Typo: s/with error/with the error/.
> > 
> >> PROFILE_ERRIO neither a file stream is closed nor ctx is freed
> >> in case of incorrect return status checking.
> > 
> > Strictly saying there are two problems:
> > * Possible leakage for PROFILE_ERRRUN
> > * Double free for PROFILE_ERRIO
> > 
> > So I propose the following wording:
> > 
> > | When memory profiler fails to start with PROFILE_ERRRUN status both
> > | stream and ctx are not released. At the same time when memory profiler
> > | fails to start with the PROFILE_ERRIO status both stream and ctx are
> > | released twice. Both cases occur due to invalid return status checking.
> > 
> >> 
> >> To avoid this behaviour on_stop callback is called manually inside
> > 
> > Minor: s/To avoid this behaviour/To fix the leakage/.
> > 
> >> the profiler when error on start is occurring. Checks in
> > 
> > Typo: s/is occuring/occurs/.
> > 
> >> 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.
> > 
> > Side note: The test aims to hit LUA_ERRMEM and need to be run for a long
> > time. The exact reason we didn't face this failure is the omitted
> > <lua_close> in Tarantool.
> > 
> >> 
> >> src/lib_misc.c   | 4 ----
> >> src/lj_memprof.c | 8 ++++++--
> >> 2 files changed, 6 insertions(+), 6 deletions(-)
> >> 
> > 
> > <snipped>
> > 
> >> 
> >> -- 
> >> 2.28.0
> >> 
> > 
> > -- 
> > Best regards,
> > IM
> 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 2/3] core: fix resources leak in memory profiler

[Sergey Kaplun]

Igor,

Thanks for the review!

I've updated commit message to the following as you purposed:

| core: fix resources leak in memory profiler
|
| When memory profiler fails to start with PROFILE_ERRRUN status both
| stream and ctx are not released. At the same time when memory profiler
| fails to start with the PROFILE_ERRIO status both stream and ctx are
| released twice. Both cases occur due to invalid return status checking.
|
| To fix the leakage on_stop callback is called manually inside
| the profiler when error on start occurs. Checks in
| misc.memprof.start() are omitted.
|
| Follows up tarantool/tarantool#5442


Branch is force-pushed.

On 30.12.20, Igor Munkin wrote:
> Sergey,
> 
> Thanks for the patch! LGTM except the wording in commit message
> (consider the comments below).
> 
> Side note: I want to notice that with on_start callback things would be
> clearer. Let's return to this again later.
> 
> On 30.12.20, Sergey Kaplun wrote:
> > When the profiler is failing to start with error different from
> 
> Typo: s/is failing/fails/.
> Typo: s/with error/with the error/.
> 
> > PROFILE_ERRIO neither a file stream is closed nor ctx is freed
> > in case of incorrect return status checking.
> 
> Strictly saying there are two problems:
> * Possible leakage for PROFILE_ERRRUN
> * Double free for PROFILE_ERRIO
> 
> So I propose the following wording:
> 
> | When memory profiler fails to start with PROFILE_ERRRUN status both
> | stream and ctx are not released. At the same time when memory profiler
> | fails to start with the PROFILE_ERRIO status both stream and ctx are
> | released twice. Both cases occur due to invalid return status checking.
> 
> > 
> > To avoid this behaviour on_stop callback is called manually inside
> 
> Minor: s/To avoid this behaviour/To fix the leakage/.
> 
> > the profiler when error on start is occurring. Checks in
> 
> Typo: s/is occuring/occurs/.
> 
> > 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.
> 
> Side note: The test aims to hit LUA_ERRMEM and need to be run for a long
> time. The exact reason we didn't face this failure is the omitted
> <lua_close> in Tarantool.

Yep, I've found this pretty comment inside <src/main.cc>:

| #if 0
| 	/*
| 	 * This doesn't work reliably since things
| 	 * are too interconnected.
| 	 */
| 	tarantool_lua_free();
| 	session_free();
| 	user_cache_free();
| 	fiber_free();
| 	memory_free();
| 	random_free();
| #endif

> 
> > 
> >  src/lib_misc.c   | 4 ----
> >  src/lj_memprof.c | 8 ++++++--
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> > 
> 
> <snipped>
> 
> >  
> > -- 
> > 2.28.0
> > 
> 
> -- 
> Best regards,
> IM

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 2/3] core: fix resources leak in memory profiler

[Sergey Ostanevich]

[-- Attachment #1: Type: text/plain, Size: 4066 bytes --]

LGTM.

Sergos

> On 30 Dec 2020, at 12:32, Sergey Kaplun <skaplun@tarantool.org> wrote:
> 
> Igor,
> 
> Thanks for the review!
> 
> I've updated commit message to the following as you purposed:
> 
> | core: fix resources leak in memory profiler
> |
> | When memory profiler fails to start with PROFILE_ERRRUN status both
> | stream and ctx are not released. At the same time when memory profiler
> | fails to start with the PROFILE_ERRIO status both stream and ctx are
> | released twice. Both cases occur due to invalid return status checking.
> |
> | To fix the leakage on_stop callback is called manually inside
> | the profiler when error on start occurs. Checks in
> | misc.memprof.start() are omitted.
> |
> | Follows up tarantool/tarantool#5442
> 
> 
> Branch is force-pushed.
> 
> On 30.12.20, Igor Munkin wrote:
>> Sergey,
>> 
>> Thanks for the patch! LGTM except the wording in commit message
>> (consider the comments below).
>> 
>> Side note: I want to notice that with on_start callback things would be
>> clearer. Let's return to this again later.
>> 
>> On 30.12.20, Sergey Kaplun wrote:
>>> When the profiler is failing to start with error different from
>> 
>> Typo: s/is failing/fails/.
>> Typo: s/with error/with the error/.
>> 
>>> PROFILE_ERRIO neither a file stream is closed nor ctx is freed
>>> in case of incorrect return status checking.
>> 
>> Strictly saying there are two problems:
>> * Possible leakage for PROFILE_ERRRUN
>> * Double free for PROFILE_ERRIO
>> 
>> So I propose the following wording:
>> 
>> | When memory profiler fails to start with PROFILE_ERRRUN status both
>> | stream and ctx are not released. At the same time when memory profiler
>> | fails to start with the PROFILE_ERRIO status both stream and ctx are
>> | released twice. Both cases occur due to invalid return status checking.
>> 
>>> 
>>> To avoid this behaviour on_stop callback is called manually inside
>> 
>> Minor: s/To avoid this behaviour/To fix the leakage/.
>> 
>>> the profiler when error on start is occurring. Checks in
>> 
>> Typo: s/is occuring/occurs/.
>> 
>>> 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.
>> 
>> Side note: The test aims to hit LUA_ERRMEM and need to be run for a long
>> time. The exact reason we didn't face this failure is the omitted
>> <lua_close> in Tarantool.
> 
> Yep, I've found this pretty comment inside <src/main.cc <http://main.cc/>>:
> 
> | #if 0
> | 	/*
> | 	 * This doesn't work reliably since things
> | 	 * are too interconnected.
> | 	 */
> | 	tarantool_lua_free();
> | 	session_free();
> | 	user_cache_free();
> | 	fiber_free();
> | 	memory_free();
> | 	random_free();
> | #endif
> 
>> 
>>> 
>>> src/lib_misc.c   | 4 ----
>>> src/lj_memprof.c | 8 ++++++--
>>> 2 files changed, 6 insertions(+), 6 deletions(-)
>>> 
>> 
>> <snipped>
>> 
>>> 
>>> -- 
>>> 2.28.0
>>> 
>> 
>> -- 
>> Best regards,
>> IM
> 
> -- 
> Best regards,
> Sergey Kaplun


[-- Attachment #2: Type: text/html, Size: 42292 bytes --]

[Tarantool-patches] [PATCH luajit 3/3] core: remove excess assertion inside memprof

[Sergey Kaplun]

lj_debug_frameline() may return -1 for Lua functions on top.
In this case assertion inside memprof_write_lfunc() that returned line
is not negative is incorrect.

This patch removes this assertion. For negative returned line value
profiler is reported zero frameline.

Follows up tarantool/tarantool#5442
---

I have not thought of any more correct check than:
| -- Check alocations for Lua function on top.
| -- This is not the best test case but the most simple.
| -- Trace allocation on top leads to assertion.
| jit.on()
| for _ = 1, 100 do
|   local _ = tostring(_)
| end
| jit.off()
| jit.flush()

But this is not very fair: here we have tail call to cpcall()
(with trace_state() as an argument) without creating a new guest stack
frame. Also test looks redundant -- assertion obviously can't fail as
far as it doesn't exist. But I can add this test case if you want.

 src/lj_memprof.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/lj_memprof.c b/src/lj_memprof.c
index c4d2645..0568049 100644
--- a/src/lj_memprof.c
+++ b/src/lj_memprof.c
@@ -90,15 +90,15 @@ static void memprof_write_lfunc(struct lj_wbuf *out, uint8_t aevent,
 				cTValue *nextframe)
 {
   const BCLine line = lj_debug_frameline(L, fn, nextframe);
+  lj_wbuf_addbyte(out, aevent | ASOURCE_LFUNC);
+  lj_wbuf_addu64(out, (uintptr_t)funcproto(fn));
   /*
-  ** Line is always >= 0 if we are inside a Lua function.
+  ** Line is >= 0 if we are inside a Lua function.
+  ** An exception may be when the Lua function is on top.
   ** Equals to zero when LuaJIT is built with the
   ** -DLUAJIT_DISABLE_DEBUGINFO flag.
   */
-  lua_assert(line >= 0);
-  lj_wbuf_addbyte(out, aevent | ASOURCE_LFUNC);
-  lj_wbuf_addu64(out, (uintptr_t)funcproto(fn));
-  lj_wbuf_addu64(out, (uint64_t)line);
+  lj_wbuf_addu64(out, line >= 0 ? (uint64_t)line : 0);
 }
 
 static void memprof_write_cfunc(struct lj_wbuf *out, uint8_t aevent,
-- 
2.28.0

Re: [Tarantool-patches] [PATCH luajit 3/3] core: remove excess assertion inside memprof

[Igor Munkin]

Sergey,

Thanks for the patch! AFAICS, the issue relates to invalid writer to be
chosen but not to the <lj_debug_frameline> return value. As we discussed
offline, all these allocations should be reported as internal, but now
the engine attempts to attribute them with a Lua function being
recorded. Please mention this fact in the commit message.

I believe there is an issue with the VM states introduces in the
previous series and it should be fixed. For now I'm OK with this fix
though it is not quite relevant. Anyway, the world is better with it,
crashes are gone, so formally LGTM with two nits below.

Please create a ticket for this to investigate the root cause.

On 30.12.20, Sergey Kaplun wrote:
> lj_debug_frameline() may return -1 for Lua functions on top.
> In this case assertion inside memprof_write_lfunc() that returned line
> is not negative is incorrect.

I propose the following wording:
| There are the cases when memory profiler attempts to attribute
| allocations triggered by JIT engine recording phase with a Lua
| function to be recorded. At this case lj_debug_frameline() may return
| BC_NOPOS (i.e. a negative value) so the assertion in the Lua writer
| memprof_write_lfunc() is violated.

> 
> This patch removes this assertion. For negative returned line value
> profiler is reported zero frameline.
> 
> Follows up tarantool/tarantool#5442
> ---
> 
> I have not thought of any more correct check than:
> | -- Check alocations for Lua function on top.
> | -- This is not the best test case but the most simple.
> | -- Trace allocation on top leads to assertion.
> | jit.on()
> | for _ = 1, 100 do
> |   local _ = tostring(_)
> | end
> | jit.off()
> | jit.flush()
> 
> But this is not very fair: here we have tail call to cpcall()
> (with trace_state() as an argument) without creating a new guest stack
> frame. Also test looks redundant -- assertion obviously can't fail as
> far as it doesn't exist. But I can add this test case if you want.
> 
>  src/lj_memprof.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/src/lj_memprof.c b/src/lj_memprof.c
> index c4d2645..0568049 100644
> --- a/src/lj_memprof.c
> +++ b/src/lj_memprof.c
> @@ -90,15 +90,15 @@ static void memprof_write_lfunc(struct lj_wbuf *out, uint8_t aevent,
>  				cTValue *nextframe)
>  {
>    const BCLine line = lj_debug_frameline(L, fn, nextframe);
> +  lj_wbuf_addbyte(out, aevent | ASOURCE_LFUNC);
> +  lj_wbuf_addu64(out, (uintptr_t)funcproto(fn));
>    /*
> -  ** Line is always >= 0 if we are inside a Lua function.
> +  ** Line is >= 0 if we are inside a Lua function.
> +  ** An exception may be when the Lua function is on top.
>    ** Equals to zero when LuaJIT is built with the

Minor: Please make this wording clearer considering my proposal for
commit message. It doesn't refer to the original problem for now.

>    ** -DLUAJIT_DISABLE_DEBUGINFO flag.
>    */
> -  lua_assert(line >= 0);
> -  lj_wbuf_addbyte(out, aevent | ASOURCE_LFUNC);
> -  lj_wbuf_addu64(out, (uintptr_t)funcproto(fn));
> -  lj_wbuf_addu64(out, (uint64_t)line);
> +  lj_wbuf_addu64(out, line >= 0 ? (uint64_t)line : 0);
>  }
>  
>  static void memprof_write_cfunc(struct lj_wbuf *out, uint8_t aevent,
> -- 
> 2.28.0
> 

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit 3/3] core: remove excess assertion inside memprof

[Sergey Kaplun]

Igor,

Thanks for the review!

On 30.12.20, Igor Munkin wrote:
> Sergey,
> 
> Thanks for the patch! AFAICS, the issue relates to invalid writer to be
> chosen but not to the <lj_debug_frameline> return value. As we discussed
> offline, all these allocations should be reported as internal, but now
> the engine attempts to attribute them with a Lua function being
> recorded. Please mention this fact in the commit message.
> 
> I believe there is an issue with the VM states introduces in the
> previous series and it should be fixed. For now I'm OK with this fix
> though it is not quite relevant. Anyway, the world is better with it,
> crashes are gone, so formally LGTM with two nits below.
> 
> Please create a ticket for this to investigate the root cause.

Sure.

> 
> On 30.12.20, Sergey Kaplun wrote:
> > lj_debug_frameline() may return -1 for Lua functions on top.
> > In this case assertion inside memprof_write_lfunc() that returned line
> > is not negative is incorrect.
> 
> I propose the following wording:
> | There are the cases when memory profiler attempts to attribute
> | allocations triggered by JIT engine recording phase with a Lua
> | function to be recorded. At this case lj_debug_frameline() may return
> | BC_NOPOS (i.e. a negative value) so the assertion in the Lua writer
> | memprof_write_lfunc() is violated.
> 
> > 
> > This patch removes this assertion. For negative returned line value
> > profiler is reported zero frameline.
> > 
> > Follows up tarantool/tarantool#5442
> > ---

I've reworded commit message to the following, as you purposed:


> > 
> > I have not thought of any more correct check than:
> > | -- Check alocations for Lua function on top.
> > | -- This is not the best test case but the most simple.
> > | -- Trace allocation on top leads to assertion.
> > | jit.on()
> > | for _ = 1, 100 do
> > |   local _ = tostring(_)
> > | end
> > | jit.off()
> > | jit.flush()
> > 
> > But this is not very fair: here we have tail call to cpcall()
> > (with trace_state() as an argument) without creating a new guest stack
> > frame. Also test looks redundant -- assertion obviously can't fail as
> > far as it doesn't exist. But I can add this test case if you want.
> > 
> >  src/lj_memprof.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/lj_memprof.c b/src/lj_memprof.c
> > index c4d2645..0568049 100644
> > --- a/src/lj_memprof.c
> > +++ b/src/lj_memprof.c
> > @@ -90,15 +90,15 @@ static void memprof_write_lfunc(struct lj_wbuf *out, uint8_t aevent,
> >  				cTValue *nextframe)
> >  {
> >    const BCLine line = lj_debug_frameline(L, fn, nextframe);
> > +  lj_wbuf_addbyte(out, aevent | ASOURCE_LFUNC);
> > +  lj_wbuf_addu64(out, (uintptr_t)funcproto(fn));
> >    /*
> > -  ** Line is always >= 0 if we are inside a Lua function.
> > +  ** Line is >= 0 if we are inside a Lua function.
> > +  ** An exception may be when the Lua function is on top.
> >    ** Equals to zero when LuaJIT is built with the
> 
> Minor: Please make this wording clearer considering my proposal for
> commit message. It doesn't refer to the original problem for now.

Reworded. See the iterative patch below. Branch is force-pushed.

| core: remove excess assertion inside memprof
|
| There are the cases when the memory profiler attempts to attribute
| allocations triggered by JIT engine recording phase with a Lua function
| to be recorded. At this case lj_debug_frameline() may return BC_NOPOS
| (i.e. a negative value) so the assertion in the Lua writer
| memprof_write_lfunc() is violated.
|
| This patch removes this assertion. For negative returned line value
| profiler is reported zero frameline.
|
| Follows up tarantool/tarantool#5442

> 
> >    ** -DLUAJIT_DISABLE_DEBUGINFO flag.
> >    */
> > -  lua_assert(line >= 0);
> > -  lj_wbuf_addbyte(out, aevent | ASOURCE_LFUNC);
> > -  lj_wbuf_addu64(out, (uintptr_t)funcproto(fn));
> > -  lj_wbuf_addu64(out, (uint64_t)line);
> > +  lj_wbuf_addu64(out, line >= 0 ? (uint64_t)line : 0);
> >  }
> >  
> >  static void memprof_write_cfunc(struct lj_wbuf *out, uint8_t aevent,
> > -- 
> > 2.28.0
> > 
> 
> -- 
> Best regards,
> IM

===================================================================
diff --git a/src/lj_memprof.c b/src/lj_memprof.c
index 0568049..37ec4c9 100644
--- a/src/lj_memprof.c
+++ b/src/lj_memprof.c
@@ -94,7 +94,10 @@ static void memprof_write_lfunc(struct lj_wbuf *out, uint8_t aevent,
   lj_wbuf_addu64(out, (uintptr_t)funcproto(fn));
   /*
   ** Line is >= 0 if we are inside a Lua function.
-  ** An exception may be when the Lua function is on top.
+  ** There are the cases when the memory profiler attempts
+  ** to attribute allocations triggered by JIT engine recording
+  ** phase with a Lua function to be recorded. At this case
+  ** lj_debug_frameline() may return BC_NOPOS (i.e. a negative value).
   ** Equals to zero when LuaJIT is built with the
   ** -DLUAJIT_DISABLE_DEBUGINFO flag.
   */
===================================================================

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 3/3] core: remove excess assertion inside memprof

[Sergey Ostanevich]

[-- Attachment #1: Type: text/plain, Size: 2177 bytes --]

Hi!

Thanks for the patch!
Just one nit in comments.

LGTM.
Sergos

> 
> Reworded. See the iterative patch below. Branch is force-pushed.
> 
> | core: remove excess assertion inside memprof
> |
> | There are the cases when the memory profiler attempts to attribute
	      ^^^ remove, just 'there are cases'
		
> | allocations triggered by JIT engine recording phase with a Lua function
> | to be recorded. At this case lj_debug_frameline() may return BC_NOPOS
> | (i.e. a negative value) so the assertion in the Lua writer
> | memprof_write_lfunc() is violated.
> |
> | This patch removes this assertion. For negative returned line value
> | profiler is reported zero frameline.
> |
> | Follows up tarantool/tarantool#5442
> 
>> 
>>>   ** -DLUAJIT_DISABLE_DEBUGINFO flag.
>>>   */
>>> -  lua_assert(line >= 0);
>>> -  lj_wbuf_addbyte(out, aevent | ASOURCE_LFUNC);
>>> -  lj_wbuf_addu64(out, (uintptr_t)funcproto(fn));
>>> -  lj_wbuf_addu64(out, (uint64_t)line);
>>> +  lj_wbuf_addu64(out, line >= 0 ? (uint64_t)line : 0);
>>> }
>>> 
>>> static void memprof_write_cfunc(struct lj_wbuf *out, uint8_t aevent,
>>> -- 
>>> 2.28.0
>>> 
>> 
>> -- 
>> Best regards,
>> IM
> 
> ===================================================================
> diff --git a/src/lj_memprof.c b/src/lj_memprof.c
> index 0568049..37ec4c9 100644
> --- a/src/lj_memprof.c
> +++ b/src/lj_memprof.c
> @@ -94,7 +94,10 @@ static void memprof_write_lfunc(struct lj_wbuf *out, uint8_t aevent,
>   lj_wbuf_addu64(out, (uintptr_t)funcproto(fn));
>   /*
>   ** Line is >= 0 if we are inside a Lua function.
> -  ** An exception may be when the Lua function is on top.
> +  ** There are the cases when the memory profiler attempts
> +  ** to attribute allocations triggered by JIT engine recording
> +  ** phase with a Lua function to be recorded. At this case
> +  ** lj_debug_frameline() may return BC_NOPOS (i.e. a negative value).
>   ** Equals to zero when LuaJIT is built with the
>   ** -DLUAJIT_DISABLE_DEBUGINFO flag.
>   */
> ===================================================================
> 
> -- 
> Best regards,
> Sergey Kaplun


[-- Attachment #2: Type: text/html, Size: 29367 bytes --]

Re: [Tarantool-patches] [PATCH luajit 3/3] core: remove excess assertion inside memprof

[Sergey Kaplun]

Hi!

Thanks for the review!

On 30.12.20, Sergey Ostanevich wrote:
> Hi!
> 
> Thanks for the patch!
> Just one nit in comments.
> 
> LGTM.
> Sergos
> 
> > 
> > Reworded. See the iterative patch below. Branch is force-pushed.
> > 
> > | core: remove excess assertion inside memprof
> > |
> > | There are the cases when the memory profiler attempts to attribute
> 	      ^^^ remove, just 'there are cases'

Thanks! Fixed in the commit message and the comment. Branch is
force-pushed. See the iterative diff below.

> 		
> > | allocations triggered by JIT engine recording phase with a Lua function
> > | to be recorded. At this case lj_debug_frameline() may return BC_NOPOS
> > | (i.e. a negative value) so the assertion in the Lua writer
> > | memprof_write_lfunc() is violated.
> > |
> > | This patch removes this assertion. For negative returned line value
> > | profiler is reported zero frameline.
> > |
> > | Follows up tarantool/tarantool#5442
> > 
> >> 
> >>>   ** -DLUAJIT_DISABLE_DEBUGINFO flag.
> >>>   */
> >>> -  lua_assert(line >= 0);
> >>> -  lj_wbuf_addbyte(out, aevent | ASOURCE_LFUNC);
> >>> -  lj_wbuf_addu64(out, (uintptr_t)funcproto(fn));
> >>> -  lj_wbuf_addu64(out, (uint64_t)line);
> >>> +  lj_wbuf_addu64(out, line >= 0 ? (uint64_t)line : 0);
> >>> }
> >>> 
> >>> static void memprof_write_cfunc(struct lj_wbuf *out, uint8_t aevent,
> >>> -- 
> >>> 2.28.0
> >>> 
> >> 
> >> -- 
> >> Best regards,
> >> IM
> > 
> > ===================================================================
> > diff --git a/src/lj_memprof.c b/src/lj_memprof.c
> > index 0568049..37ec4c9 100644
> > --- a/src/lj_memprof.c
> > +++ b/src/lj_memprof.c
> > @@ -94,7 +94,10 @@ static void memprof_write_lfunc(struct lj_wbuf *out, uint8_t aevent,
> >   lj_wbuf_addu64(out, (uintptr_t)funcproto(fn));
> >   /*
> >   ** Line is >= 0 if we are inside a Lua function.
> > -  ** An exception may be when the Lua function is on top.
> > +  ** There are the cases when the memory profiler attempts
> > +  ** to attribute allocations triggered by JIT engine recording
> > +  ** phase with a Lua function to be recorded. At this case
> > +  ** lj_debug_frameline() may return BC_NOPOS (i.e. a negative value).
> >   ** Equals to zero when LuaJIT is built with the
> >   ** -DLUAJIT_DISABLE_DEBUGINFO flag.
> >   */
> > ===================================================================
> > 
> > -- 
> > Best regards,
> > Sergey Kaplun
> 

===================================================================
diff --git a/src/lj_memprof.c b/src/lj_memprof.c
index 37ec4c9..2c1ef3b 100644
--- a/src/lj_memprof.c
+++ b/src/lj_memprof.c
@@ -94,7 +94,7 @@ static void memprof_write_lfunc(struct lj_wbuf *out, uint8_t aevent,
   lj_wbuf_addu64(out, (uintptr_t)funcproto(fn));
   /*
   ** Line is >= 0 if we are inside a Lua function.
-  ** There are the cases when the memory profiler attempts
+  ** There are cases when the memory profiler attempts
   ** to attribute allocations triggered by JIT engine recording
   ** phase with a Lua function to be recorded. At this case
   ** lj_debug_frameline() may return BC_NOPOS (i.e. a negative value).
===================================================================

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 0/3] LuaJIT memory profiler bug fixes

[Alexander V. Tikhonov]

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

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

On Wed, Dec 30, 2020 at 01:22:55AM +0300, Sergey Kaplun via Tarantool-patches wrote:
> This patchset accumulate different fixes for memory profiler bugs
> introduced in scope of [1].
> 
> Branch: https://github.com/tarantool/luajit/tree/skaplun/gh-noticket-misc-memprof-fixes
> Branch CI: https://github.com/tarantool/tarantool/tree/skaplun/gh-noticket-misc-memprof-fixes
> 
> CI is red [2], [3] but it isn't related to the patch, AFAICS. It has failed
> before my commit (see [4], [5]).
> CI :https://gitlab.com/tarantool/tarantool/-/pipelines/235622340
> 
> [1]: https://github.com/tarantool/tarantool/issues/5442
> [2]: https://gitlab.com/tarantool/tarantool/-/jobs/936802176
> [3]: https://gitlab.com/tarantool/tarantool/-/jobs/936802176
> [4]: https://gitlab.com/tarantool/tarantool/-/jobs/936547205#L4118
> [5]: https://gitlab.com/tarantool/tarantool/-/pipelines/235383279
> 
> Sergey Kaplun (3):
>   misc: fix build with disabled memory profiler
>   core: fix resources leak in memory profiler
>   core: remove excess assertion inside memprof
> 
>  src/lib_misc.c   |  8 ++++----
>  src/lj_errmsg.h  |  2 +-
>  src/lj_memprof.c | 18 +++++++++++-------
>  3 files changed, 16 insertions(+), 12 deletions(-)
> 
> -- 
> 2.28.0
> 

Re: [Tarantool-patches] [PATCH luajit 0/3] LuaJIT memory profiler bug fixes

[Igor Munkin]

Sergey,

On 30.12.20, Sergey Kaplun wrote:
> This patchset accumulate different fixes for memory profiler bugs
> introduced in scope of [1].
> 
> Branch: https://github.com/tarantool/luajit/tree/skaplun/gh-noticket-misc-memprof-fixes
> Branch CI: https://github.com/tarantool/tarantool/tree/skaplun/gh-noticket-misc-memprof-fixes
> 
> CI is red [2], [3] but it isn't related to the patch, AFAICS. It has failed
> before my commit (see [4], [5]).
> CI :https://gitlab.com/tarantool/tarantool/-/pipelines/235622340
> 
> [1]: https://github.com/tarantool/tarantool/issues/5442
> [2]: https://gitlab.com/tarantool/tarantool/-/jobs/936802176
> [3]: https://gitlab.com/tarantool/tarantool/-/jobs/936802176
> [4]: https://gitlab.com/tarantool/tarantool/-/jobs/936547205#L4118
> [5]: https://gitlab.com/tarantool/tarantool/-/pipelines/235383279
> 
> Sergey Kaplun (3):
>   misc: fix build with disabled memory profiler
>   core: fix resources leak in memory profiler
>   core: remove excess assertion inside memprof

I've checked your patchset into tarantool/luajit bleeding branch and
bumped a new version in Tarantool master.

> 

<snipped>

> 
> -- 
> 2.28.0
> 

-- 
Best regards,
IM