ffi/gc: restore back rehashing of finalizers table

This is a follow-up to the commits
2115828 ("FFI: Drop finalizer table
rehash after GC cycle.") and bfcbaa7
("Drop unused function wrapper."). After them, the rehashing of the cdata
finalizer table at the end of the GC cycle is dropped. Without
reshashing of this table, the table increases the estimated amount of
memory for the GC. Hence, with the bigger `estimate`, the threshold
before starting the GC cycle is increased too. This allows allocating
more cdata objects and increasing the size of the finalizer table again.
This increases the memory estimate again and so on. As a result, we have
unlimited memory growth without rehashing of the table for the
cdata-intensive workloads.

This patch reverts back the code changes (but not the test) of the
aforementioned commits. Also, it fixes the possible crash after
rehashing of the cdata finalizers table by adding the protected call to
the GC steps on the trace itself and on the trace exit.

Reviewed-by: Sergey Bronnikov <[email protected]>
Signed-off-by: Sergey Kaplun <[email protected]>

Author: Buristan

src/lj_gc.c

@@ -548,6 +548,7 @@ static void gc_finalize(lua_State *L)
     setcdataV(L, &tmp, gco2cd(o));
     tv = lj_tab_set(L, tabref(g->gcroot[GCROOT_FFI_FIN]), &tmp);
     if (!tvisnil(tv)) {
+      g->gc.nocdatafin = 0;
       copyTV(L, &tmp, tv);
       setnilV(tv);  /* Clear entry in finalizer table. */
       gc_call_finalizer(g, L, &tmp, o);
@@ -693,6 +694,9 @@ static size_t gc_onestep(lua_State *L)
 	lj_str_resize(L, g->strmask >> 1);  /* Shrink string table. */
       if (gcref(g->gc.mmudata)) {  /* Need any finalizations? */
 	g->gc.state = GCSfinalize;
+#if LJ_HASFFI
+	g->gc.nocdatafin = 1;
+#endif
       } else {  /* Otherwise skip this phase to help the JIT. */
 	g->gc.state = GCSpause;  /* End of GC cycle. */
 	g->gc.debt = 0;
@@ -709,6 +713,9 @@ static size_t gc_onestep(lua_State *L)
 	g->gc.estimate -= GCFINALIZECOST;
       return GCFINALIZECOST;
     }
+#if LJ_HASFFI
+    if (!g->gc.nocdatafin) lj_tab_rehash(L, tabref(g->gcroot[GCROOT_FFI_FIN]));
+#endif
     g->gc.state = GCSpause;  /* End of GC cycle. */
     g->gc.debt = 0;
     return 0;
@@ -757,15 +764,35 @@ void LJ_FASTCALL lj_gc_step_fixtop(lua_State *L)
   lj_gc_step(L);
 }
 
+/* Need to protect lj_gc_step because it may throw. */
+static TValue *gc_step_jit_cp(lua_State *L, lua_CFunction dummy, void *ud)
+{
+  MSize steps = (MSize)(uintptr_t)ud;
+  UNUSED(dummy);
+
+  /* Always catch error here and don't call error function. */
+  cframe_errfunc(L->cframe) = 0;
+  cframe_nres(L->cframe) = -2*LUAI_MAXSTACK*(int)sizeof(TValue);
+
+  while (steps-- > 0 && lj_gc_step(L) == 0)
+    ;
+
+  return NULL;
+}
+
 #if LJ_HASJIT
 /* Perform multiple GC steps. Called from JIT-compiled code. */
 int LJ_FASTCALL lj_gc_step_jit(global_State *g, MSize steps)
 {
   lua_State *L = gco2th(gcref(g->cur_L));
+  int32_t ostate = g->vmstate;
+  int errcode;
   L->base = tvref(G(L)->jit_base);
   L->top = curr_topL(L);
-  while (steps-- > 0 && lj_gc_step(L) == 0)
-    ;
+  errcode = lj_vm_cpcall(L, NULL, (void *)(uintptr_t)steps, gc_step_jit_cp);
+  g->vmstate = ostate;
+  if (errcode)
+    lj_err_throw(L, errcode);  /* Propagate errors. */
   /* Return 1 to force a trace exit. */
   return (G(L)->gc.state == GCSatomic || G(L)->gc.state == GCSfinalize);
 }

src/lj_obj.h

@@ -611,7 +611,7 @@ typedef struct GCState {
   GCSize threshold;	/* Memory threshold. */
   uint8_t currentwhite;	/* Current white color. */
   uint8_t state;	/* GC state. */
-  uint8_t unused0;
+  uint8_t nocdatafin;	/* No cdata finalizer called. */
 #if LJ_64
   uint8_t lightudnum;	/* Number of lightuserdata segments - 1. */
 #else

src/lj_tab.c

@@ -388,6 +388,13 @@ static void rehashtab(lua_State *L, GCtab *t, cTValue *ek)
   lj_tab_resize(L, t, asize, hsize2hbits(total));
 }
 
+#if LJ_HASFFI
+void lj_tab_rehash(lua_State *L, GCtab *t)
+{
+  rehashtab(L, t, niltv(L));
+}
+#endif
+
 void lj_tab_reasize(lua_State *L, GCtab *t, uint32_t nasize)
 {
   lj_tab_resize(L, t, nasize+1, t->hmask > 0 ? lj_fls(t->hmask)+1 : 0);

src/lj_tab.h

@@ -41,6 +41,9 @@ LJ_FUNC GCtab * LJ_FASTCALL lj_tab_new1(lua_State *L, uint32_t ahsize);
 LJ_FUNCA GCtab * LJ_FASTCALL lj_tab_dup(lua_State *L, const GCtab *kt);
 LJ_FUNC void LJ_FASTCALL lj_tab_clear(GCtab *t);
 LJ_FUNC void LJ_FASTCALL lj_tab_free(global_State *g, GCtab *t);
+#if LJ_HASFFI
+LJ_FUNC void lj_tab_rehash(lua_State *L, GCtab *t);
+#endif
 LJ_FUNC void lj_tab_resize(lua_State *L, GCtab *t, uint32_t asize, uint32_t hbits);
 LJ_FUNCA void lj_tab_reasize(lua_State *L, GCtab *t, uint32_t nasize);
 

src/lj_trace.c

@@ -823,6 +823,18 @@ static TValue *trace_exit_cp(lua_State *L, lua_CFunction dummy, void *ud)
   return NULL;
 }
 
+/* Need to protect lj_gc_step because it may throw. */
+static TValue *trace_exit_gc_cp(lua_State *L, lua_CFunction dummy, void *unused)
+{
+  /* Always catch error here and don't call error function. */
+  cframe_errfunc(L->cframe) = 0;
+  cframe_nres(L->cframe) = -2*LUAI_MAXSTACK*(int)sizeof(TValue);
+  lj_gc_step(L);
+  UNUSED(dummy);
+  UNUSED(unused);
+  return NULL;
+}
+
 #ifndef LUAJIT_DISABLE_VMEVENT
 /* Push all registers from exit state. */
 static void trace_exit_regs(lua_State *L, ExitState *ex)
@@ -918,8 +930,12 @@ int LJ_FASTCALL lj_trace_exit(jit_State *J, void *exptr)
   } else if (LJ_HASPROFILE && (G(L)->hookmask & HOOK_PROFILE)) {
     /* Just exit to interpreter. */
   } else if (G(L)->gc.state == GCSatomic || G(L)->gc.state == GCSfinalize) {
-    if (!(G(L)->hookmask & HOOK_GC))
-      lj_gc_step(L);  /* Exited because of GC: drive GC forward. */
+    if (!(G(L)->hookmask & HOOK_GC)) {
+      /* Exited because of GC: drive GC forward. */
+      errcode = lj_vm_cpcall(L, NULL, NULL, trace_exit_gc_cp);
+      if (errcode)
+	return -errcode;  /* Return negated error code. */
+    }
   } else if ((J->flags & JIT_F_ON)) {
     trace_hotside(J, pc);
   }

