From 0ceada8da92135717d31a3954b5b89a954f9e71a Mon Sep 17 00:00:00 2001 From: Roberto Ierusalimschy Date: Tue, 22 Dec 2020 10:54:25 -0300 Subject: [PATCH] Report last error in closing methods When there are multiple errors around closing methods, report the last error instead of the original. --- lcorolib.c | 7 +++- lfunc.c | 10 ++--- manual/manual.of | 5 --- testes/coroutine.lua | 15 ++----- testes/locals.lua | 99 +++++++++++--------------------------------- 5 files changed, 35 insertions(+), 101 deletions(-) diff --git a/lcorolib.c b/lcorolib.c index c165031d..ed7c58b2 100644 --- a/lcorolib.c +++ b/lcorolib.c @@ -75,8 +75,11 @@ static int luaB_auxwrap (lua_State *L) { int r = auxresume(L, co, lua_gettop(L)); if (r < 0) { /* error? */ int stat = lua_status(co); - if (stat != LUA_OK && stat != LUA_YIELD) /* error in the coroutine? */ - lua_resetthread(co); /* close its tbc variables */ + if (stat != LUA_OK && stat != LUA_YIELD) { /* error in the coroutine? */ + stat = lua_resetthread(co); /* close its tbc variables */ + lua_assert(stat != LUA_OK); + lua_xmove(co, L, 1); /* copy error message */ + } if (stat != LUA_ERRMEM && /* not a memory error and ... */ lua_type(L, -1) == LUA_TSTRING) { /* ... error object is a string? */ luaL_where(L, 1); /* add extra info, if available */ diff --git a/lfunc.c b/lfunc.c index 6608592b..bfbf270b 100644 --- a/lfunc.c +++ b/lfunc.c @@ -162,14 +162,10 @@ static int callclosemth (lua_State *L, StkId level, int status) { luaD_seterrorobj(L, status, level); /* set error message */ if (prepclosingmethod(L, uv, s2v(level))) { /* something to call? */ int newstatus = luaD_pcall(L, callclose, NULL, oldtop, 0); - if (newstatus != LUA_OK && status == CLOSEPROTECT) /* first error? */ - status = newstatus; /* this will be the new error */ - else { - if (newstatus != LUA_OK) /* suppressed error? */ - luaE_warnerror(L, "__close metamethod"); - /* leave original error (or nil) on top */ + if (newstatus != LUA_OK) /* new error? */ + status = newstatus; /* this will be the error now */ + else /* leave original error (or nil) on top */ L->top = restorestack(L, oldtop); - } } /* else no metamethod; ignore this case and keep original error */ } diff --git a/manual/manual.of b/manual/manual.of index 5d0c35cf..c5385258 100644 --- a/manual/manual.of +++ b/manual/manual.of @@ -1630,13 +1630,8 @@ they are closed in the reverse order that they were declared. If there is any error while running a closing method, that error is handled like an error in the regular code where the variable was defined. - After an error, the other pending closing methods will still be called. -Errors in these methods -interrupt the respective method and generate a warning, -but are otherwise ignored; -the error reported is only the original one. If a coroutine yields and is never resumed again, some variables may never go out of scope, diff --git a/testes/coroutine.lua b/testes/coroutine.lua index aaf565fb..0a970e98 100644 --- a/testes/coroutine.lua +++ b/testes/coroutine.lua @@ -172,13 +172,12 @@ do assert(not X and coroutine.status(co) == "dead") -- error closing a coroutine - warn("@on") local x = 0 co = coroutine.create(function() local y = func2close(function (self,err) - if (err ~= 111) then os.exit(false) end -- should not happen + assert(err == 111) x = 200 - error("200") + error(200) end) local x = func2close(function (self, err) assert(err == nil); error(111) @@ -187,16 +186,8 @@ do end) coroutine.resume(co) assert(x == 0) - -- with test library, use 'store' mode to check warnings - warn(not T and "@off" or "@store") local st, msg = coroutine.close(co) - if not T then - warn("@on") - else -- test library - assert(string.find(_WARN, "200")); _WARN = false - warn("@normal") - end - assert(st == false and coroutine.status(co) == "dead" and msg == 111) + assert(st == false and coroutine.status(co) == "dead" and msg == 200) assert(x == 200) end diff --git a/testes/locals.lua b/testes/locals.lua index d32a9a3e..84e0b03a 100644 --- a/testes/locals.lua +++ b/testes/locals.lua @@ -232,7 +232,11 @@ end do local X = false - local x, closescope = func2close(function () stack(10); X = true end, 100) + local x, closescope = func2close(function (_, msg) + stack(10); + assert(msg == nil) + X = true + end, 100) assert(x == 100); x = 101; -- 'x' is not read-only -- closing functions do not corrupt returning values @@ -246,10 +250,11 @@ do X = false foo = function (x) - local _ = func2close(function () + local _ = func2close(function (_, msg) -- without errors, enclosing function should be still active when -- __close is called assert(debug.getinfo(2).name == "foo") + assert(msg == nil) end) local _ = closescope local y = 15 @@ -328,64 +333,20 @@ do end --- auxiliary functions for testing warnings in '__close' -local function prepwarn () - if not T then -- no test library? - warn("@off") -- do not show (lots of) warnings - else - warn("@store") -- to test the warnings - end -end - - -local function endwarn () - if not T then - warn("@on") -- back to normal - else - assert(_WARN == false) - warn("@normal") - end -end - - --- errors inside __close can generate a warning instead of an --- error. This new 'assert' force them to appear. -local function assert(cond, msg) - if not cond then - local line = debug.getinfo(2).currentline or "?" - msg = string.format("assertion failed! line %d (%s)\n", line, msg or "") - io.stderr:write(msg) - os.exit(1) - end -end - - -local function checkwarn (msg) - if T then - assert(_WARN and string.find(_WARN, msg)) - _WARN = false -- reset variable to check next warning - end -end - -warn("@on") - do print("testing errors in __close") - prepwarn() - -- original error is in __close local function foo () local x = func2close(function (self, msg) - assert(string.find(msg, "@z")) + assert(string.find(msg, "@y")) error("@x") end) local x1 = func2close(function (self, msg) - checkwarn("@y") - assert(string.find(msg, "@z")) + assert(string.find(msg, "@y")) end) local gc = func2close(function () collectgarbage() end) @@ -406,8 +367,7 @@ do print("testing errors in __close") end local stat, msg = pcall(foo, false) - assert(string.find(msg, "@z")) - checkwarn("@x") + assert(string.find(msg, "@x")) -- original error not in __close @@ -418,14 +378,13 @@ do print("testing errors in __close") -- after error, 'foo' was discarded, so caller now -- must be 'pcall' assert(debug.getinfo(2).name == "pcall") - assert(msg == 4) + assert(string.find(msg, "@x1")) end) local x1 = func2close(function (self, msg) assert(debug.getinfo(2).name == "pcall") - checkwarn("@y") - assert(msg == 4) + assert(string.find(msg, "@y")) error("@x1") end) @@ -434,8 +393,7 @@ do print("testing errors in __close") local y = func2close(function (self, msg) assert(debug.getinfo(2).name == "pcall") - assert(msg == 4) -- error in body - checkwarn("@z") + assert(string.find(msg, "@z")) error("@y") end) @@ -453,8 +411,7 @@ do print("testing errors in __close") end local stat, msg = pcall(foo, true) - assert(msg == 4) - checkwarn("@x1") -- last error + assert(string.find(msg, "@x1")) -- error leaving a block local function foo (...) @@ -466,7 +423,8 @@ do print("testing errors in __close") end) local x123 = - func2close(function () + func2close(function (_, msg) + assert(msg == nil) error("@X") end) end @@ -474,9 +432,7 @@ do print("testing errors in __close") end local st, msg = xpcall(foo, debug.traceback) - assert(string.match(msg, "^[^ ]* @X")) - assert(string.find(msg, "in metamethod 'close'")) - checkwarn("@Y") + assert(string.match(msg, "^[^ ]* @Y")) -- error in toclose in vararg function local function foo (...) @@ -486,7 +442,6 @@ do print("testing errors in __close") local st, msg = xpcall(foo, debug.traceback) assert(string.match(msg, "^[^ ]* @x123")) assert(string.find(msg, "in metamethod 'close'")) - endwarn() end @@ -511,8 +466,6 @@ end if rawget(_G, "T") then - warn("@off") - -- memory error inside closing function local function foo () local y = func2close(function () T.alloccount() end) @@ -527,7 +480,7 @@ if rawget(_G, "T") then -- despite memory error, 'y' will be executed and -- memory limit will be lifted local _, msg = pcall(foo) - assert(msg == 1000) + assert(msg == "not enough memory") local close = func2close(function (self, msg) T.alloccount() @@ -570,7 +523,7 @@ if rawget(_G, "T") then end local _, msg = pcall(test) - assert(msg == "not enough memory") -- reported error is the first one + assert(msg == 1000) do -- testing 'toclose' in C string buffer collectgarbage() @@ -625,7 +578,6 @@ if rawget(_G, "T") then print'+' end - warn("@on") end @@ -655,14 +607,14 @@ end do - prepwarn() -- error in a wrapped coroutine raising errors when closing a variable local x = 0 local co = coroutine.wrap(function () - local xx = func2close(function () + local xx = func2close(function (_, msg) x = x + 1; - checkwarn("@XXX"); error("@YYY") + assert(string.find(msg, "@XXX")) + error("@YYY") end) local xv = func2close(function () x = x + 1; error("@XXX") end) coroutine.yield(100) @@ -670,8 +622,7 @@ do end) assert(co() == 100); assert(x == 0) local st, msg = pcall(co); assert(x == 2) - assert(not st and msg == 200) -- should get first error raised - checkwarn("@YYY") + assert(not st and string.find(msg, "@YYY")) -- should get error raised local x = 0 local y = 0 @@ -691,10 +642,8 @@ do local st, msg = pcall(co) assert(x == 1 and y == 1) -- should get first error raised - assert(not st and string.find(msg, "%w+%.%w+:%d+: XXX")) - checkwarn("YYY") + assert(not st and string.find(msg, "%w+%.%w+:%d+: YYY")) - endwarn() end