More effort in avoiding errors in finalizers

Before calling a finalizer, Lua not only checks stack limits, but
actually ensures that a minimum number of slots are already allocated
for the call. (If it cannot ensure that, it postpones the finalizer.)
That avoids finalizers not running due to memory errors that the
programmer cannot control.

ldo.c

@@ -221,13 +221,21 @@ l_noret luaD_errerr (lua_State *L) {
 
 
 /*
-** Check whether stack has enough space to run a simple function (such
-** as a finalizer): At least BASIC_STACK_SIZE in the Lua stack and
-** 2 slots in the C stack.
+** Check whether stacks have enough space to run a simple function (such
+** as a finalizer): At least BASIC_STACK_SIZE in the Lua stack, two
+** available CallInfos, and two "slots" in the C stack.
 */
 int luaD_checkminstack (lua_State *L) {
-  return ((stacksize(L) < MAXSTACK - BASIC_STACK_SIZE) &&
-          (getCcalls(L) < LUAI_MAXCCALLS - 2));
+  if (getCcalls(L) >= LUAI_MAXCCALLS - 2)
+    return 0;  /* not enough C-stack slots */
+  if (L->ci->next == NULL && luaE_extendCI(L, 0) == NULL)
+    return 0;  /* unable to allocate first ci */
+  if (L->ci->next->next == NULL && luaE_extendCI(L, 0) == NULL)
+    return 0;  /* unable to allocate second ci */
+  if (L->stack_last.p - L->top.p >= BASIC_STACK_SIZE)
+    return 1;  /* enough (BASIC_STACK_SIZE) free slots in the Lua stack */
+  else  /* try to grow stack to a size with enough free slots */
+    return luaD_growstack(L, BASIC_STACK_SIZE, 0);
 }
 
 
@@ -616,7 +624,7 @@ void luaD_poscall (lua_State *L, CallInfo *ci, int nres) {
 
 
 
-#define next_ci(L)  (L->ci->next ? L->ci->next : luaE_extendCI(L))
+#define next_ci(L)  (L->ci->next ? L->ci->next : luaE_extendCI(L, 1))
 
 
 /*

lgc.c

@@ -1293,7 +1293,7 @@ static void finishgencycle (lua_State *L, global_State *g) {
   correctgraylists(g);
   checkSizes(L, g);
   g->gcstate = GCSpropagate;  /* skip restart */
-  if (!g->gcemergency && luaD_checkminstack(L))
+  if (g->tobefnz != NULL && !g->gcemergency && luaD_checkminstack(L))
     callallpendingfinalizers(L);
 }
 

lstate.c

@@ -68,14 +68,19 @@ void luaE_setdebt (global_State *g, l_mem debt) {
 }
 
 
-CallInfo *luaE_extendCI (lua_State *L) {
+CallInfo *luaE_extendCI (lua_State *L, int err) {
   CallInfo *ci;
-  lua_assert(L->ci->next == NULL);
-  ci = luaM_new(L, CallInfo);
-  lua_assert(L->ci->next == NULL);
-  L->ci->next = ci;
+  ci = luaM_reallocvector(L, NULL, 0, 1, CallInfo);
+  if (l_unlikely(ci == NULL)) {  /* allocation failed? */
+    if (err)
+      luaM_error(L);  /* raise the error */
+    return NULL;  /* else only report it */
+  }
+  ci->next = L->ci->next;
   ci->previous = L->ci;
-  ci->next = NULL;
+  L->ci->next = ci;
+  if (ci->next)
+    ci->next->previous = ci;
   ci->u.l.trap = 0;
   L->nci++;
   return ci;

lstate.h

@@ -438,7 +438,7 @@ union GCUnion {
 LUAI_FUNC void luaE_setdebt (global_State *g, l_mem debt);
 LUAI_FUNC void luaE_freethread (lua_State *L, lua_State *L1);
 LUAI_FUNC lu_mem luaE_threadsize (lua_State *L);
-LUAI_FUNC CallInfo *luaE_extendCI (lua_State *L);
+LUAI_FUNC CallInfo *luaE_extendCI (lua_State *L, int err);
 LUAI_FUNC void luaE_shrinkCI (lua_State *L);
 LUAI_FUNC void luaE_checkcstack (lua_State *L);
 LUAI_FUNC void luaE_incCstack (lua_State *L);

ltests.c

@@ -1106,6 +1106,27 @@ static int stacklevel (lua_State *L) {
 }
 
 
+static int resetCI (lua_State *L) {
+  CallInfo *ci = L->ci;
+  while (ci->next != NULL) {
+    CallInfo *tofree = ci->next;
+    ci->next = ci->next->next;
+    luaM_free(L, tofree);
+    L->nci--;
+  }
+  return 0;
+}
+
+
+static int reallocstack (lua_State *L) {
+  int n = cast_int(luaL_checkinteger(L, 1));
+  lua_lock(L);
+  luaD_reallocstack(L, cast_int(L->top.p - L->stack.p) + n, 1);
+  lua_unlock(L);
+  return 0;
+}
+
+
 static int table_query (lua_State *L) {
   const Table *t;
   int i = cast_int(luaL_optinteger(L, 2, -1));
@@ -2182,6 +2203,8 @@ static const struct luaL_Reg tests_funcs[] = {
   {"s2d", s2d},
   {"sethook", sethook},
   {"stacklevel", stacklevel},
+  {"resetCI", resetCI},
+  {"reallocstack", reallocstack},
   {"sizes", get_sizes},
   {"testC", testC},
   {"makeCfunc", makeCfunc},

testes/gc.lua

@@ -707,4 +707,46 @@ end
 
 collectgarbage(oldmode)
 
+
+if T then
+  print("testing stack issues when calling finalizers")
+
+  local X
+  local obj
+
+  local function initobj ()
+    X = false
+    obj = setmetatable({}, {__gc = function ()  X = true end})
+  end
+
+  local function loop (n)
+    if n > 0 then loop(n - 1) end
+  end
+
+  -- should not try to call finalizer without a CallInfo available
+  initobj()
+  loop(20)   -- ensure stack space
+  T.resetCI()   -- remove extra CallInfos
+  T.alloccount(0)   -- cannot allocate more CallInfos
+  obj = nil
+  collectgarbage()   -- will not call finalizer
+  T.alloccount()
+  assert(X == false)
+  collectgarbage()   -- now will call finalizer (it was still pending)
+  assert(X == true)
+
+  -- should not try to call finalizer without stack space available
+  initobj()
+  loop(5)     -- ensure enough CallInfos
+  T.reallocstack(0)   -- remove extra stack slots
+  T.alloccount(0)   -- cannot reallocate stack
+  obj = nil
+  collectgarbage()   -- will not call finalizer
+  T.alloccount()
+  assert(X == false)
+  collectgarbage()   -- now will call finalizer (it was still pending)
+  assert(X == true)
+end
+
+
 print('OK')

testes/memerr.lua

@@ -282,6 +282,25 @@ testamem("growing stack", function ()
   return foo(100)
 end)
 
+
+collectgarbage()
+collectgarbage()
+global io, T, setmetatable, collectgarbage, print
+
+local Count = 0
+testamem("finalizers", function ()
+  local X = false
+  local obj = setmetatable({}, {__gc = function ()  X = true end})
+  obj = nil
+  T.resetCI()   -- remove extra CallInfos
+  T.reallocstack(18)   -- remove extra stack slots
+  Count = Count + 1
+  io.stderr:write(Count, "\n")
+  T.trick(io)
+  collectgarbage()
+  return X
+end)
+
 -- }==================================================================
 
 

testes/tracegc.lua

@@ -1,10 +1,15 @@
 -- track collections
 
+
 local M = {}
 
 -- import list
-local setmetatable, stderr, collectgarbage =
-         setmetatable, io.stderr, collectgarbage
+local stderr, collectgarbage = io.stderr, collectgarbage
+
+-- the debug version of setmetatable does not create any object (such as
+-- a '__metatable' string), and so it is more appropriate to be used in
+-- a finalizer
+local setmetatable = require"debug".setmetatable
 
 global none