Browse Source

No more to-be-closed functions

To-be-closed variables must contain objects with '__toclose'
metamethods (or nil). Functions were removed for several reasons:

* Functions interact badly with sandboxes. If a sandbox raises
an error to interrupt a script, a to-be-closed function still
can hijack control and continue running arbitrary sandboxed code.

* Functions interact badly with coroutines. If a coroutine yields
and is never resumed again, its to-be-closed functions will never
run. To-be-closed objects, on the other hand, will still be closed,
provided they have appropriate finalizers.

* If you really need a function, it is easy to create a dummy
object to run that function in its '__toclose' metamethod.

This comit also adds closing of variables in case of panic.
Roberto Ierusalimschy 6 years ago
parent
commit
4ace93ca65
8 changed files with 97 additions and 68 deletions
  1. 1 0
      ldo.c
  2. 10 14
      lfunc.c
  3. 11 7
      ltests.c
  4. 14 20
      manual/manual.of
  5. 18 1
      testes/api.lua
  6. 11 2
      testes/coroutine.lua
  7. 1 1
      testes/goto.lua
  8. 31 23
      testes/locals.lua

+ 1 - 0
ldo.c

@@ -118,6 +118,7 @@ l_noret luaD_throw (lua_State *L, int errcode) {
   }
   }
   else {  /* thread has no error handler */
   else {  /* thread has no error handler */
     global_State *g = G(L);
     global_State *g = G(L);
+    errcode = luaF_close(L, L->stack, errcode);  /* close all upvalues */
     L->status = cast_byte(errcode);  /* mark it as dead */
     L->status = cast_byte(errcode);  /* mark it as dead */
     if (g->mainthread->errorJmp) {  /* main thread has a handler? */
     if (g->mainthread->errorJmp) {  /* main thread has a handler? */
       setobjs2s(L, g->mainthread->top++, L->top - 1);  /* copy error obj. */
       setobjs2s(L, g->mainthread->top++, L->top - 1);  /* copy error obj. */

+ 10 - 14
lfunc.c

@@ -100,28 +100,23 @@ UpVal *luaF_findupval (lua_State *L, StkId level) {
 
 
 static void callclose (lua_State *L, void *ud) {
 static void callclose (lua_State *L, void *ud) {
   UNUSED(ud);
   UNUSED(ud);
-  luaD_callnoyield(L, L->top - 2, 0);
+  luaD_callnoyield(L, L->top - 3, 0);
 }
 }
 
 
 
 
 /*
 /*
-** Prepare closing method plus its argument for object 'obj' with
+** Prepare closing method plus its arguments for object 'obj' with
 ** error message 'err'. (This function assumes EXTRA_STACK.)
 ** error message 'err'. (This function assumes EXTRA_STACK.)
 */
 */
 static int prepclosingmethod (lua_State *L, TValue *obj, TValue *err) {
 static int prepclosingmethod (lua_State *L, TValue *obj, TValue *err) {
   StkId top = L->top;
   StkId top = L->top;
-  if (ttisfunction(obj)) {  /* object to-be-closed is a function? */
-    setobj2s(L, top, obj);  /* push function */
-    setobj2s(L, top + 1, err);  /* push error msg. as argument */
-  }
-  else {  /* try '__close' metamethod */
-    const TValue *tm = luaT_gettmbyobj(L, obj, TM_CLOSE);
-    if (ttisnil(tm))  /* no metamethod? */
-      return 0;  /* nothing to call */
-    setobj2s(L, top, tm);  /* will call metamethod... */
-    setobj2s(L, top + 1, obj);  /* with 'self' as the argument */
-  }
-  L->top = top + 2;  /* add function and argument */
+  const TValue *tm = luaT_gettmbyobj(L, obj, TM_CLOSE);
+  if (ttisnil(tm))  /* no metamethod? */
+    return 0;  /* nothing to call */
+  setobj2s(L, top, tm);  /* will call metamethod... */
+  setobj2s(L, top + 1, obj);  /* with 'self' as the 1st argument */
+  setobj2s(L, top + 2, err);  /* and error msg. as 2nd argument */
+  L->top = top + 3;  /* add function and arguments */
   return 1;
   return 1;
 }
 }
 
 
@@ -156,6 +151,7 @@ static int callclosemth (lua_State *L, TValue *uv, StkId level, int status) {
       if (newstatus != LUA_OK)  /* another error when closing? */
       if (newstatus != LUA_OK)  /* another error when closing? */
         status = newstatus;  /* this will be the new error */
         status = newstatus;  /* this will be the new error */
     }
     }
+    /* else no metamethod; ignore this case and keep original error */
   }
   }
   return status;
   return status;
 }
 }

