From: Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: Sergey Ostanevich <sergos@tarantool.org>, Igor Munkin <imun@tarantool.org> Cc: tarantool-patches@dev.tarantool.org Subject: [Tarantool-patches] [PATCH luajit] core: fix cdata decrementing Date: Tue, 16 Feb 2021 23:10:44 +0300 [thread overview] Message-ID: <20210216201044.20952-1-skaplun@tarantool.org> (raw) When cdata has custom finalizer (and so LJ_GC_CDATA_FIN flag) it is not collected immediately, when lj_cdata_free() is called. Instead, it is resurrected and marked finalized, so it is collected at the next GC cycle. The reason of the bug is that gc_cdatanum is decremented when cdata is resurrected too (i.e. twice). This patch excludes cdata decrementing from resurrection branch and adds corresponding tests. Resolves tarantool/tarantool#5820 Follows up tarantool/tarantool#5187 --- Branch: https://github.com/tarantool/luajit/tree/skaplun/gh-5820-improperly-cdata-counting Test Branch: https://github.com/tarantool/tarantool/tree/skaplun/gh-5820-improperly-cdata-counting Issue: https://github.com/tarantool/tarantool/issues/5820 ChangeLog entry for bumping LuaJIT: =================================================================== ## bugfix/LuaJIT * Fix double cdata decrementing in platform metrics when finalizer is set (gh-5820). =================================================================== src/lj_cdata.c | 3 +- test/misclib-getmetrics-capi.test.lua | 15 +++++++++- test/misclib-getmetrics-capi/testgetmetrics.c | 28 +++++++++++++++++++ test/misclib-getmetrics-lapi.test.lua | 15 +++++++++- 4 files changed, 58 insertions(+), 3 deletions(-) diff --git a/src/lj_cdata.c b/src/lj_cdata.c index 0dd8553..d3042f2 100644 --- a/src/lj_cdata.c +++ b/src/lj_cdata.c @@ -80,10 +80,11 @@ void LJ_FASTCALL lj_cdata_free(global_State *g, GCcdata *cd) lua_assert(ctype_hassize(ct->info) || ctype_isfunc(ct->info) || ctype_isextern(ct->info)); lj_mem_free(g, cd, sizeof(GCcdata) + sz); + g->gc.cdatanum--; } else { lj_mem_free(g, memcdatav(cd), sizecdatav(cd)); + g->gc.cdatanum--; } - g->gc.cdatanum--; } void lj_cdata_setfin(lua_State *L, GCcdata *cd, GCobj *obj, uint32_t it) diff --git a/test/misclib-getmetrics-capi.test.lua b/test/misclib-getmetrics-capi.test.lua index 1ad6958..e088c48 100755 --- a/test/misclib-getmetrics-capi.test.lua +++ b/test/misclib-getmetrics-capi.test.lua @@ -14,7 +14,7 @@ local jit_opt_default = { local tap = require('tap') local test = tap.test("clib-misc-getmetrics") -test:plan(10) +test:plan(11) local testgetmetrics = require("testgetmetrics") @@ -62,6 +62,19 @@ test:ok(testgetmetrics.objcount(function(iterations) jit.opt.start(unpack(jit_opt_default)) end)) +test:ok(testgetmetrics.objcount_cdata_decrement(function() + -- cdata decrement test. + -- See https://github.com/tarantool/tarantool/issues/5820. + local ffi = require("ffi") + local function nop() end + ffi.gc(ffi.cast("void *", 0), nop) + -- Does not collect cdata, but removes LJ_GC_CDATA_FIN flag + -- and resurrects object. + collectgarbage() + -- Collects cdata. + collectgarbage() +end)) + -- Compiled loop with a direct exit to the interpreter. test:ok(testgetmetrics.snap_restores(function() jit.opt.start(0, "hotloop=1") diff --git a/test/misclib-getmetrics-capi/testgetmetrics.c b/test/misclib-getmetrics-capi/testgetmetrics.c index 7fd3eef..6777633 100644 --- a/test/misclib-getmetrics-capi/testgetmetrics.c +++ b/test/misclib-getmetrics-capi/testgetmetrics.c @@ -155,6 +155,33 @@ static int objcount(lua_State *L) return 1; } +static int objcount_cdata_decrement(lua_State *L) +{ + /* + * cdata decrement test. + * See https://github.com/tarantool/tarantool/issues/5820. + */ + struct luam_Metrics oldm, newm; + int n = lua_gettop(L); + if (n != 1 || !lua_isfunction(L, 1)) + luaL_error(L, "incorrect argument: 1 function is required"); + + /* Force up garbage collect all dead objects. */ + lua_gc(L, LUA_GCCOLLECT, 0); + + luaM_metrics(L, &oldm); + /* + * The function generates and collects cdata with + * LJ_GC_CDATA_FIN flag. + */ + lua_call(L, 0, 0); + luaM_metrics(L, &newm); + assert(newm.gc_cdatanum - oldm.gc_cdatanum == 0); + + lua_pushboolean(L, 1); + return 1; +} + static int snap_restores(lua_State *L) { struct luam_Metrics oldm, newm; @@ -229,6 +256,7 @@ static const struct luaL_Reg testgetmetrics[] = { {"gc_allocated_freed", gc_allocated_freed}, {"gc_steps", gc_steps}, {"objcount", objcount}, + {"objcount_cdata_decrement", objcount_cdata_decrement}, {"snap_restores", snap_restores}, {"strhash", strhash}, {"tracenum_base", tracenum_base}, diff --git a/test/misclib-getmetrics-lapi.test.lua b/test/misclib-getmetrics-lapi.test.lua index 3b3d1f8..59bcea6 100755 --- a/test/misclib-getmetrics-lapi.test.lua +++ b/test/misclib-getmetrics-lapi.test.lua @@ -172,7 +172,7 @@ test:test("gc-steps", function(subtest) end) test:test("objcount", function(subtest) - subtest:plan(4) + subtest:plan(5) local ffi = require("ffi") jit.opt.start(0) @@ -231,6 +231,19 @@ test:test("objcount", function(subtest) subtest:is(new_metrics.gc_cdatanum, old_metrics.gc_cdatanum, "cdatanum don't change") + -- cdata decrement test. + -- See https://github.com/tarantool/tarantool/issues/5820. + local function nop() end + local cdatanum_old = misc.getmetrics().gc_cdatanum + ffi.gc(ffi.cast("void *", 0), nop) + -- Does not collect cdata, but removes LJ_GC_CDATA_FIN flag + -- and resurrects object. + collectgarbage() + -- Collects cdata. + collectgarbage() + subtest:is(misc.getmetrics().gc_cdatanum, cdatanum_old, + "cdatanum is decremented correctly") + -- Restore default jit settings. jit.opt.start(unpack(jit_opt_default)) end) -- 2.28.0
next reply other threads:[~2021-02-16 20:11 UTC|newest] Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-02-16 20:10 Sergey Kaplun via Tarantool-patches [this message] 2021-02-26 22:07 ` Igor Munkin via Tarantool-patches 2021-03-02 9:27 ` Sergey Ostanevich via Tarantool-patches 2021-03-04 13:35 ` Sergey Kaplun via Tarantool-patches 2021-03-04 13:28 ` Sergey Kaplun via Tarantool-patches 2021-03-04 22:04 ` Igor Munkin via Tarantool-patches 2021-03-04 22:04 ` Igor Munkin via Tarantool-patches
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=20210216201044.20952-1-skaplun@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=imun@tarantool.org \ --cc=sergos@tarantool.org \ --cc=skaplun@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH luajit] core: fix cdata decrementing' \ /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