lua_settop/lua_pop closes to-be-closed variables

The existence of 'lua_closeslot' is no reason for lua_pop not to close
to-be-closed variables too.  It is too error-prone for lua_pop not to
close tbc variables being popped from the stack.

3 files changed, 44 insertions, 20 deletions

diff --git a/lapi.c b/lapi.c
index a9cf2fdb..f8f70cd0 100644
--- a/lapi.c
+++ b/lapi.c
@@ -173,7 +173,7 @@ LUA_API int lua_gettop (lua_State *L) {
 
 LUA_API void lua_settop (lua_State *L, int idx) {
   CallInfo *ci;
-  StkId func;
+  StkId func, newtop;
   ptrdiff_t diff;  /* difference for new top */
   lua_lock(L);
   ci = L->ci;
@@ -188,12 +188,13 @@ LUA_API void lua_settop (lua_State *L, int idx) {
     api_check(L, -(idx+1) <= (L->top - (func + 1)), "invalid new top");
     diff = idx + 1;  /* will "subtract" index (as it is negative) */
   }
-#if defined(LUA_COMPAT_5_4_0)
-  if (diff < 0 && hastocloseCfunc(ci->nresults))
-    luaF_close(L, L->top + diff, CLOSEKTOP, 0);
-#endif
-  api_check(L, L->tbclist < L->top + diff, "cannot pop an unclosed slot");
-  L->top += diff;
+  api_check(L, L->tbclist < L->top, "previous pop of an unclosed slot");
+  newtop = L->top + diff;
+  if (diff < 0 && L->tbclist >= newtop) {
+    lua_assert(hastocloseCfunc(ci->nresults));
+    luaF_close(L, newtop, CLOSEKTOP, 0);
+  }
+  L->top = newtop;  /* correct top only after closing any upvalue */
   lua_unlock(L);
 }
 
diff --git a/manual/manual.of b/manual/manual.of
index 2e15839a..c69970d2 100644
--- a/manual/manual.of
+++ b/manual/manual.of
@@ -4253,12 +4253,8 @@ If the new top is greater than the old one,
 then the new elements are filled with @nil.
 If @id{index} @N{is 0}, then all stack elements are removed.
 
-For compatibility reasons,
-this function may close slots marked as to-be-closed @see{lua_toclose},
-and therefore it can run arbitrary code.
-You should not rely on this behavior:
-Instead, always close to-be-closed slots explicitly,
-with @Lid{lua_closeslot}, before removing them from the stack.
+This function can run arbitrary code when removing an index
+marked as to-be-closed from the stack.
 
 }
 
@@ -4347,19 +4343,22 @@ otherwise, returns @id{NULL}.
 @apii{0,0,m}
 
 Marks the given index in the stack as a
-to-be-closed @Q{variable} @see{to-be-closed}.
+to-be-closed slot @see{to-be-closed}.
 Like a to-be-closed variable in Lua,
-the value at that index in the stack will be closed
+the value at that slot in the stack will be closed
 when it goes out of scope.
 Here, in the context of a C function,
 to go out of scope means that the running function returns to Lua,
-there is an error,
+or there is an error,
+or the slot is removed from the stack through
+@Lid{lua_settop} or @Lid{lua_pop},
 or there is a call to @Lid{lua_closeslot}.
-An index marked as to-be-closed should neither be removed from the stack
-nor modified before a corresponding call to @Lid{lua_closeslot}.
+A slot 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},
+unless previously deactivated by @Lid{lua_closeslot}.
 
 This function should not be called for an index
-that is equal to or below an active to-be-closed index.
+that is equal to or below an active to-be-closed slot.
 
 Note that, both in case of errors and of a regular return,
 by the time the @idx{__close} metamethod runs,
diff --git a/testes/api.lua b/testes/api.lua
index fb7e7085..c1bcb4b7 100644
--- a/testes/api.lua
+++ b/testes/api.lua
@@ -1130,7 +1130,7 @@ do
   -- closing resources with 'closeslot'
   _ENV.xxx = true
   local a = T.testC([[
-    pushvalue 2  # stack: S, NR, CH
+    pushvalue 2  # stack: S, NR, CH, NR
     call 0 1   # create resource; stack: S, NR, CH, R
     toclose -1 # mark it to be closed
     pushvalue 2  #  stack: S, NR, CH, R, NR
@@ -1151,6 +1151,30 @@ do
   ]], newresource, check)
   assert(a == 3 and _ENV.xxx == nil)   -- no extra items left in the stack
 
+  -- closing resources with 'pop'
+  local a = T.testC([[
+    pushvalue 2  # stack: S, NR, CH, NR
+    call 0 1   # create resource; stack: S, NR, CH, R
+    toclose -1 # mark it to be closed
+    pushvalue 2  #  stack: S, NR, CH, R, NR
+    call 0 1   # create another resource; stack: S, NR, CH, R, R
+    toclose -1 # mark it to be closed
+    pushvalue 3  # stack: S, NR, CH, R, R, CH
+    pushint 2   # there should be two open resources
+    call 1 0  #  stack: S, NR, CH, R, R
+    pop 1   # pop second resource
+    pushvalue 3  # stack: S, NR, CH, R, CH
+    pushint 1   # there should be one open resource
+    call 1 0  # stack: S, NR, CH, R
+    pop 1       # pop other resource from the stack
+    pushvalue 3  # stack: S, NR, CH, CH
+    pushint 0   # there should be no open resources
+    call 1 0  # stack: S, NR, CH
+    pushint *
+    return 1    # return stack size
+  ]], newresource, check)
+  assert(a == 3)   -- no extra items left in the stack
+
   -- non-closable value
   local a, b = pcall(T.makeCfunc[[
     pushint 32