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