Browse Source

To-be-closed variables must be closed on initialization

When initializing a to-be-closed variable, check whether it has a
'__close' metamethod (or is a false value) and raise an error if
if it hasn't. This produces more accurate error messages. (The
check before closing still need to be done: in the C API, the value
is not constant; and the object may lose its '__close' metamethod
during the block.)
Roberto Ierusalimschy 6 years ago
parent
commit
f645d31573
6 changed files with 70 additions and 41 deletions
  1. 32 17
      lfunc.c
  2. 3 0
      ltests.c
  3. 2 4
      lvm.c
  4. 11 8
      manual/manual.of
  5. 10 1
      testes/api.lua
  6. 12 11
      testes/locals.lua

+ 32 - 17
lfunc.c

@@ -123,12 +123,24 @@ static int prepclosingmethod (lua_State *L, TValue *obj, TValue *err) {
 }
 }
 
 
 
 
+/*
+** Raise an error with message 'msg', inserting the name of the
+** local variable at position 'level' in the stack.
+*/
+static void varerror (lua_State *L, StkId level, const char *msg) {
+  int idx = cast_int(level - L->ci->func);
+  const char *vname = luaG_findlocal(L, L->ci, idx, NULL);
+  if (vname == NULL) vname = "?";
+  luaG_runerror(L, msg, vname);
+}
+
+
 /*
 /*
 ** Prepare and call a closing method. If status is OK, code is still
 ** Prepare and call a closing method. If status is OK, code is still
 ** inside the original protected call, and so any error will be handled
 ** inside the original protected call, and so any error will be handled
-** there. Otherwise, a previous error already activated original
+** there. Otherwise, a previous error already activated the original
 ** protected call, and so the call to the closing method must be
 ** protected call, and so the call to the closing method must be
-** protected here. (A status = CLOSEPROTECT behaves like a previous
+** protected here. (A status == CLOSEPROTECT behaves like a previous
 ** error, to also run the closing method in protected mode).
 ** error, to also run the closing method in protected mode).
 ** If status is OK, the call to the closing method will be pushed
 ** If status is OK, the call to the closing method will be pushed
 ** at the top of the stack. Otherwise, values are pushed after
 ** at the top of the stack. Otherwise, values are pushed after
@@ -140,12 +152,8 @@ static int callclosemth (lua_State *L, StkId level, int status) {
   if (likely(status == LUA_OK)) {
   if (likely(status == LUA_OK)) {
     if (prepclosingmethod(L, uv, &G(L)->nilvalue))  /* something to call? */
     if (prepclosingmethod(L, uv, &G(L)->nilvalue))  /* something to call? */
       callclose(L, NULL);  /* call closing method */
       callclose(L, NULL);  /* call closing method */
