From 7696c6474fe51ed59fee324e78c1233af74febdd Mon Sep 17 00:00:00 2001 From: Roberto Ierusalimschy Date: Mon, 26 Nov 2018 14:16:17 -0200 Subject: [PATCH] Auxiliary buffer cannot close box with 'lua_remove' To remove a to-be-closed variable from the stack in the C API a function must use 'lua_settop' or 'lua_pop'. Previous implementation of 'luaL_pushresult' was not closing the box. (This commit also added tests to check that box is being closed "as soon as possible".) --- lapi.c | 4 ++-- lauxlib.c | 12 +++++----- testes/locals.lua | 56 +++++++++++++++++++++++++++++++++-------------- 3 files changed, 49 insertions(+), 23 deletions(-) diff --git a/lapi.c b/lapi.c index da866a66..147ed0ff 100644 --- a/lapi.c +++ b/lapi.c @@ -1213,8 +1213,8 @@ LUA_API void lua_toclose (lua_State *L, int idx) { lua_lock(L); o = index2stack(L, idx); nresults = L->ci->nresults; - api_check(L, L->openupval == NULL || uplevel(L->openupval) < o, - "there is an already marked index below"); + api_check(L, L->openupval == NULL || uplevel(L->openupval) <= o, + "marked index below or equal new one"); luaF_newtbcupval(L, o); /* create new to-be-closed upvalue */ if (!hastocloseCfunc(nresults)) /* function not marked yet? */ L->ci->nresults = codeNresults(nresults); /* mark it */ diff --git a/lauxlib.c b/lauxlib.c index 6069ed1b..fd4acbd1 100644 --- a/lauxlib.c +++ b/lauxlib.c @@ -524,8 +524,8 @@ static size_t newbuffsize (luaL_Buffer *B, size_t sz) { /* ** Returns a pointer to a free area with at least 'sz' bytes in buffer -** 'B'. 'boxidx' is the position in the stack where the buffer's box is -** or should be. +** 'B'. 'boxidx' is the relative position in the stack where the +** buffer's box is or should be. */ static char *prepbuffsize (luaL_Buffer *B, size_t sz, int boxidx) { if (B->size - B->n >= sz) /* enough space? */ @@ -538,8 +538,10 @@ static char *prepbuffsize (luaL_Buffer *B, size_t sz, int boxidx) { if (buffonstack(B)) /* buffer already has a box? */ newbuff = (char *)resizebox(L, boxidx, newsize); /* resize it */ else { /* no box yet */ + lua_pushnil(L); /* reserve slot for final result */ newbox(L); /* create a new box */ - lua_insert(L, boxidx); /* move box to its intended position */ + /* move box (and slot) to its intended position */ + lua_rotate(L, boxidx - 1, 2); lua_toclose(L, boxidx); newbuff = (char *)resizebox(L, boxidx, newsize); memcpy(newbuff, B->b, B->n * sizeof(char)); /* copy original content */ @@ -576,8 +578,8 @@ LUALIB_API void luaL_pushresult (luaL_Buffer *B) { lua_State *L = B->L; lua_pushlstring(L, B->b, B->n); if (buffonstack(B)) { - resizebox(L, -2, 0); /* delete old buffer */ - lua_remove(L, -2); /* remove its header from the stack */ + lua_copy(L, -1, -3); /* move string to reserved slot */ + lua_pop(L, 2); /* pop string and box (closing the box) */ } } diff --git a/testes/locals.lua b/testes/locals.lua index 8766b3d5..90a8b845 100644 --- a/testes/locals.lua +++ b/testes/locals.lua @@ -242,6 +242,7 @@ do assert(a == 1 and b == 2 and c == 3 and X == 20 and Y == 10 and d == nil) end + do -- errors in __close local log = {} local function foo (err) @@ -264,7 +265,9 @@ do -- errors in __close and #log == 4) end + if rawget(_G, "T") then + -- memory error inside closing function local function foo () local *toclose y = function () T.alloccount() end @@ -296,7 +299,7 @@ if rawget(_G, "T") then local function test () local *toclose x = enter(0) -- set a memory limit -- creation of previous upvalue will raise a memory error - os.exit(false) -- should not run + assert(false) -- should not run end local _, msg = pcall(test) @@ -326,33 +329,54 @@ if rawget(_G, "T") then do -- testing 'toclose' in C string buffer - local s = string.rep("a", 10000) + collectgarbage() + local s = string.rep('a', 10000) -- large string + local m = T.totalmem() + collectgarbage("stop") + s = string.upper(s) -- allocate buffer + new string (10K each) + -- ensure buffer was deallocated + assert(T.totalmem() - m <= 11000) + collectgarbage("restart") + end + + do -- now some tests for freeing buffer in case of errors + local lim = 10000 -- some size larger than the static buffer + local extra = 2000 -- some extra memory (for callinfo, etc.) + + local s = string.rep("a", lim) + + -- concat this table needs two buffer resizes (one for each 's') local a = {s, s} - -- ensure proper initialization (stack space, metatable) - table.concat(a) - collectgarbage(); collectgarbage() + collectgarbage() - local m = T.totalmem() + m = T.totalmem() + collectgarbage("stop") + + -- error in the first buffer allocation + T. totalmem(m + extra) + assert(not pcall(table.concat, a)) + -- first buffer was not even allocated + assert(T.totalmem() - m <= extra) -- error in the second buffer allocation - T.alloccount(3) + T. totalmem(m + lim + extra) assert(not pcall(table.concat, a)) - T.alloccount() -- first buffer was released by 'toclose' - assert(T.totalmem() - m <= 5000) + assert(T.totalmem() - m <= extra) -- error in creation of final string - T.alloccount(4) + T.totalmem(m + 2 * lim + extra) assert(not pcall(table.concat, a)) - T.alloccount() -- second buffer was released by 'toclose' - assert(T.totalmem() - m <= 5000) + assert(T.totalmem() - m <= extra) - -- userdata, upvalue, buffer, buffer, string - T.alloccount(5) - assert(#table.concat(a) == 20000) - T.alloccount() + -- userdata, upvalue, buffer, buffer, final string + T.totalmem(m + 4*lim + extra) + assert(#table.concat(a) == 2*lim) + + T.totalmem(0) -- remove memory limit + collectgarbage("restart") print'+' end