+ 11 - 7
ltests.c

@@ -1201,8 +1201,8 @@ static const char *const delimits = " \t\n,;";
 static void skip (const char **pc) {
 static void skip (const char **pc) {
   for (;;) {
   for (;;) {
     if (**pc != '\0' && strchr(delimits, **pc)) (*pc)++;
     if (**pc != '\0' && strchr(delimits, **pc)) (*pc)++;
-    else if (**pc == '#') {
-      while (**pc != '\n' && **pc != '\0') (*pc)++;
+    else if (**pc == '#') {  /* comment? */
+      while (**pc != '\n' && **pc != '\0') (*pc)++;  /* until end-of-line */
     }
     }
     else break;
     else break;
   }
   }
@@ -1544,18 +1544,22 @@ static int runC (lua_State *L, lua_State *L1, const char *pc) {
     }
     }
     else if EQ("setfield") {
     else if EQ("setfield") {
       int t = getindex;
       int t = getindex;
-      lua_setfield(L1, t, getstring);
+      const char *s = getstring;
+      lua_setfield(L1, t, s);
     }
     }
     else if EQ("setglobal") {
     else if EQ("setglobal") {
-      lua_setglobal(L1, getstring);
+      const char *s = getstring;
+      lua_setglobal(L1, s);
     }
     }
     else if EQ("sethook") {
     else if EQ("sethook") {
       int mask = getnum;
       int mask = getnum;
       int count = getnum;
       int count = getnum;
-      sethookaux(L1, mask, count, getstring);
+      const char *s = getstring;
+      sethookaux(L1, mask, count, s);
     }
     }
     else if EQ("setmetatable") {
     else if EQ("setmetatable") {
-      lua_setmetatable(L1, getindex);
+      int idx = getindex;
+      lua_setmetatable(L1, idx);
     }
     }
     else if EQ("settable") {
     else if EQ("settable") {
       lua_settable(L1, getindex);
       lua_settable(L1, getindex);
@@ -1620,7 +1624,7 @@ static struct X { int x; } x;
       return lua_yieldk(L1, nres, i, Cfunck);
       return lua_yieldk(L1, nres, i, Cfunck);
     }
     }
     else if EQ("toclose") {
     else if EQ("toclose") {
-      lua_toclose(L, getnum);
+      lua_toclose(L1, getnum);
     }
     }
     else luaL_error(L, "unknown instruction %s", buff);
     else luaL_error(L, "unknown instruction %s", buff);
   }
   }

+ 14 - 20
manual/manual.of

@@ -1536,33 +1536,29 @@ goes out of scope, including normal block termination,
 exiting its block by @Rw{break}/@Rw{goto}/@Rw{return},
 exiting its block by @Rw{break}/@Rw{goto}/@Rw{return},
 or exiting by an error.
 or exiting by an error.
 
 
-To \emph{close} a value has the following meaning here:
-If the value of the variable when it goes out of scope is a function,
-that function is called;
-otherwise, if the value has a @idx{__close} metamethod,
-that metamethod is called;
-otherwise, if the value is @nil, nothing is done;
-otherwise, an error is raised.
-In the function case,
-if the scope is being closed by an error,
-the error object is passed as an argument to the function;
-if there is no error, the function gets @nil.
-In the metamethod case,
-the value itself always is passed as an argument to the metamethod.
+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;
+if there was no error, the second argument is @nil.
 
 
 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.
