Browse Source

Report last error in closing methods

When there are multiple errors around closing methods, report the
last error instead of the original.
Roberto Ierusalimschy 4 years ago
parent
commit
0ceada8da9
5 changed files with 35 additions and 101 deletions
  1. 5 2
      lcorolib.c
  2. 3 7
      lfunc.c
  3. 0 5
      manual/manual.of
  4. 3 12
      testes/coroutine.lua
  5. 24 75
      testes/locals.lua

+ 5 - 2
lcorolib.c

@@ -75,8 +75,11 @@ static int luaB_auxwrap (lua_State *L) {
   int r = auxresume(L, co, lua_gettop(L));
   int r = auxresume(L, co, lua_gettop(L));
   if (r < 0) {  /* error? */
   if (r < 0) {  /* error? */
     int stat = lua_status(co);
     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 ... */
     if (stat != LUA_ERRMEM &&  /* not a memory error and ... */
         lua_type(L, -1) == LUA_TSTRING) {  /* ... error object is a string? */
         lua_type(L, -1) == LUA_TSTRING) {  /* ... error object is a string? */
       luaL_where(L, 1);  /* add extra info, if available */
       luaL_where(L, 1);  /* add extra info, if available */

+ 3 - 7
lfunc.c

@@ -162,14 +162,10 @@ static int callclosemth (lua_State *L, StkId level, int status) {
     luaD_seterrorobj(L, status, level);  /* set error message */
     luaD_seterrorobj(L, status, level);  /* set error message */
     if (prepclosingmethod(L, uv, s2v(level))) {  /* something to call? */
     if (prepclosingmethod(L, uv, s2v(level))) {  /* something to call? */
       int newstatus = luaD_pcall(L, callclose, NULL, oldtop, 0);
       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);
         L->top = restorestack(L, oldtop);
-      }
     }
     }
     /* else no metamethod; ignore this case and keep original error */
     /* else no metamethod; ignore this case and keep original error */
   }
   }

+ 0 - 5
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,
 If there is any error while running a closing method,
 that error is handled like an error in the regular code
 that error is handled like an error in the regular code
 where the variable was defined.
 where the variable was defined.
-
 After an error,
 After an error,
 the other pending closing methods will still be called.
 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,
 If a coroutine yields and is never resumed again,
 some variables may never go out of scope,
 some variables may never go out of scope,

+ 3 - 12
testes/coroutine.lua

@@ -172,13 +172,12 @@ do
   assert(not X and coroutine.status(co) == "dead")
   assert(not X and coroutine.status(co) == "dead")
 
 
   -- error closing a coroutine
   -- error closing a coroutine
-  warn("@on")
   local x = 0
   local x = 0
   co = coroutine.create(function()
   co = coroutine.create(function()
     local y <close> = func2close(function (self,err)
     local y <close> = func2close(function (self,err)
-      if (err ~= 111) then os.exit(false) end   -- should not happen
+      assert(err == 111)
       x = 200
       x = 200
-      error("200")
+      error(200)
     end)
     end)
     local x <close> = func2close(function (self, err)
     local x <close> = func2close(function (self, err)
       assert(err == nil); error(111)
       assert(err == nil); error(111)
@@ -187,16 +186,8 @@ do
   end)
   end)
   coroutine.resume(co)
   coroutine.resume(co)
   assert(x == 0)
   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)
   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)
   assert(x == 200)
 
 
 end
 end

+ 24 - 75
testes/locals.lua

@@ -232,7 +232,11 @@ end
 do
 do
   local X = false
   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
   assert(x == 100);  x = 101;   -- 'x' is not read-only
 
 
   -- closing functions do not corrupt returning values
   -- closing functions do not corrupt returning values
@@ -246,10 +250,11 @@ do
 
 
   X = false
   X = false
   foo = function (x)
   foo = function (x)
-    local _<close> = func2close(function ()
+    local _<close> = func2close(function (_, msg)
       -- without errors, enclosing function should be still active when
       -- without errors, enclosing function should be still active when
       -- __close is called
       -- __close is called
       assert(debug.getinfo(2).name == "foo")
       assert(debug.getinfo(2).name == "foo")
+      assert(msg == nil)
     end)
     end)
     local  _<close> = closescope
     local  _<close> = closescope
     local y = 15
     local y = 15
@@ -328,64 +333,20 @@ do
 end
 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")
 do print("testing errors in __close")
 
 
-  prepwarn()
-
   -- original error is in __close
   -- original error is in __close
   local function foo ()
   local function foo ()
 
 
     local x <close> =
     local x <close> =
       func2close(function (self, msg)
       func2close(function (self, msg)
-        assert(string.find(msg, "@z"))
+        assert(string.find(msg, "@y"))
         error("@x")
         error("@x")
       end)
       end)
 
 
     local x1 <close> =
     local x1 <close> =
       func2close(function (self, msg)
       func2close(function (self, msg)
-        checkwarn("@y")
-        assert(string.find(msg, "@z"))
+        assert(string.find(msg, "@y"))
       end)
       end)
 
 
     local gc <close> = func2close(function () collectgarbage() end)
     local gc <close> = func2close(function () collectgarbage() end)
@@ -406,8 +367,7 @@ do print("testing errors in __close")
   end
   end
 
 
   local stat, msg = pcall(foo, false)
   local stat, msg = pcall(foo, false)
-  assert(string.find(msg, "@z"))
-  checkwarn("@x")
+  assert(string.find(msg, "@x"))
 
 
 
 
   -- original error not in __close
   -- original error not in __close
@@ -418,14 +378,13 @@ do print("testing errors in __close")
         -- after error, 'foo' was discarded, so caller now
         -- after error, 'foo' was discarded, so caller now
         -- must be 'pcall'
         -- must be 'pcall'
         assert(debug.getinfo(2).name == "pcall")
         assert(debug.getinfo(2).name == "pcall")
-        assert(msg == 4)
+        assert(string.find(msg, "@x1"))
       end)
       end)
 
 
     local x1 <close> =
     local x1 <close> =
       func2close(function (self, msg)
       func2close(function (self, msg)
         assert(debug.getinfo(2).name == "pcall")
         assert(debug.getinfo(2).name == "pcall")
-        checkwarn("@y")
-        assert(msg == 4)
+        assert(string.find(msg, "@y"))
         error("@x1")
         error("@x1")
       end)
       end)
 
 
