Skip to content

Commit 7368193

Browse files
committed
Fix serialization of open upvalues on threads
This fixes a long-standing bug I've always suspected I might've accidentally introduced. Luau's handling of upvalues is so optimized I had trouble finding a case that would trigger it before I incidentally tripped over it while working on `lljson`.
1 parent 604f5b8 commit 7368193

File tree

2 files changed

+53
-1
lines changed

2 files changed

+53
-1
lines changed

VM/src/ares.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2380,7 +2380,10 @@ p_thread(Info *info) { /* ... thread */
23802380
pushpath(info, "[%d]", level++);
23812381
WRITE_VALUE(eris_savestackidx(thread, uv->v) + 1, ares_size_t);
23822382
eris_setobj(info->L, info->L->top - 1, uv->v); /* ... thread obj */
2383-
lua_pushlightuserdata(info->L, uv); /* ... thread obj id */
2383+
// Use `uv->v` (stack slot address) with `LUTAG_ARES_UPREF as the key
2384+
// so we can have this point at the underlying value on the stack.
2385+
// `uv` itself is generally irrelevant.
2386+
lua_pushlightuserdatatagged(info->L, uv->v, LUTAG_ARES_UPREF); /* ... thread obj id */
23842387
persist_keyed(info, LUA_TUPVAL); /* ... thread obj */
23852388
poppath(info);
23862389
eris_assert(uv_top == lua_gettop(info->L));

tests/conformance/ares_coros.lua

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,5 +144,54 @@ assert(tabs[3] == rtabs[3])
144144
assert(tabs[3].one == rtabs[3].one)
145145
assert(tabs[2].three == rtabs[2].three)
146146

147+
-- open upvalue patching: closure on the stack (not executing) at yield time
148+
-- Regression test: p_thread's open upvalue section must use the same reftable key
149+
-- as p_closure so u_thread can patch closure upvalue references to be open.
150+
-- Without the fix, the inner closure's upvalues remain closed after unpersist,
151+
-- so writes from the closure go to a copy instead of the outer function's local.
152+
do
153+
local co = coroutine.create(function()
154+
local captured = nil
155+
local function writer()
156+
captured = "written"
157+
end
158+
-- Push writer onto the stack and yield before calling it.
159+
-- This means writer (with its open upvalue for `captured`) is a raw
160+
-- value on the coroutine's stack at persist time.
161+
local f = writer
162+
coroutine.yield()
163+
-- After round-trip, call writer. Its upvalue must be open (pointing to
164+
-- our stack slot for `captured`), not closed (pointing to a copy).
165+
f()
166+
return captured
167+
end)
168+
169+
coroutine.resume(co)
170+
co = ares.unpersist(uperms, ares.persist(perms, co))
171+
local ok, result = coroutine.resume(co)
172+
assert(ok)
173+
assert(result == "written", "expected 'written', got " .. tostring(result))
174+
end
175+
176+
-- same test but with two closures sharing the open upvalue
177+
do
178+
local co = coroutine.create(function()
179+
local shared = 0
180+
local function inc() shared = shared + 1 end
181+
local function get() return shared end
182+
local fi, fg = inc, get
183+
coroutine.yield()
184+
fi()
185+
fi()
186+
return fg()
187+
end)
188+
189+
coroutine.resume(co)
190+
co = ares.unpersist(uperms, ares.persist(perms, co))
191+
local ok, result = coroutine.resume(co)
192+
assert(ok)
193+
assert(result == 2, "expected 2, got " .. tostring(result))
194+
end
195+
147196
print('OK')
148197
return 'OK'

0 commit comments

Comments
 (0)