fix: stabilize protected calls across direct, unwind, and yield paths - #32
Merged
Merged
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.
wolfy-j
force-pushed
the
fix/xpcall-recover
branch
from
August 31, 2026 21:51
0ba54f6 to
802aa9e
Compare
wolfy-j
changed the title
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.
Rebases the protected-call stability work onto current
mainafter #39, preserving the original focused commits and removing stale compiler, CI, golden-fixture, and type-system drift.Protected calls
xpcallerrors underDoString/PCalland invoke the message handler while throw-site frames remain inspectable.pcall,xpcall, and APIPCallframes before their stack indexes are reused.return pcall(...)and its Lua caller is collapsed by a tail call.Regression coverage includes direct and nested
xpcall, handler failures, stale-frame reuse, nestedCallKunwinds, yielding tail-pcall, and root Go-function resume.Issue #37 stability coverage
Adds the Wippy-shaped resumed-thread +
pcall+ yielding-Go-call + register-pressure regression. It forces a real post-resume registry growth at the unchanged baseline size (256) and configured sizes 128 and 384. The hypothesized nil-register loss does not reproduce in upstream core under these conditions, so this PR deliberately avoids a speculative registry mutation.Inspect
GetStackFramenow uses the*LFunctionreturned byGetInfo("...f...")for Go-function name resolution and Lua upvalue capture.Verification
go test ./... -count=1-racego vet ./...git diff --checkThe compiler
LOADNILfix is intentionally not duplicated here; it landed separately in #39.