From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 7498A70358; Tue, 16 Feb 2021 23:11:39 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 7498A70358 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1613506299; bh=JZganqkt06w+2vvlQ/bLeje1JHhTwscmFnZDnXWLKxY=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=v1mFVN4GI/QEKh+OWyC3gSyWX09qHP80Ehy7DBwwqHmJETojm0coFMioMVGzjqChF nESaZM/DxeS+gFlaDBcyBBjwZft7F/6Tog3iWczHM79jCVgf2jkfheVuKUDUCQiIMu hlM2D68eJLCzNPNIbjxZzth55ud0NMgl84JBTM5A= Received: from smtp60.i.mail.ru (smtp60.i.mail.ru [217.69.128.40]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 9AED670358 for ; Tue, 16 Feb 2021 23:11:37 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 9AED670358 Received: by smtp60.i.mail.ru with esmtpa (envelope-from ) id 1lC6h6-0001Gr-95; Tue, 16 Feb 2021 23:11:36 +0300 To: Sergey Ostanevich , Igor Munkin Date: Tue, 16 Feb 2021 23:10:44 +0300 Message-Id: <20210216201044.20952-1-skaplun@tarantool.org> X-Mailer: git-send-email 2.28.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD91883A1EE8D2E9932147046C009FE7103E1819F397FE8CCAE182A05F5380850404060C3B06925AC63616FA36883A68B0182B7035CA9142580089395586FFD9C07 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE729508FF2E8683A3EB287FD4696A6DC2FA8DF7F3B2552694A4E2F5AFA99E116B42401471946AA11AF855697CD6E92616736C75B72B9FDC35052120BFB3F63BC1840A5AABA2AD371197C6FB206A91F05B23FD2FEE70BCAF5F3A1C64CDAB264EA51F04B652EEC242312D2E47CDBA5A96583C09775C1D3CA48CF17B107DEF921CE79117882F4460429724CE54428C33FAD30A8DF7F3B2552694AC26CFBAC0749D213D2E47CDBA5A9658378DA827A17800CE71AE4D56B06699BBC9FA2833FD35BB23DF004C9065253843057739F23D657EF2B13377AFFFEAFD26923F8577A6DFFEA7C468D16C903838CAB93EC92FD9297F6715571747095F342E857739F23D657EF2BD5E8D9A59859A8B69DC5D3EDC80DC730089D37D7C0E48F6C5571747095F342E857739F23D657EF2B6825BDBE14D8E7024847893F9AA87235E5BFE6E7EFDEDCD789D4C264860C145E X-C1DE0DAB: C20DE7B7AB408E4181F030C43753B8186998911F362727C414F749A5E30D975CF81F587C2CCB19332232D77712D535E4D9526685BD16023D9C2B6934AE262D3EE7EAB7254005DCED193B6F3DB70418581E0A4E2319210D9B64D260DF9561598F01A9E91200F654B0CCA28C6D779E2CD78E8E86DC7131B365E7726E8460B7C23C X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34987A0E8D235BB53484640E072FEF41204C51BE69AD4DA6F5F9EB38636990FD0350DE3A214E6E905B1D7E09C32AA3244CD7022DA929F791DB831E15259FDD966605AB220A9D022EBC927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojbL9S8ysBdXjpc5wwM2EzcyOeqKFNprE5 X-Mailru-Sender: 3B9A0136629DC91206CBC582EFEF4CB404F6F3792B82D139D20D4292AAAEEC701BE96DBAF4D557C5F2400F607609286E924004A7DEC283833C7120B22964430C52B393F8C72A41A89437F6177E88F7363CDA0F3B3F5B9367 X-Mras: Ok Subject: [Tarantool-patches] [PATCH luajit] core: fix cdata decrementing X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 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