fix(xpcall): catch errors under direct PCall/DoString + inspect upvalues#32
Open
skhaz wants to merge 5 commits into
Open
fix(xpcall): catch errors under direct PCall/DoString + inspect upvalues#32skhaz wants to merge 5 commits into
skhaz wants to merge 5 commits into
Conversation
Why: xpcall leaked its error and never invoked the message handler when invoked outside a coroutine (e.g. under DoString/PCall), aborting the surrounding chunk. basePCall was updated to wrap CallK in its own defer/recover, but baseXPCall was left depending on threadRun's recover via handleProtectedError — a layer that only exists inside coroutines. What: - Wrap baseXPCall's CallK in an inline defer/recover, mirroring basePCall. - Invoke the error handler BEFORE resetting the stack so it can inspect the throw site (debug.getlocal / debug.traceback), matching standard Lua semantics (PCall's own errfunc path does the same). - Add errorObjectFromRecover and invokeErrorHandler helpers. - Add xpcall_direct_test.go covering direct-call catch, chunk preservation, success path, nesting under pcall, and the still-working coroutine path.
Why: GetStackFrame never reported upvalues: it read funcTable["f"] for the frame function, but GetInfo's 'f' flag RETURNS the *LFunction (it does not store it under any table key). The funcTable was always empty, so the upvalue loop was skipped and debug.getupvalue kept disagreeing with inspect. What: - Capture the *LFunction from GetInfo's return value and use it for both the Go-function name resolution and the upvalue walk. - Drop the unused funcTable allocation. - Add TestGetStackFrameCapturesUpvalues guarding against regression.
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
2 participants
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.
Two latent fixes that are prerequisites for the Wippy runtime traceback work in wippyai/runtime#448.
xpcall: catch direct-call errors without corrupting later callsxpcallleaked its error and never invoked the message handler outside a coroutine (for example underDoString/PCall). It depended onthreadRun's recovery boundary, which only handles the coroutine path.baseXPCallnow catches synchronous errors inline and invokes the message handler before unwinding, while throw-site frames remain inspectable bydebug.getlocal/debug.traceback.The protected-call unwind is also centralized: discarded frame extensions are cleared before their stack indexes can be reused. Without that cleanup, a failed direct
xpcallleft itsCallKcontinuation and handler behind; a subsequent Go-backed Lua call at the same depth could execute the stale continuation instead of its own function. The same invariant now covers Luapcalland APIPCallunwinds.Regression coverage includes:
xpcallcatch, success, nesting, and coroutine behavior;xpcall;xpcallmessage handler;CallKframes discarded by Luapcalland APIPCall;Inspect: capture upvalues
GetStackFramenever reported upvalues because it readfuncTable["f"], whileGetInfo'sfflag returns the*LFunctiondirectly. The returned function is now used for Go-function name resolution and the upvalue walk.CI repair
The workflow previously failed validation before creating any jobs because
runner.tempwas referenced at job scope. Cache environment variables now live on execution steps, where therunnercontext is valid. The existing staticcheck/unused findings were also removed so the restored lint job has a clean baseline.Verification
go test ./... -count=1go test -race ./... -count=1git diff --checkThis PR does not tag or publish a go-lua release.