-    else if (!ttisnil(uv)) {  /* non-closable non-nil value? */
-      int idx = cast_int(level - L->ci->func);
-      const char *vname = luaG_findlocal(L, L->ci, idx, NULL);
-      if (vname == NULL) vname = "?";
-      luaG_runerror(L, "attempt to close non-closable variable '%s'", vname);
-    }
+    else if (!l_isfalse(uv))  /* non-closable non-false value? */
+      varerror(L, level, "attempt to close non-closable variable '%s'");
   }
   }
   else {  /* must close the object in protected mode */
   else {  /* must close the object in protected mode */
     ptrdiff_t oldtop;
     ptrdiff_t oldtop;
@@ -170,9 +178,7 @@ static int callclosemth (lua_State *L, StkId level, int status) {
 ** (can raise a memory-allocation error)
 ** (can raise a memory-allocation error)
 */
 */
 static void trynewtbcupval (lua_State *L, void *ud) {
 static void trynewtbcupval (lua_State *L, void *ud) {
-  StkId level = cast(StkId, ud);
-  lua_assert(L->openupval == NULL || uplevel(L->openupval) < level);
-  newupval(L, 1, level, &L->openupval);
+  newupval(L, 1, cast(StkId, ud), &L->openupval);
 }
 }
 
 
 
 
@@ -182,13 +188,22 @@ static void trynewtbcupval (lua_State *L, void *ud) {
 ** as there is no upvalue to call it later.
 ** as there is no upvalue to call it later.
 */
 */
 void luaF_newtbcupval (lua_State *L, StkId level) {
 void luaF_newtbcupval (lua_State *L, StkId level) {
-  int status = luaD_rawrunprotected(L, trynewtbcupval, level);
-  if (unlikely(status != LUA_OK)) {  /* memory error creating upvalue? */
-    lua_assert(status == LUA_ERRMEM);
-    luaD_seterrorobj(L, LUA_ERRMEM, level + 1);  /* save error message */
-    if (prepclosingmethod(L, s2v(level), s2v(level + 1)))
+  TValue *obj = s2v(level);
+  lua_assert(L->openupval == NULL || uplevel(L->openupval) < level);
+  if (!l_isfalse(obj)) {  /* false doesn't need to be closed */
+    int status;
+    const TValue *tm = luaT_gettmbyobj(L, obj, TM_CLOSE);
+    if (ttisnil(tm))  /* no metamethod? */
+      varerror(L, level, "variable '%s' got a non-closable value");
+    status = luaD_rawrunprotected(L, trynewtbcupval, level);
+    if (unlikely(status != LUA_OK)) {  /* memory error creating upvalue? */
+      lua_assert(status == LUA_ERRMEM);
+      luaD_seterrorobj(L, LUA_ERRMEM, level + 1);  /* save error message */
+      /* next call must succeed, as object is closable */
+      prepclosingmethod(L, s2v(level), s2v(level + 1));
       callclose(L, NULL);  /* call closing method */
       callclose(L, NULL);  /* call closing method */
-    luaD_throw(L, LUA_ERRMEM);  /* throw memory error */
+      luaD_throw(L, LUA_ERRMEM);  /* throw memory error */
+    }
   }
   }
 }
 }
 
 

+ 3 - 0
ltests.c

@@ -1572,6 +1572,9 @@ static int runC (lua_State *L, lua_State *L1, const char *pc) {
     else if EQ("error") {
     else if EQ("error") {
       lua_error(L1);
       lua_error(L1);
     }
     }
+    else if EQ("abort") {
+      abort();
+    }
     else if EQ("throw") {
     else if EQ("throw") {
 #if defined(__cplusplus)
 #if defined(__cplusplus)
 static struct X { int x; } x;
 static struct X { int x; } x;

+ 2 - 4
lvm.c

@@ -1739,10 +1739,8 @@ void luaV_execute (lua_State *L, CallInfo *ci) {
         vmbreak;
         vmbreak;
       }
       }
       vmcase(OP_TFORPREP) {
       vmcase(OP_TFORPREP) {
-        if (!ttisnil(s2v(ra + 3))) {  /* is 'toclose' not nil? */
-          /* create to-be-closed upvalue for it */
-          halfProtect(luaF_newtbcupval(L, ra + 3));
-        }
+        /* create to-be-closed upvalue (if needed) */
+        halfProtect(luaF_newtbcupval(L, ra + 3));
         pc += GETARG_Bx(i);
         pc += GETARG_Bx(i);
         i = *(pc++);  /* go to next instruction */
         i = *(pc++);  /* go to next instruction */
         lua_assert(GET_OPCODE(i) == OP_TFORCALL && ra == RA(i));
         lua_assert(GET_OPCODE(i) == OP_TFORCALL && ra == RA(i));

+ 11 - 8
manual/manual.of

@@ -1534,15 +1534,17 @@ or exiting by an error.
 
 
 Here, to @emph{close} a value means
 Here, to @emph{close} a value means
 to call its @idx{__close} metamethod.
 to call its @idx{__close} metamethod.
-If the value is @nil, it is ignored;
-otherwise,
-if it does not have a @idx{__close} metamethod,
-an error is raised.
 When calling the metamethod,
 When calling the metamethod,
 the value itself is passed as the first argument
 the value itself is passed as the first argument
-and the error object (if any) is passed as a second argument;
+and the error object that caused the exit (if any)
+is passed as a second argument;
 if there was no error, the second argument is @nil.
 if there was no error, the second argument is @nil.
 
 
+The value assigned to a to-be-closed variable
+must have a @idx{__close} metamethod
+or be a false value.
+(@nil and @false are ignored as to-be-closed values.)
+
 If several to-be-closed variables go out of scope at the same event,
 If several to-be-closed variables go out of scope at the same event,
 they are closed in the reverse order that they were declared.
 they are closed in the reverse order that they were declared.
 
 
@@ -2917,8 +2919,9 @@ it is left unchanged.
 @APIEntry{void lua_close (lua_State *L);|
 @APIEntry{void lua_close (lua_State *L);|
 @apii{0,0,-}
 @apii{0,0,-}
 
 
-Destroys all objects in the given Lua state
-(calling the corresponding garbage-collection metamethods, if any)
+Close all active to-be-closed variables in the main thread,
+release all objects in the given Lua state
+(calling the corresponding garbage-collection metamethods, if any),
 and frees all dynamic memory used by this state.
 and frees all dynamic memory used by this state.
 
 
 On several platforms, you may not need to call this function,
 On several platforms, you may not need to call this function,
@@ -4186,7 +4189,7 @@ An index marked as to-be-closed should not be removed from the stack
 by any other function in the API except @Lid{lua_settop} or @Lid{lua_pop}.
 by any other function in the API except @Lid{lua_settop} or @Lid{lua_pop}.
 
 
 This function should not be called for an index
 This function should not be called for an index
-that is equal to or below an already marked to-be-closed index.
+that is equal to or below an active to-be-closed index.
 
 
 This function can raise an out-of-memory error.
 This function can raise an out-of-memory error.
 In that case, the value in the given index is immediately closed,
 In that case, the value in the given index is immediately closed,

+ 10 - 1
testes/api.lua

@@ -1096,7 +1096,7 @@ do
   assert(type(a[1]) == "string" and a[2][1] == 11)
   assert(type(a[1]) == "string" and a[2][1] == 11)
   assert(#openresource == 0)    -- was closed
   assert(#openresource == 0)    -- was closed
 
 
-  -- error
+  -- closing by error
   local a, b = pcall(T.makeCfunc[[
   local a, b = pcall(T.makeCfunc[[
     call 0 1   # create resource
     call 0 1   # create resource
     toclose -1 # mark it to be closed
     toclose -1 # mark it to be closed
@@ -1105,6 +1105,15 @@ do
   assert(a == false and b[1] == 11)
   assert(a == false and b[1] == 11)
   assert(#openresource == 0)    -- was closed
   assert(#openresource == 0)    -- was closed
 
 
+  -- non-closable value
+  local a, b = pcall(T.makeCfunc[[
+    newtable   # create non-closable object
+    toclose -1 # mark it to be closed (shoud raise an error)
+    abort  # will not be executed
+  ]])
+  assert(a == false and
+    string.find(b, "non%-closable value"))
+
   local function check (n)
   local function check (n)
     assert(#openresource == n)
     assert(#openresource == n)
   end
   end

+ 12 - 11
testes/locals.lua

@@ -215,11 +215,13 @@ end
 do
 do
   local a = {}
   local a = {}
   do
   do
+    local b <close> = false   -- not to be closed
     local x <close> = setmetatable({"x"}, {__close = function (self)
     local x <close> = setmetatable({"x"}, {__close = function (self)
                                                    a[#a + 1] = self[1] end})
                                                    a[#a + 1] = self[1] end})
     local w, y <close>, z = func2close(function (self, err)
     local w, y <close>, z = func2close(function (self, err)
                                 assert(err == nil); a[#a + 1] = "y"
                                 assert(err == nil); a[#a + 1] = "y"
                               end, 10, 20)
                               end, 10, 20)
+    local c <close> = nil  -- not to be closed
     a[#a + 1] = "in"
     a[#a + 1] = "in"
     assert(w == 10 and z == 20)
     assert(w == 10 and z == 20)
   end
   end
@@ -325,24 +327,22 @@ do   -- errors in __close
 end
 end
 
 
 
 
-do
-
-  -- errors due to non-closable values
+do   -- errors due to non-closable values
   local function foo ()
   local function foo ()
     local x <close> = {}
     local x <close> = {}
+    os.exit(false)    -- should not run
   end
   end
   local stat, msg = pcall(foo)
   local stat, msg = pcall(foo)
-  assert(not stat and string.find(msg, "variable 'x'"))
+  assert(not stat and
+    string.find(msg, "variable 'x' got a non%-closable value"))
 
 
-
-  -- with other errors, non-closable values are ignored
   local function foo ()
   local function foo ()
-    local x <close> = 34
-    local y <close> = func2close(function () error(32) end)
+    local xyz <close> = setmetatable({}, {__close = print})
+    getmetatable(xyz).__close = nil   -- remove metamethod
   end
   end
   local stat, msg = pcall(foo)
   local stat, msg = pcall(foo)
-  assert(not stat and msg == 32)
-
+  assert(not stat and
+    string.find(msg, "attempt to close non%-closable variable 'xyz'"))
 end
 end
 
 
 
 
@@ -519,7 +519,8 @@ end
 -- a suspended coroutine should not close its variables when collected
 -- a suspended coroutine should not close its variables when collected
 local co
 local co
 co = coroutine.wrap(function()
 co = coroutine.wrap(function()
-  local x <close> = function () os.exit(false) end    -- should not run
+  -- should not run
+  local x <close> = func2close(function () os.exit(false) end)
   co = nil
   co = nil
   coroutine.yield()
   coroutine.yield()
 end)
 end)