Skip to content

Commit 2a7cf4f

Browse files
roberto-ieruroberto-ieru
committed
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.
1 parent 5cfc725 commit 2a7cf4f

8 files changed

Lines changed: 118 additions & 16 deletions

File tree

‎ldo.c‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -221,13 +221,21 @@ l_noret luaD_errerr (lua_State *L) {
221221

222222

223223
/*
224-
** Check whether stack has enough space to run a simple function (such
225-
** as a finalizer): At least BASIC_STACK_SIZE in the Lua stack and
226-
** 2 slots in the C stack.
224+
** Check whether stacks have enough space to run a simple function (such
225+
** as a finalizer): At least BASIC_STACK_SIZE in the Lua stack, two
226+
** available CallInfos, and two "slots" in the C stack.
227227
*/
228228
int luaD_checkminstack (lua_State *L) {
229-
return ((stacksize(L) < MAXSTACK - BASIC_STACK_SIZE) &&
230-
(getCcalls(L) < LUAI_MAXCCALLS - 2));
229+
if (getCcalls(L) >= LUAI_MAXCCALLS - 2)
230+
return 0; /* not enough C-stack slots */
231+
if (L->ci->next == NULL && luaE_extendCI(L, 0) == NULL)
232+
return 0; /* unable to allocate first ci */
233+
if (L->ci->next->next == NULL && luaE_extendCI(L, 0) == NULL)
234+
return 0; /* unable to allocate second ci */
235+
if (L->stack_last.p - L->top.p >= BASIC_STACK_SIZE)
236+
return 1; /* enough (BASIC_STACK_SIZE) free slots in the Lua stack */
237+
else /* try to grow stack to a size with enough free slots */
238+
return luaD_growstack(L, BASIC_STACK_SIZE, 0);
231239
}
232240

233241

@@ -616,7 +624,7 @@ void luaD_poscall (lua_State *L, CallInfo *ci, int nres) {
616624

617625

618626

619-
#define next_ci(L) (L->ci->next ? L->ci->next : luaE_extendCI(L))
627+
#define next_ci(L) (L->ci->next ? L->ci->next : luaE_extendCI(L, 1))
620628

621629

622630
/*

‎lgc.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1293,7 +1293,7 @@ static void finishgencycle (lua_State *L, global_State *g) {
12931293
correctgraylists(g);
12941294
checkSizes(L, g);
12951295
g->gcstate = GCSpropagate; /* skip restart */
1296-
if (!g->gcemergency && luaD_checkminstack(L))
1296+
if (g->tobefnz != NULL && !g->gcemergency && luaD_checkminstack(L))
12971297
callallpendingfinalizers(L);
12981298
}
12991299

‎lstate.c‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,19 @@ void luaE_setdebt (global_State *g, l_mem debt) {
6868
}
6969

7070

71-
CallInfo *luaE_extendCI (lua_State *L) {
71+
CallInfo *luaE_extendCI (lua_State *L, int err) {
7272
CallInfo *ci;
73-
lua_assert(L->ci->next == NULL);
74-
ci = luaM_new(L, CallInfo);
75-
lua_assert(L->ci->next == NULL);
76-
L->ci->next = ci;
73+
ci = luaM_reallocvector(L, NULL, 0, 1, CallInfo);
74+
if (l_unlikely(ci == NULL)) { /* allocation failed? */
75+
if (err)
76+
luaM_error(L); /* raise the error */
77+
return NULL; /* else only report it */
78+
}
79+
ci->next = L->ci->next;
7780
ci->previous = L->ci;
78-
ci->next = NULL;
81+
L->ci->next = ci;
82+
if (ci->next)
83+
ci->next->previous = ci;
7984
ci->u.l.trap = 0;
8085
L->nci++;
8186
return ci;

‎lstate.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,7 @@ union GCUnion {
438438
LUAI_FUNC void luaE_setdebt (global_State *g, l_mem debt);
439439
LUAI_FUNC void luaE_freethread (lua_State *L, lua_State *L1);
440440
LUAI_FUNC lu_mem luaE_threadsize (lua_State *L);
441-
LUAI_FUNC CallInfo *luaE_extendCI (lua_State *L);
441+
LUAI_FUNC CallInfo *luaE_extendCI (lua_State *L, int err);
442442
LUAI_FUNC void luaE_shrinkCI (lua_State *L);
443443
LUAI_FUNC void luaE_checkcstack (lua_State *L);
444444
LUAI_FUNC void luaE_incCstack (lua_State *L);

‎ltests.c‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1106,6 +1106,27 @@ static int stacklevel (lua_State *L) {
11061106
}
11071107

11081108

1109+
static int resetCI (lua_State *L) {
1110+
CallInfo *ci = L->ci;
1111+
while (ci->next != NULL) {
1112+
CallInfo *tofree = ci->next;
1113+
ci->next = ci->next->next;
1114+
luaM_free(L, tofree);
1115+
L->nci--;
1116+
}
1117+
return 0;
1118+
}
1119+
1120+
1121+
static int reallocstack (lua_State *L) {
1122+
int n = cast_int(luaL_checkinteger(L, 1));
1123+
lua_lock(L);
1124+
luaD_reallocstack(L, cast_int(L->top.p - L->stack.p) + n, 1);
1125+
lua_unlock(L);
1126+
return 0;
1127+
}
1128+
1129+
11091130
static int table_query (lua_State *L) {
11101131
const Table *t;
11111132
int i = cast_int(luaL_optinteger(L, 2, -1));
@@ -2182,6 +2203,8 @@ static const struct luaL_Reg tests_funcs[] = {
21822203
{"s2d", s2d},
21832204
{"sethook", sethook},
21842205
{"stacklevel", stacklevel},
2206+
{"resetCI", resetCI},
2207+
{"reallocstack", reallocstack},
21852208
{"sizes", get_sizes},
21862209
{"testC", testC},
21872210
{"makeCfunc", makeCfunc},

‎testes/gc.lua‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -707,4 +707,46 @@ end
707707

708708
collectgarbage(oldmode)
709709

710+
711+
if T then
712+
print("testing stack issues when calling finalizers")
713+
714+
local X
715+
local obj
716+
717+
local function initobj ()
718+
X = false
719+
obj = setmetatable({}, {__gc = function () X = true end})
720+
end
721+
722+
local function loop (n)
723+
if n > 0 then loop(n - 1) end
724+
end
725+
726+
-- should not try to call finalizer without a CallInfo available
727+
initobj()
728+
loop(20) -- ensure stack space
729+
T.resetCI() -- remove extra CallInfos
730+
T.alloccount(0) -- cannot allocate more CallInfos
731+
obj = nil
732+
collectgarbage() -- will not call finalizer
733+
T.alloccount()
734+
assert(X == false)
735+
collectgarbage() -- now will call finalizer (it was still pending)
736+
assert(X == true)
737+
738+
-- should not try to call finalizer without stack space available
739+
initobj()
740+
loop(5) -- ensure enough CallInfos
741+
T.reallocstack(0) -- remove extra stack slots
742+
T.alloccount(0) -- cannot reallocate stack
743+
obj = nil
744+
collectgarbage() -- will not call finalizer
745+
T.alloccount()
746+
assert(X == false)
747+
collectgarbage() -- now will call finalizer (it was still pending)
748+
assert(X == true)
749+
end
750+
751+
710752
print('OK')

‎testes/memerr.lua‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,25 @@ testamem("growing stack", function ()
282282
return foo(100)
283283
end)
284284

285+
286+
collectgarbage()
287+
collectgarbage()
288+
global io, T, setmetatable, collectgarbage, print
289+
290+
local Count = 0
291+
testamem("finalizers", function ()
292+
local X = false
293+
local obj = setmetatable({}, {__gc = function () X = true end})
294+
obj = nil
295+
T.resetCI() -- remove extra CallInfos
296+
T.reallocstack(18) -- remove extra stack slots
297+
Count = Count + 1
298+
io.stderr:write(Count, "\n")
299+
T.trick(io)
300+
collectgarbage()
301+
return X
302+
end)
303+
285304
-- }==================================================================
286305

287306

‎testes/tracegc.lua‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,15 @@
11
-- track collections
22

3+
34
local M = {}
45

56
-- import list
6-
local setmetatable, stderr, collectgarbage =
7-
setmetatable, io.stderr, collectgarbage
7+
local stderr, collectgarbage = io.stderr, collectgarbage
8+
9+
-- the debug version of setmetatable does not create any object (such as
10+
-- a '__metatable' string), and so it is more appropriate to be used in
11+
-- a finalizer
12+
local setmetatable = require"debug".setmetatable
813

914
global none
1015

0 commit comments

Comments
 (0)