From bfc9153893c943b57eb2547a5b57867b23c5bc2a Mon Sep 17 00:00:00 2001 From: Eldo Loguzzo Date: Wed, 7 Oct 2026 15:08:22 -0600 Subject: [PATCH] Issue #448 : a caught error closes only the upvalues of the frames it discards raiseError and Error called closeAllUpvalues() on every error, closing the open upvalues of all frames up to the root, including the frames that called pcall and keep running. A closure and the frame that owns the variable then stop sharing it. With xpcall nothing was closed, so a closure created inside the failed call read a register that the next call reused. LState.pcallBase records the register base of the innermost PCall. An error closes the upvalues from that base (from 0 when no pcall is active, as before), and PCall closes the same range when it recovers, also after a Go panic or an xpcall handler. Co-Authored-By: Claude Opus 5.5 (1M context) --- _glua-tests/issues.lua | 69 ++++++++++++++++++++++++++++++++++++++++++ _state.go | 12 ++++++-- state.go | 12 ++++++-- state_test.go | 12 ++++++++ value.go | 1 + 5 files changed, 102 insertions(+), 4 deletions(-) diff --git a/_glua-tests/issues.lua b/_glua-tests/issues.lua index 6d6343ab..079ba35a 100644 --- a/_glua-tests/issues.lua +++ b/_glua-tests/issues.lua @@ -490,3 +490,72 @@ function test() assert(b == nil) end test() + +-- issue #448: a caught error must not close the upvalues of the frames that +-- called pcall and keep running +function test() + local a + local function use_a() return a("test") end + pcall(function() error("an error") end) + a = function(v) return v end + assert(use_a() == "test") + + local x = 1 + local set = function(v) x = v end + local get = function() return x end + pcall(function() error("boom") end) + set(2) + assert(x == 2) + x = 3 + assert(get() == 3) + + pcall(string.rep) + x = 4 + assert(get() == 4) + + pcall(function() local y; return y.z end) + x = 5 + assert(get() == 5) + + local function inner() pcall(function() error("e") end) end + inner() + x = 6 + assert(get() == 6) + + pcall(function() + local y = 1 + local gy = function() return y end + pcall(function() error("in") end) + y = 2 + x = gy() + 10 + end) + assert(get() == 12) + + local passed, failed = 0, 0 + local function check(ok) if ok then passed = passed + 1 else failed = failed + 1 end end + for i = 1, 3 do + check(pcall(function() if i == 2 then error("case " .. i) end end)) + end + assert(passed == 2 and failed == 1) +end +test() + +-- issue #448: a closure created inside a failed xpcall/pcall keeps its own value +function test() + local keep + xpcall(function() local s = "inner"; keep = function() return s end; error("x") end, + function(e) return e end) + local function clobber() local a, b, c, d, e, f, g, h = 1, 2, 3, 4, 5, 6, 7, 8; return a end + clobber() + assert(keep() == "inner") + + pcall(function() local s = "inner2"; keep = function() return s end; error("x") end) + clobber() + assert(keep() == "inner2") + + local co = coroutine.create(function() local s = "co"; keep = function() return s end; error("die") end) + coroutine.resume(co) + clobber() + assert(keep() == "co") +end +test() diff --git a/_state.go b/_state.go index 6e9febfb..4a0ba823 100644 --- a/_state.go +++ b/_state.go @@ -634,7 +634,7 @@ func (ls *LState) closeAllUpvalues() { // +inline-start func (ls *LState) raiseError(level int, format string, args ...interface{}) { if !ls.hasErrorFunc { - ls.closeAllUpvalues() + ls.closeUpvalues(ls.pcallBase) } message := format if len(args) > 0 { @@ -1523,7 +1523,7 @@ func (ls *LState) Error(lv LValue, level int) { ls.raiseError(level, string(str)) } else { if !ls.hasErrorFunc { - ls.closeAllUpvalues() + ls.closeUpvalues(ls.pcallBase) } ls.Push(lv) ls.Panic(ls) @@ -1822,6 +1822,10 @@ func (ls *LState) PCall(nargs, nret int, errfunc *LFunction) (err error) { if errfunc != nil { ls.hasErrorFunc = true } + // This call is the innermost protected boundary until it returns (#448). + oldPcallBase := ls.pcallBase + ls.pcallBase = base + defer func() { ls.pcallBase = oldPcallBase }() defer func() { ls.Panic = oldpanic ls.hasErrorFunc = false @@ -1856,6 +1860,7 @@ func (ls *LState) PCall(nargs, nret int, errfunc *LFunction) (err error) { err = rcv.(*ApiError) err.(*ApiError).StackTrace = ls.stackTrace(0) } + ls.closeUpvalues(base) ls.stack.SetSp(sp) ls.currentFrame = ls.stack.Last() ls.reg.SetTop(base) @@ -1866,6 +1871,9 @@ func (ls *LState) PCall(nargs, nret int, errfunc *LFunction) (err error) { } else if len(err.(*ApiError).StackTrace) == 0 { err.(*ApiError).StackTrace = ls.stackTrace(0) } + // The frames above base are gone; close the upvalues they own + // (also after a Go panic or an xpcall handler, which skip the close in raiseError). + ls.closeUpvalues(base) ls.stack.SetSp(sp) ls.currentFrame = ls.stack.Last() ls.reg.SetTop(base) diff --git a/state.go b/state.go index 292f93b4..faa38abc 100644 --- a/state.go +++ b/state.go @@ -734,7 +734,7 @@ func (ls *LState) closeAllUpvalues() { // +inline-start func (ls *LState) raiseError(level int, format string, args ...interface{}) { if !ls.hasErrorFunc { - ls.closeAllUpvalues() + ls.closeUpvalues(ls.pcallBase) } message := format if len(args) > 0 { @@ -1736,7 +1736,7 @@ func (ls *LState) Error(lv LValue, level int) { ls.raiseError(level, string(str)) } else { if !ls.hasErrorFunc { - ls.closeAllUpvalues() + ls.closeUpvalues(ls.pcallBase) } ls.Push(lv) ls.Panic(ls) @@ -2035,6 +2035,10 @@ func (ls *LState) PCall(nargs, nret int, errfunc *LFunction) (err error) { if errfunc != nil { ls.hasErrorFunc = true } + // This call is the innermost protected boundary until it returns (#448). + oldPcallBase := ls.pcallBase + ls.pcallBase = base + defer func() { ls.pcallBase = oldPcallBase }() defer func() { ls.Panic = oldpanic ls.hasErrorFunc = false @@ -2069,6 +2073,7 @@ func (ls *LState) PCall(nargs, nret int, errfunc *LFunction) (err error) { err = rcv.(*ApiError) err.(*ApiError).StackTrace = ls.stackTrace(0) } + ls.closeUpvalues(base) ls.stack.SetSp(sp) ls.currentFrame = ls.stack.Last() ls.reg.SetTop(base) @@ -2079,6 +2084,9 @@ func (ls *LState) PCall(nargs, nret int, errfunc *LFunction) (err error) { } else if len(err.(*ApiError).StackTrace) == 0 { err.(*ApiError).StackTrace = ls.stackTrace(0) } + // The frames above base are gone; close the upvalues they own + // (also after a Go panic or an xpcall handler, which skip the close in raiseError). + ls.closeUpvalues(base) ls.stack.SetSp(sp) ls.currentFrame = ls.stack.Last() ls.reg.SetTop(base) diff --git a/state_test.go b/state_test.go index 7b7aa53d..75a2e4f1 100644 --- a/state_test.go +++ b/state_test.go @@ -760,3 +760,15 @@ func BenchmarkRegistrySetTop(t *testing.B) { reg.SetTop(0) } } + +// An error with no pcall on the Lua stack still closes every open upvalue of +// the state, so a closure keeps its value when the host reuses the state (#448). +func TestUnprotectedErrorClosesUpvalues(t *testing.T) { + L := NewState() + defer L.Close() + errorIfScriptNotFail(t, L, ` + local s = "kept" + keep = function() return s end + error("top level")`, "top level") + errorIfScriptFail(t, L, `local a, b, c, d = 9, 9, 9, 9; assert(keep() == "kept")`) +} diff --git a/value.go b/value.go index 4156e9d5..1abfc06d 100644 --- a/value.go +++ b/value.go @@ -192,6 +192,7 @@ type LState struct { wrapped bool uvcache *Upvalue hasErrorFunc bool + pcallBase int // register base of the innermost PCall (0: none), see #448 mainLoop func(*LState, *callFrame) ctx context.Context ctxCancelFn context.CancelFunc