api: claim ownerless upvalue joins

Author: Frityet

notes/api-debug-claim-cleanup.md

@@ -85,6 +85,10 @@ Permanent shape:
   slots or value slots, keep the claim through GC-owned field/upvalue
   publication, and drop it only after the API's consumed stack slots are
   removed. `lua_setfenv` uses a separate nested claim for the thread value case.
+- `lua_upvaluejoin()` resume-claims the target state before reading both Lua
+  function slots, keeps the claim through trace invalidation and release
+  publication of the joined upvalue pointer, then drops the claim without
+  changing stack shape.
 - Upvalue introspection APIs (`lua_getupvalue` and `lua_upvalueid`) claim the
   target state before reading the function slot. `lua_getupvalue` uses the
   protected one-slot growth helper and release-publishes the copied upvalue

src/lj_api.c

@@ -1672,9 +1672,14 @@ LUA_API void *lua_upvalueid(lua_State *L, int idx, int n)
 
 LUA_API void lua_upvaluejoin(lua_State *L, int idx1, int n1, int idx2, int n2)
 {
+  LJStateClaim claim;
+  lua_State *errL = api_errstate(L);
   TValue snap1, snap2;
-  GCfunc *fn1 = funcV(index2adr_read(L, idx1, &snap1));
-  GCfunc *fn2 = funcV(index2adr_read(L, idx2, &snap2));
+  GCfunc *fn1, *fn2;
+  if (!lj_state_resumeclaim(L, lj_thr_current_id(G(L)), &claim))
+    lj_err_callermsg(errL, "thread busy");
+  fn1 = funcV(index2adr_read(L, idx1, &snap1));
+  fn2 = funcV(index2adr_read(L, idx2, &snap2));
   n1--; n2--;
   lj_checkapi(isluafunc(fn1), "stack slot %d is not a Lua function", idx1);
   lj_checkapi(isluafunc(fn2), "stack slot %d is not a Lua function", idx2);
@@ -1684,11 +1689,12 @@ LUA_API void lua_upvaluejoin(lua_State *L, int idx1, int n1, int idx2, int n2)
     GCobj *uv = func_uvptr_acq(&fn2->l, (uint32_t)n2);
     GCobj *old = func_uvptr_acq(&fn1->l, (uint32_t)n1);
     if (old != uv) {
-      api_trace_flush_mutation(L);
+      api_trace_flush_mutation_claimed(L, errL, &claim);
       setgcrefrel(fn1->l.uvptr[n1], uv);
       lj_gc_pubobjobj(L, fn1, uv);
     }
   }
+  lj_state_dropresumeclaim(&claim);
 }
 
 LUALIB_API void *luaL_testudata(lua_State *L, int idx, const char *tname)

tests/suites/m5_publication.lua

@@ -1158,6 +1158,32 @@ END {
     exit 1
   }
 }
+]=], "src/lj_api.c")
+
+  awk([=[
+BEGIN {
+  infn = 0; claim = 0; read1 = 0; read2 = 0
+  flush = 0; store = 0; pub = 0; drop = 0
+}
+/^LUA_API void lua_upvaluejoin\(lua_State \*L,/ { infn = 1; next }
+infn && /^}/ { infn = 0; next }
+infn && /lj_state_resumeclaim/ { claim = NR }
+infn && /index2adr_read/ {
+  if (!read1) read1 = NR
+  else if (!read2) read2 = NR
+}
+infn && /api_trace_flush_mutation_claimed/ { flush = NR }
+infn && /setgcrefrel/ { store = NR }
+infn && /lj_gc_pubobjobj/ { pub = NR }
+infn && /lj_state_dropresumeclaim/ { drop = NR }
+END {
+  if (!claim || !read1 || !read2 || !flush || !store || !pub || !drop ||
+      claim > read1 || read1 > read2 || read2 > flush || flush > store ||
+      store > pub || pub > drop) {
+    print "lua_upvaluejoin owner-claim boundary missing"
+    exit 1
+  }
+}
 ]=], "src/lj_api.c")
 
   awk([=[

tests/t-state-owner.c

@@ -37,13 +37,21 @@ static void expect_thread_busy(lua_State *L, lua_CFunction fn,
 			       const char *what)
 {
   int status;
+  const char *msg;
   lua_pushcfunction(L, fn);
   status = lua_pcall(L, 0, 0, 0);
-  assert(status == LUA_ERRRUN);
-  assert(lua_tostring(L, -1) != NULL);
-  assert(strstr(lua_tostring(L, -1), "thread busy") != NULL);
+  if (status != LUA_ERRRUN) {
+    fprintf(stderr, "%s: expected LUA_ERRRUN, got %d\n", what, status);
+    assert(status == LUA_ERRRUN);
+  }
+  msg = lua_tostring(L, -1);
+  if (msg == NULL || strstr(msg, "thread busy") == NULL) {
+    fprintf(stderr, "%s: unexpected error object type=%s message=%s\n",
+	    what, lua_typename(L, lua_type(L, -1)), msg ? msg : "<null>");
+    assert(msg != NULL);
+    assert(strstr(msg, "thread busy") != NULL);
+  }
   lua_pop(L, 1);
-  (void)what;
 }
 
 static uint32_t foreign_tid(lua_State *L)
@@ -455,6 +463,16 @@ static void check_upvalue_api_unowned(lua_State *L)
   assert(lua_tointeger(co, -1) == 80);
   assert(lj_state_owner_acq(co) == 0);
 
+  lua_settop(L, 0);
+  co = load_ownerless_results(L,
+    "local function make(x) return function() return x end end\n"
+    "return make(1), make(2)", 2);
+  lua_upvaluejoin(co, 1, 1, 2, 1);
+  lua_pushvalue(co, 1);
+  lua_call(co, 0, 1);
+  assert(lua_tointeger(co, -1) == 2);
+  assert(lj_state_owner_acq(co) == 0);
+
   lua_settop(L, 0);
 }
 
@@ -1189,6 +1207,19 @@ static int busy_lua_setupvalue(lua_State *L)
   return 0;
 }
 
+static int busy_lua_upvaluejoin(lua_State *L)
+{
+  lua_State *co = lua_newthread(L);
+  assert(luaL_loadstring(L,
+    "local function make(x) return function() return x end end\n"
+    "return make(1), make(2)") == 0);
+  lua_call(L, 0, 2);
+  lua_xmove(L, co, 2);
+  lj_state_owner_rel(co, foreign_tid(L));
+  lua_upvaluejoin(co, 1, 1, 2, 1);
+  return 0;
+}
+
 static lua_State *busy_udata_prepare(lua_State *L)
 {
   lua_State *co;
@@ -1602,6 +1633,7 @@ int main(void)
   expect_thread_busy(L, busy_lua_getupvalue, "busy lua_getupvalue");
   expect_thread_busy(L, busy_lua_upvalueid, "busy lua_upvalueid");
   expect_thread_busy(L, busy_lua_setupvalue, "busy lua_setupvalue");
+  expect_thread_busy(L, busy_lua_upvaluejoin, "busy lua_upvaluejoin");
   expect_thread_busy(L, busy_luaL_testudata, "busy luaL_testudata");
   expect_thread_busy(L, busy_luaL_checkudata, "busy luaL_checkudata");
   expect_thread_busy(L, busy_luaL_getmetafield, "busy luaL_getmetafield");