Skip to content

Issue #448 : a caught error closes only the upvalues of the frames it discards #542

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
eldologuzzo wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Issue #448 : a caught error closes only the upvalues of the frames it discards #542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 69 additions & 0 deletions _glua-tests/issues.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
12 changes: 10 additions & 2 deletions _state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 }()
Comment on lines +1825 to +1828
defer func() {
ls.Panic = oldpanic
ls.hasErrorFunc = false
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
12 changes: 10 additions & 2 deletions state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
12 changes: 12 additions & 0 deletions state_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")`)
}
1 change: 1 addition & 0 deletions value.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down