Repository navigation
Issue #448 : a caught error closes only the upvalues of the frames it discards - #542
Open
eldologuzzo wants to merge 1 commit into
Open
eldologuzzo wants to merge 1 commit into
eldologuzzo wants to merge 1 commit into
Conversation
…s 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) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Nested protected calls do not restore hasErrorFunc, allowing premature upvalue closure before an outer xpcall handler runs.
1 open finding
What changed in this PR
Fixes upvalue handling when protected Lua calls discard stack frames.
Changes:
- Tracks the innermost protected-call register base.
- Closes only upvalues owned by discarded frames.
- Adds regression coverage for protected and unprotected errors.
| File | Description |
|---|---|
value.go |
Adds protected-call state tracking. |
_state.go |
Implements scoped upvalue closing. |
state.go |
Regenerates the runtime implementation. |
state_test.go |
Tests unprotected-error behavior. |
_glua-tests/issues.lua |
Adds issue #448 regression scenarios. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+1825
to
+1828
| // This call is the innermost protected boundary until it returns (#448). | ||
| oldPcallBase := ls.pcallBase | ||
| ls.pcallBase = base | ||
| defer func() { ls.pcallBase = oldPcallBase }() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #448 .
Changes proposed in this pull request:
raiseErrorandErrorcalledcloseAllUpvalues()on every error. That closes the open upvalues of all frames up to the root, including the frames that calledpcalland keep running after it returns. From then on a closure and the frame that owns the variable no longer share it (the example in bug: pcall affecting function upvalues #448; also a test harness whose pass/fail counters stop being updated after the first caught error).xpcallnothing was closed, so a closure created inside the failed call kept pointing at a stack register that the next call reused.LState.pcallBase: the register base of the innermostPCall, set for the duration of the call and restored afterwards. An error closes the upvalues from that base. With nopcallactive the base is 0, so an unprotected error still closes everything, as before.PCallcloses the same range when it recovers an error, which also covers a Go panic and anxpcallhandler (both skip the close inraiseError)._state.go;state.gois regenerated withmake build._glua-tests/issues.lua: the bug: pcall affecting function upvalues #448 example plus several cases (Go function errors, runtime errors, a caller two frames up, nestedpcall, closures created inside failedpcall/xpcall/coroutines). On current master the first one fails withattempt to call a non-function object; with this change they all pass and match Lua 5.1.TestUnprotectedErrorClosesUpvalueskeeps the previous behavior for unprotected errors.go testpasses.🤖 Generated with Claude Code