test/tarantool-tests/lj-1350-fix-gc-finalizer-pressure.test.lua

@@ -0,0 +1,62 @@
+local tap = require('tap')
+local ffi = require('ffi')
+
+-- Test file to demonstrate too fast memory growth without
+-- shrinking the finalizer table for cdata at the end of the GC
+-- cycle.
+-- See also: https://github.com/LuaJIT/LuaJIT/issues/1350.
+
+local test = tap.test('lj-1350-fix-gc-finalizer-pressure'):skipcond({
+  -- Tarantool has its own cdata objects. This makes the test
+  -- unpredictable. Disable it.
+  ['Disable test for Tarantool'] = _TARANTOOL,
+  ['Test requires GC64 mode enabled'] = not ffi.abi('gc64'),
+  ['Disabled with Valgrind (Timeout)'] = os.getenv('LUAJIT_TEST_USE_VALGRIND'),
+})
+
+test:plan(1)
+
+-- This stress test shows the memory overconsumption if LuaJIT
+-- does not rehash the table with cdata finalizers at the end of
+-- the GC cycle. Without reshashing of this table, it increases
+-- the estimated amount of memory for the GC. Hence, with the
+-- bigger `estimate`, the threshold before starting the GC cycle
+-- is increased too. This allows allocating more cdata objects and
+-- increasing the size of the finalizer table again. This
+-- increases the memory estimate again and so on. As a result, we
+-- have unlimited memory growth without rehashing of the table for
+-- the cdata-intensive workloads.
+
+local cnt = 0
+-- Finalizer function for cdata object.
+local function dec() cnt = cnt - 1 end
+
+local function new_obj()
+  -- Use an array type to make it possible to set a GC finalizer.
+  local obj = ffi.new('uint64_t [1]')
+  -- Insert the object into the finalizer table.
+  ffi.gc(obj, dec)
+  cnt = cnt + 1
+  -- The empirical assertion check. Without rehashing, there are
+  -- more cdata objects than the mentioned limit.
+  assert(cnt < 10e6, 'too much cdata alive at the moment')
+  return obj
+end
+
+-- Reset the GC.
+collectgarbage('collect')
+
+local t = {}
+-- Don't use too huge limit to avoid the test run being too long.
+-- But it is big enough to trigger the assertion above without
+-- cdata finalizer table rehashing at the end of the GC cycle.
+for i = 1, 3e7 do
+  table.insert(t, new_obj())
+  -- Reset the table. Now cdata objects are collectable.
+  if i % 3.5e6 == 0 then
+    t = {}
+  end
+end
+
+test:ok(true, 'not too much cdata alive at the moment')
+test:done(true)