More effort in avoiding errors in finalizersHEADmaster

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.

8 files changed, 118 insertions, 16 deletions

diff --git a/ldo.c b/ldo.c
index 6d0184ec..12e0364b 100644
--- a/ldo.c
+++ b/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))
 
 
 /*
diff --git a/lgc.c b/lgc.c
index f1d9a7ce..0f89451c 100644
--- a/lgc.c
+++ b/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);
 }
 
diff --git a/lstate.c b/lstate.c
index 70a11aae..7d341991 100644
--- a/lstate.c
+++ b/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;
diff --git a/lstate.h b/lstate.h
index 20dc4d24..01387283 100644
--- a/lstate.h
+++ b/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);
diff --git a/ltests.c b/ltests.c
index c4905f94..ce2b20ca 100644
--- a/ltests.c
+++ b/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},
diff --git a/testes/gc.lua b/testes/gc.lua
index 62713dac..e50d9029 100644
--- a/testes/gc.lua
+++ b/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')
diff --git a/testes/memerr.lua b/testes/memerr.lua
index 9c940ca7..a55514a9 100644
--- a/testes/memerr.lua
+++ b/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)
+
 -- }==================================================================
 
 
diff --git a/testes/tracegc.lua b/testes/tracegc.lua
index a8c929df..c1154f90 100644
--- a/testes/tracegc.lua
+++ b/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