Skip to content

Commit

Permalink
core: fix cdata decrementing
Browse files Browse the repository at this point in the history
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

Reviewed-by: Igor Munkin <[email protected]>
Reviewed-by: Sergey Ostanevich <[email protected]>
Signed-off-by: Igor Munkin <[email protected]>
(cherry picked from commit 82932be)
  • Loading branch information
Buristan authored and igormunkin committed Mar 4, 2021
1 parent 45f7912 commit 2779973
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 3 deletions.
3 changes: 2 additions & 1 deletion src/lj_cdata.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
15 changes: 14 additions & 1 deletion test/tarantool-tests/misclib-getmetrics-capi.test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,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")

Expand Down Expand Up @@ -63,6 +63,19 @@ test:ok(testgetmetrics.objcount(function(iterations)
jit.opt.start(unpack(jit_opt_default))
end))

test:ok(testgetmetrics.objcount_cdata_decrement(function()
-- gc_cdatanum 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 the cdata, but resurrects the object and
-- removes LJ_GC_CDATA_FIN flag.
collectgarbage()
-- Collects the cdata.
collectgarbage()
end))

-- Compiled loop with a direct exit to the interpreter.
test:ok(testgetmetrics.snap_restores(function()
jit.opt.start(0, "hotloop=1")
Expand Down
28 changes: 28 additions & 0 deletions test/tarantool-tests/misclib-getmetrics-capi/testgetmetrics.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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},
Expand Down
15 changes: 14 additions & 1 deletion test/tarantool-tests/misclib-getmetrics-lapi.test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,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)
Expand Down Expand Up @@ -236,6 +236,19 @@ test:test("objcount", function(subtest)
subtest:is(new_metrics.gc_cdatanum, old_metrics.gc_cdatanum,
"cdatanum don't change")

-- gc_cdatanum 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 the cdata, but resurrects the object and
-- removes LJ_GC_CDATA_FIN flag.
collectgarbage()
-- Collects the 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)
Expand Down

0 comments on commit 2779973

Please sign in to comment.