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.)

6 files changed, 70 insertions, 41 deletions

diff --git a/lfunc.c b/lfunc.c
index 6f2f897f..8f39f6b0 100644
--- a/lfunc.c
+++ b/lfunc.c
@@ -124,11 +124,23 @@ 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
 ** 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 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).
 ** If status is OK, the call to the closing method will be pushed
 ** 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 (prepclosingmethod(L, uv, &G(L)->nilvalue))  /* something to call? */
       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 */
     ptrdiff_t oldtop;
@@ -170,9 +178,7 @@ static int callclosemth (lua_State *L, StkId level, int status) {
 ** (can raise a memory-allocation error)
 */
 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.
 */
 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 */
-    luaD_throw(L, LUA_ERRMEM);  /* throw memory error */
+      luaD_throw(L, LUA_ERRMEM);  /* throw memory error */
+    }
   }
 }
 
diff --git a/ltests.c b/ltests.c
index 09876ee7..21273ea9 100644
--- a/ltests.c
+++ b/ltests.c
@@ -1572,6 +1572,9 @@ static int runC (lua_State *L, lua_State *L1, const char *pc) {
     else if EQ("error") {
       lua_error(L1);
     }
+    else if EQ("abort") {
+      abort();
+    }
     else if EQ("throw") {
 #if defined(__cplusplus)
 static struct X { int x; } x;
diff --git a/lvm.c b/lvm.c
index f177ce6a..1cfc1035 100644
--- a/lvm.c
+++ b/lvm.c
@@ -1739,10 +1739,8 @@ void luaV_execute (lua_State *L, CallInfo *ci) {
         vmbreak;
       }
       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);
         i = *(pc++);  /* go to next instruction */
         lua_assert(GET_OPCODE(i) == OP_TFORCALL && ra == RA(i));
diff --git a/manual/manual.of b/manual/manual.of
index 1646f113..8eebe9cb 100644
--- a/manual/manual.of
+++ b/manual/manual.of
@@ -1534,15 +1534,17 @@ or exiting by an error.
 
 Here, to @emph{close} a value means
 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,
 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.
 
+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,
 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);|
 @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.
 
 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}.
 
 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.
 In that case, the value in the given index is immediately closed,
diff --git a/testes/api.lua b/testes/api.lua
index 3f7f7596..f6915c3e 100644
--- a/testes/api.lua
+++ b/testes/api.lua
@@ -1096,7 +1096,7 @@ do
   assert(type(a[1]) == "string" and a[2][1] == 11)
   assert(#openresource == 0)    -- was closed
 
-  -- error
+  -- closing by error
   local a, b = pcall(T.makeCfunc[[
     call 0 1   # create resource
     toclose -1 # mark it to be closed
@@ -1105,6 +1105,15 @@ do
   assert(a == false and b[1] == 11)
   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)
     assert(#openresource == n)
   end
diff --git a/testes/locals.lua b/testes/locals.lua
index 3b145ca3..6eb1ba0e 100644
--- a/testes/locals.lua
+++ b/testes/locals.lua
@@ -215,11 +215,13 @@ end
 do
   local a = {}
   do
+    local b <close> = false   -- not to be closed
     local x <close> = setmetatable({"x"}, {__close = function (self)
                                                    a[#a + 1] = self[1] end})
     local w, y <close>, z = func2close(function (self, err)
                                 assert(err == nil); a[#a + 1] = "y"
                               end, 10, 20)
+    local c <close> = nil  -- not to be closed
     a[#a + 1] = "in"
     assert(w == 10 and z == 20)
   end
@@ -325,24 +327,22 @@ do   -- errors in __close
 end
 
 
-do
-
-  -- errors due to non-closable values
+do   -- errors due to non-closable values
   local function foo ()
     local x <close> = {}
+    os.exit(false)    -- should not run
   end
   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 x <close> = 34
-    local y <close> = func2close(function () error(32) end)
+    local xyz <close> = setmetatable({}, {__close = print})
+    getmetatable(xyz).__close = nil   -- remove metamethod
   end
   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
 
 
@@ -519,7 +519,8 @@ end
 -- a suspended coroutine should not close its variables when collected
 local co
 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
   coroutine.yield()
 end)