@@ -434,8 +393,7 @@ do print("testing errors in __close")
     local y <close> =
     local y <close> =
       func2close(function (self, msg)
       func2close(function (self, msg)
         assert(debug.getinfo(2).name == "pcall")
         assert(debug.getinfo(2).name == "pcall")
-        assert(msg == 4)   -- error in body
-        checkwarn("@z")
+        assert(string.find(msg, "@z"))
         error("@y")
         error("@y")
       end)
       end)
 
 
@@ -453,8 +411,7 @@ do print("testing errors in __close")
   end
   end
 
 
   local stat, msg = pcall(foo, true)
   local stat, msg = pcall(foo, true)
-  assert(msg == 4)
-  checkwarn("@x1")   -- last error
+  assert(string.find(msg, "@x1"))
 
 
   -- error leaving a block
   -- error leaving a block
   local function foo (...)
   local function foo (...)
@@ -466,7 +423,8 @@ do print("testing errors in __close")
         end)
         end)
 
 
       local x123 <close> =
       local x123 <close> =
-        func2close(function ()
+        func2close(function (_, msg)
+          assert(msg == nil)
           error("@X")
           error("@X")
         end)
         end)
     end
     end
@@ -474,9 +432,7 @@ do print("testing errors in __close")
   end
   end
 
 
   local st, msg = xpcall(foo, debug.traceback)
   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
   -- error in toclose in vararg function
   local function foo (...)
   local function foo (...)
@@ -486,7 +442,6 @@ do print("testing errors in __close")
   local st, msg = xpcall(foo, debug.traceback)
   local st, msg = xpcall(foo, debug.traceback)
   assert(string.match(msg, "^[^ ]* @x123"))
   assert(string.match(msg, "^[^ ]* @x123"))
   assert(string.find(msg, "in metamethod 'close'"))
   assert(string.find(msg, "in metamethod 'close'"))
-  endwarn()
 end
 end
 
 
 
 
@@ -511,8 +466,6 @@ end
 
 
 if rawget(_G, "T") then
 if rawget(_G, "T") then
 
 
-  warn("@off")
-
   -- memory error inside closing function
   -- memory error inside closing function
   local function foo ()
   local function foo ()
     local y <close> = func2close(function () T.alloccount() end)
     local y <close> = func2close(function () T.alloccount() end)
@@ -527,7 +480,7 @@ if rawget(_G, "T") then
   -- despite memory error, 'y' will be executed and
   -- despite memory error, 'y' will be executed and
   -- memory limit will be lifted
   -- memory limit will be lifted
   local _, msg = pcall(foo)
   local _, msg = pcall(foo)
-  assert(msg == 1000)
+  assert(msg == "not enough memory")
 
 
   local close = func2close(function (self, msg)
   local close = func2close(function (self, msg)
     T.alloccount()
     T.alloccount()
@@ -570,7 +523,7 @@ if rawget(_G, "T") then
   end
   end
 
 
   local _, msg = pcall(test)
   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
   do    -- testing 'toclose' in C string buffer
     collectgarbage()
     collectgarbage()
@@ -625,7 +578,6 @@ if rawget(_G, "T") then
     print'+'
     print'+'
   end
   end
 
 
-  warn("@on")
 end
 end
 
 
 
 
@@ -655,14 +607,14 @@ end
 
 
 
 
 do
 do
-  prepwarn()
 
 
   -- error in a wrapped coroutine raising errors when closing a variable
   -- error in a wrapped coroutine raising errors when closing a variable
   local x = 0
   local x = 0
   local co = coroutine.wrap(function ()
   local co = coroutine.wrap(function ()
-    local xx <close> = func2close(function ()
+    local xx <close> = func2close(function (_, msg)
       x = x + 1;
       x = x + 1;
-      checkwarn("@XXX"); error("@YYY")
+      assert(string.find(msg, "@XXX"))
+      error("@YYY")
     end)
     end)
     local xv <close> = func2close(function () x = x + 1; error("@XXX") end)
     local xv <close> = func2close(function () x = x + 1; error("@XXX") end)
     coroutine.yield(100)
     coroutine.yield(100)
@@ -670,8 +622,7 @@ do
   end)
   end)
   assert(co() == 100); assert(x == 0)
   assert(co() == 100); assert(x == 0)
   local st, msg = pcall(co); assert(x == 2)
   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 x = 0
   local y = 0
   local y = 0
@@ -691,10 +642,8 @@ do
   local st, msg = pcall(co)
   local st, msg = pcall(co)
   assert(x == 1 and y == 1)
   assert(x == 1 and y == 1)
   -- should get first error raised
   -- 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
 end