Upvalues removed from 'openupval' before being closed

Undo commit c220b0a5d0: '__close' is not called again in case of
errors. (Upvalue is removed from the list before the call.) The
common error that justified that change was C stack overflows, which
are much rarer with the stackless implementation.

3 files changed, 31 insertions, 26 deletions

diff --git a/lfunc.c b/lfunc.c
index c4360f09..6608592b 100644
--- a/lfunc.c
+++ b/lfunc.c
@@ -220,24 +220,38 @@ void luaF_unlinkupval (UpVal *uv) {
 }
 
 
+/*
+** Close all upvalues up to the given stack level. 'status' indicates
+** how/why the function was called:
+** - LUA_OK: regular code exiting the scope of a variable; may raise
+** an error due to errors in __close metamethods;
+** - CLOSEPROTECT: finishing a thread; run all metamethods in protected
+** mode;
+** - NOCLOSINGMETH: close upvalues without running __close metamethods;
+** - other values: error status from previous errors, to be propagated.
+**
+** Returns the resulting status, either the original status or an error
+** in a closing method.
+*/
 int luaF_close (lua_State *L, StkId level, int status) {
   UpVal *uv;
-  while ((uv = L->openupval) != NULL && uplevel(uv) >= level) {
+  StkId upl;  /* stack index pointed by 'uv' */
+  while ((uv = L->openupval) != NULL && (upl = uplevel(uv)) >= level) {
     TValue *slot = &uv->u.value;  /* new position for value */
     lua_assert(uplevel(uv) < L->top);
-    if (uv->tbc && status != NOCLOSINGMETH) {
-      /* must run closing method, which may change the stack */
-      ptrdiff_t levelrel = savestack(L, level);
-      status = callclosemth(L, uplevel(uv), status);
-      level = restorestack(L, levelrel);
-    }
-    luaF_unlinkupval(uv);
+    luaF_unlinkupval(uv);  /* remove upvalue from 'openupval' list */
     setobj(L, slot, uv->v);  /* move value to upvalue slot */
     uv->v = slot;  /* now current value lives here */
     if (!iswhite(uv)) {  /* neither white nor dead? */
       nw2black(uv);  /* closed upvalues cannot be gray */
       luaC_barrier(L, uv, slot);
     }
+    if (uv->tbc && status != NOCLOSINGMETH) {
+      /* must run closing method, which may change the stack */
+      ptrdiff_t levelrel = savestack(L, level);
+      status = callclosemth(L, upl, status);
+      level = restorestack(L, levelrel);
+    }
   }
   return status;
 }
diff --git a/manual/manual.of b/manual/manual.of
index 164e359a..5d0c35cf 100644
--- a/manual/manual.of
+++ b/manual/manual.of
@@ -1630,7 +1630,6 @@ 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.
-However, Lua may call the method one more time.
 
 After an error,
 the other pending closing methods will still be called.
diff --git a/testes/locals.lua b/testes/locals.lua
index e2f6f35c..d32a9a3e 100644
--- a/testes/locals.lua
+++ b/testes/locals.lua
@@ -392,21 +392,13 @@ do print("testing errors in __close")
 
     local y <close> =
       func2close(function (self, msg)
-        assert(string.find(msg, "@z"))  -- first error in 'z'
-        checkwarn("@z")   -- second error in 'z' generated a warning
+        assert(string.find(msg, "@z"))  -- error in 'z'
         error("@y")
       end)
 
-    local first = true
     local z <close> =
-      -- 'z' close is called twice
       func2close(function (self, msg)
-        if first then
-          assert(msg == nil)
-          first = false
-        else
-         assert(string.find(msg, "@z"))   -- own error
-        end
+        assert(msg == nil)
         error("@z")
       end)
 
@@ -468,8 +460,8 @@ do print("testing errors in __close")
   local function foo (...)
     do
       local x1 <close> =
-        func2close(function ()
-          checkwarn("@X")
+        func2close(function (self, msg)
+          assert(string.find(msg, "@X"))
           error("@Y")
         end)
 
@@ -494,8 +486,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'"))
-  checkwarn("@x123")   -- from second call to close 'x123'
-
   endwarn()
 end
 
@@ -686,8 +676,10 @@ do
   local x = 0
   local y = 0
   co = coroutine.wrap(function ()
-    local xx <close> = func2close(function ()
-      y = y + 1; checkwarn("XXX"); error("YYY")
+    local xx <close> = func2close(function (_, err)
+      y = y + 1;
+      assert(string.find(err, "XXX"))
+      error("YYY")
     end)
     local xv <close> = func2close(function ()
       x = x + 1; error("XXX")
@@ -697,7 +689,7 @@ do
   end)
   assert(co() == 100); assert(x == 0)
   local st, msg = pcall(co)
-  assert(x == 2 and y == 1)   -- first close is called twice
+  assert(x == 1 and y == 1)
   -- should get first error raised
   assert(not st and string.find(msg, "%w+%.%w+:%d+: XXX"))
   checkwarn("YYY")