Sfoglia il codice sorgente

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.
Roberto Ierusalimschy 4 anni fa
parent
commit
f9d29b0c44
3 ha cambiato i file con 31 aggiunte e 26 eliminazioni
  1. 22 8
      lfunc.c
  2. 0 1
      manual/manual.of
  3. 9 17
      testes/locals.lua

+ 22 - 8
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) {
 int luaF_close (lua_State *L, StkId level, int status) {
   UpVal *uv;
   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 */
     TValue *slot = &uv->u.value;  /* new position for value */
     lua_assert(uplevel(uv) < L->top);
     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 */
     setobj(L, slot, uv->v);  /* move value to upvalue slot */
     uv->v = slot;  /* now current value lives here */
     uv->v = slot;  /* now current value lives here */
     if (!iswhite(uv)) {  /* neither white nor dead? */
     if (!iswhite(uv)) {  /* neither white nor dead? */
       nw2black(uv);  /* closed upvalues cannot be gray */
       nw2black(uv);  /* closed upvalues cannot be gray */
       luaC_barrier(L, uv, slot);
       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;
   return status;
 }
 }

+ 0 - 1
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,
 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.
-However, Lua may call the method one more time.
 
 
 After an error,
 After an error,
 the other pending closing methods will still be called.
 the other pending closing methods will still be called.

+ 9 - 17
testes/locals.lua

@@ -392,21 +392,13 @@ do print("testing errors in __close")
 
 
     local y <close> =
     local y <close> =
       func2close(function (self, msg)
       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")
         error("@y")
       end)
       end)
 
 
-    local first = true
     local z <close> =
     local z <close> =
-      -- 'z' close is called twice
       func2close(function (self, msg)
       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")
         error("@z")
       end)
       end)
 
 
@@ -468,8 +460,8 @@ do print("testing errors in __close")
   local function foo (...)
   local function foo (...)
     do
     do
       local x1 <close> =
       local x1 <close> =
-        func2close(function ()
-          checkwarn("@X")
+        func2close(function (self, msg)
+          assert(string.find(msg, "@X"))
           error("@Y")
           error("@Y")
         end)
         end)
 
 
@@ -494,8 +486,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'"))
-  checkwarn("@x123")   -- from second call to close 'x123'
-
   endwarn()
   endwarn()
 end
 end
 
 
@@ -686,8 +676,10 @@ do
   local x = 0
   local x = 0
   local y = 0
   local y = 0
   co = coroutine.wrap(function ()
   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)
     end)
     local xv <close> = func2close(function ()
     local xv <close> = func2close(function ()
       x = x + 1; error("XXX")
       x = x + 1; error("XXX")
@@ -697,7 +689,7 @@ do
   end)
   end)
   assert(co() == 100); assert(x == 0)
   assert(co() == 100); assert(x == 0)
   local st, msg = pcall(co)
   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
   -- should get first error raised
   assert(not st and string.find(msg, "%w+%.%w+:%d+: XXX"))
   assert(not st and string.find(msg, "%w+%.%w+:%d+: XXX"))
   checkwarn("YYY")
   checkwarn("YYY")