deps: adopt panic-safe golua fork and absorb its consequences - #10
Open
srgg wants to merge 1 commit into
Open
Conversation
Switch to the golua fork (github.com/srgg/golua; upstream PRs aarzilli/golua#130 thread-correct dispatch, #131 panic containment) that fixes two long-standing defects blim worked around: - Go panics (RaiseError, accidental) no longer unwind across in-flight lua_pcall/lua_resume frames and corrupt the VM; - Go functions called from coroutines run on the correct stack. Consequences absorbed here (all behind the same shared lua_State — the mutex-reentrancy vector is unchanged and its in-callback guards stay until the RFC-002 actor lands): - Re-enable the GC-at-close sequence in LuaEngine.Close (audit L-1): the "GC moved crash earlier" corruption was the panic-crossing class, now fixed; close-time finalizers run against an intact registry again. - Restore standard pcall/xpcall globals in blim.lua (they were hidden because Go panics bypassed them unsafely); blim.pcall becomes a deprecated alias. Go-backed errors are now catchable and the VM stays intact. - Comment hygiene: rewrite the ~14 stale "a Go panic corrupts the recovered lua_State / bypasses the VM unwinder / not catchable by pcall" rationales; keep the still-valid mutex-reentrancy and self-deadlock reasoning, move the fixed golua class to HISTORY notes. Correct the cancellation comment (the raise IS catchable now; a swallowing script falls back to the grace timeout). Remove two no-op inner recover blocks in the callback dispatchers. - README: the pcall section no longer claims Go-backed errors "abort by design"; documents standard pcall/xpcall and the catchable-cancellation caveat. Tests: - TestGoluaDependencyContract canary-guards the fork dependency through blim-domain behavior (blim.* inside user coroutines; script errors do not degrade the engine) — fails loudly if the replace is dropped to stock golua. - TestProtectedCall gains the restored-globals case and inverts the old "Go-backed error aborts the script" pin to "caught like any Lua error". The exhaustive golua mechanism tests live in the fork; they are deliberately not duplicated here.
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
1 participant
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.
What
Switches blim to the golua fork (
github.com/srgg/golua) that fixes two long-standing defects this codebase worked around, and absorbs the consequences. The fork's changes are upstreamed as aarzilli/golua#130 (thread-correct dispatch) and aarzilli/golua#131 (panic containment at the cgo boundary; also fixes upstream #24).Why
RaiseErroror accidental) used to unwind across in-flightlua_pcall/lua_resumeC frames and corrupt the VM — the root of issue SIGSEGV when a synchronous BLE read is called from inside a subscription callback #3(b) and the disabled GC-at-close workaround (audit L-1).Changes
go.mod/go.sum:replace => github.com/srgg/golua@<pseudo-version>(public fork; CI resolves it). Reverts to upstream once #130/#131 merge.LuaEngine.Close(L-1): re-enable the full-GC-then-stop sequence beforelua_close— the "GC moved crash earlier" corruption was the panic-crossing class, now fixed; finalizers run against an intact registry again.blim.lua: restore standardpcall/xpcallglobals (hidden by golua because Go panics bypassed them unsafely);blim.pcallbecomes a deprecated alias. Go-backed errors are now catchable and the VM stays intact afterwards.api.go,lua_engine.go): rewrite ~14 stale "a Go panic corrupts the recovered lua_State / not catchable by pcall" rationales — keep the still-valid mutex-reentrancy and self-deadlock reasoning, move the fixed golua class to HISTORY notes; correct the cancellation comment; remove two no-op inner recover blocks in the dispatchers.README.md: pcall section no longer claims Go-backed errors "abort by design"; documents standardpcall/xpcalland the catchable-cancellation caveat.Scope note
The in-callback guards stay — they defend the mutex-reentrancy vector (#3(a)), which is a blim architecture constraint independent of golua and is removed only when the RFC-002 actor lands. This PR is dependency adoption + its direct consequences, not the scheduler.
Tests
TestGoluaDependencyContract— canary-guards the fork dependency through blim-domain behavior (blim.*inside user coroutines; script errors do not degrade the engine); fails loudly ifreplaceis dropped to stock golua (verified: it fails with "error in error handling" on stock).TestProtectedCall— adds the restored-globals case and inverts the old "Go-backed error aborts the script" pin to "caught like any Lua error".The exhaustive golua mechanism tests live in the fork and are deliberately not duplicated here.
Full suite green (
-tags "test luajit", incl.-race).