-If there is any error while running a closing function,
+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;
 in particular,
 in particular,
-the other pending closing functions will still be called.
+the other pending closing methods will still be called.
 
 
 If a coroutine yields inside a block and is never resumed again,
 If a coroutine yields inside a block and is never resumed again,
 the variables visible at that block will never go out of scope,
 the variables visible at that block will never go out of scope,
 and therefore they will not be closed.
 and therefore they will not be closed.
-Similarly, if a script is interrupted by an unprotected error,
-its to-be-closed variables will not be closed.
+(You should use finalizers to handle this case.)
 
 
 }
 }
 
 
@@ -3002,7 +2998,7 @@ and therefore never returns
 }
 }
 
 
 @APIEntry{int lua_gc (lua_State *L, int what, int data);|
 @APIEntry{int lua_gc (lua_State *L, int what, int data);|
-@apii{0,0,v}
+@apii{0,0,-}
 
 
 Controls the garbage collector.
 Controls the garbage collector.
 
 
@@ -3056,8 +3052,6 @@ returns a boolean that tells whether the collector is running
 For more details about these options,
 For more details about these options,
 see @Lid{collectgarbage}.
 see @Lid{collectgarbage}.
 
 
-This function may raise errors when calling finalizers.
-
 }
 }
 
 
 @APIEntry{lua_Alloc lua_getallocf (lua_State *L, void **ud);|
 @APIEntry{lua_Alloc lua_getallocf (lua_State *L, void **ud);|

+ 18 - 1
testes/api.lua

@@ -396,6 +396,23 @@ do
     assert(string.find(msg, "stack overflow"))
     assert(string.find(msg, "stack overflow"))
   end
   end
 
 
+  -- exit in panic still close to-be-closed variables
+  assert(T.checkpanic([[
+    pushstring "return {__close = function () Y = 'ho'; end}"
+    newtable
+    loadstring -2
+    call 0 1
+    setmetatable -2
+    toclose -1
+    pushstring "hi"
+    error
+  ]],
+  [[
+    getglobal Y
+    concat 2         # concat original error with global Y
+  ]]) == "hiho")
+
+
 end
 end
 
 
 -- testing deep C stack
 -- testing deep C stack
@@ -1115,7 +1132,7 @@ end)
 testamem("to-be-closed variables", function()
 testamem("to-be-closed variables", function()
   local flag
   local flag
   do
   do
-    local *toclose x = function () flag = true end
+    local *toclose x = setmetatable({}, {__close = function () flag = true end})
     flag = false
     flag = false
     local x = {}
     local x = {}
   end
   end

+ 11 - 2
testes/coroutine.lua

@@ -142,8 +142,15 @@ do
 
 
   -- to-be-closed variables in coroutines
   -- to-be-closed variables in coroutines
   local X
   local X
+
+  local function func2close (f)
+    return setmetatable({}, {__close = f})
+  end
+
   co = coroutine.create(function ()
   co = coroutine.create(function ()
-    local *toclose x = function (err) assert(err == nil); X = false end
+    local *toclose x = func2close(function (self, err)
+      assert(err == nil); X = false
+    end)
     X = true
     X = true
     coroutine.yield()
     coroutine.yield()
   end)
   end)
@@ -154,7 +161,9 @@ do
 
 
   -- error killing a coroutine
   -- error killing a coroutine
   co = coroutine.create(function()
   co = coroutine.create(function()
-    local *toclose x = function (err) assert(err == nil); error(111) end
+    local *toclose x = func2close(function (self, err)
+      assert(err == nil); error(111)
+    end)
     coroutine.yield()
     coroutine.yield()
   end)
   end)
   coroutine.resume(co)
   coroutine.resume(co)

+ 1 - 1
testes/goto.lua

@@ -258,7 +258,7 @@ do
   ::L2:: goto L3
   ::L2:: goto L3
 
 
   ::L1:: do
   ::L1:: do
-    local *toclose a = function () X = true end
+    local *toclose a = setmetatable({}, {__close = function () X = true end})
     assert(X == nil)
     assert(X == nil)
     if a then goto L2 end   -- jumping back out of scope of 'a'
     if a then goto L2 end   -- jumping back out of scope of 'a'
   end
   end

+ 31 - 23
testes/locals.lua

@@ -177,13 +177,19 @@ print"testing to-be-closed variables"
 
 
 local function stack(n) n = ((n == 0) or stack(n - 1)) end
 local function stack(n) n = ((n == 0) or stack(n - 1)) end
 
 
+local function func2close (f)
+  return setmetatable({}, {__close = f})
+end
+
 
 
 do
 do
   local a = {}
   local a = {}
   do
   do
     local *toclose x = setmetatable({"x"}, {__close = function (self)
     local *toclose x = setmetatable({"x"}, {__close = function (self)
                                                    a[#a + 1] = self[1] end})
                                                    a[#a + 1] = self[1] end})
-    local *toclose y = function (x) assert(x == nil); a[#a + 1] = "y" end
+    local *toclose y = func2close(function (self, err)
+                         assert(err == nil); a[#a + 1] = "y"
+                       end)
     a[#a + 1] = "in"
     a[#a + 1] = "in"
   end
   end
   a[#a + 1] = "out"
   a[#a + 1] = "out"
@@ -193,7 +199,7 @@ end
 do
 do
   local X = false
   local X = false
 
 
-  local function closescope () stack(10); X = true end
+  local closescope = func2close(function () stack(10); X = true end)
 
 
   -- closing functions do not corrupt returning values
   -- closing functions do not corrupt returning values
   local function foo (x)
   local function foo (x)
@@ -228,13 +234,13 @@ do
   -- calls cannot be tail in the scope of to-be-closed variables
   -- calls cannot be tail in the scope of to-be-closed variables
   local X, Y
   local X, Y
   local function foo ()
   local function foo ()
-    local *toclose _ = function () Y = 10 end
+    local *toclose _ = func2close(function () Y = 10 end)
     assert(X == true and Y == nil)    -- 'X' not closed yet
     assert(X == true and Y == nil)    -- 'X' not closed yet
     return 1,2,3
     return 1,2,3
   end
   end
 
 
   local function bar ()
   local function bar ()
-    local *toclose _ = function () X = false end
+    local *toclose _ = func2close(function () X = false end)
     X = true
     X = true
     do
     do
       return foo()    -- not a tail call!
       return foo()    -- not a tail call!
@@ -249,11 +255,15 @@ end
 do   -- errors in __close
 do   -- errors in __close
   local log = {}
   local log = {}
   local function foo (err)
   local function foo (err)
-    local *toclose x = function (msg) log[#log + 1] = msg; error(1) end
-    local *toclose x1 = function (msg) log[#log + 1] = msg; end
-    local *toclose gc = function () collectgarbage() end
-    local *toclose y = function (msg) log[#log + 1] = msg; error(2) end
-    local *toclose z = function (msg) log[#log + 1] = msg or 10; error(3) end
+    local *toclose x =
+       func2close(function (self, msg) log[#log + 1] = msg; error(1) end)
+    local *toclose x1 =
+       func2close(function (self, msg) log[#log + 1] = msg; end)
+    local *toclose gc = func2close(function () collectgarbage() end)
+    local *toclose y =
+      func2close(function (self, msg) log[#log + 1] = msg; error(2) end)
+    local *toclose z =
+      func2close(function (self, msg) log[#log + 1] = msg or 10; error(3) end)
     if err then error(4) end
     if err then error(4) end
   end
   end
   local stat, msg = pcall(foo, false)
   local stat, msg = pcall(foo, false)
@@ -282,7 +292,7 @@ do
   -- with other errors, non-closable values are ignored
   -- with other errors, non-closable values are ignored
   local function foo ()
   local function foo ()
     local *toclose x = 34
     local *toclose x = 34
-    local *toclose y = function () error(32) end
+    local *toclose y = func2close(function () error(32) end)
   end
   end
   local stat, msg = pcall(foo)
   local stat, msg = pcall(foo)
   assert(not stat and msg == 32)
   assert(not stat and msg == 32)
@@ -294,7 +304,7 @@ if rawget(_G, "T") then
 
 
   -- memory error inside closing function
   -- memory error inside closing function
   local function foo ()
   local function foo ()
-    local *toclose y = function () T.alloccount() end
+    local *toclose y = func2close(function () T.alloccount() end)
     local *toclose x = setmetatable({}, {__close = function ()
     local *toclose x = setmetatable({}, {__close = function ()
       T.alloccount(0); local x = {}   -- force a memory error
       T.alloccount(0); local x = {}   -- force a memory error
     end})
     end})
@@ -308,12 +318,12 @@ if rawget(_G, "T") then
   local _, msg = pcall(foo)
   local _, msg = pcall(foo)
   assert(msg == "not enough memory")
   assert(msg == "not enough memory")
 
 
-  local function close (msg)
+  local close = func2close(function (self, msg)
     T.alloccount()
     T.alloccount()
     assert(msg == "not enough memory")
     assert(msg == "not enough memory")
-  end
+  end)
 
 
-  -- set a memory limit and return a closing function to remove the limit
+  -- set a memory limit and return a closing object to remove the limit
   local function enter (count)
   local function enter (count)
     stack(10)   -- reserve some stack space
     stack(10)   -- reserve some stack space
     T.alloccount(count)
     T.alloccount(count)
@@ -336,13 +346,13 @@ if rawget(_G, "T") then
 
 
   -- repeat test with extra closing upvalues
   -- repeat test with extra closing upvalues
   local function test ()
   local function test ()
-    local *toclose xxx = function (msg)
+    local *toclose xxx = func2close(function (self, msg)
       assert(msg == "not enough memory");
       assert(msg == "not enough memory");
       error(1000)   -- raise another error
       error(1000)   -- raise another error
-    end
-    local *toclose xx = function (msg)
+    end)
+    local *toclose xx = func2close(function (self, msg)
       assert(msg == "not enough memory");
       assert(msg == "not enough memory");
-    end
+    end)
     local *toclose x = enter(0)   -- set a memory limit
     local *toclose x = enter(0)   -- set a memory limit
     -- creation of previous upvalue will raise a memory error
     -- creation of previous upvalue will raise a memory error
     os.exit(false)    -- should not run
     os.exit(false)    -- should not run
@@ -413,9 +423,9 @@ do
   local x = false
   local x = false
   local y = false
   local y = false
   local co = coroutine.create(function ()
   local co = coroutine.create(function ()
-    local *toclose xv = function () x = true end
+    local *toclose xv = func2close(function () x = true end)
     do
     do
-      local *toclose yv = function () y = true end
+      local *toclose yv = func2close(function () y = true end)
       coroutine.yield(100)   -- yield doesn't close variable
       coroutine.yield(100)   -- yield doesn't close variable
     end
     end
     coroutine.yield(200)   -- yield doesn't close variable
     coroutine.yield(200)   -- yield doesn't close variable
@@ -453,9 +463,7 @@ do
       end,
       end,
       nil,   -- state
       nil,   -- state
       nil,   -- control variable
       nil,   -- control variable
-      function ()   -- closing function
-        numopen = numopen - 1
-      end
+      func2close(function () numopen = numopen - 1 end)   -- closing function
   end
   end
 
 
   local s = 0
   local s = 0