fix: reject blocking/state-releasing Lua ops inside callbacks (#3) - #5
Merged
Conversation
Calling a synchronous blocking primitive from inside a subscription/PTY callback either corrupted or froze the shared lua_State: - blim.sleep and io.read (and blim.term.read_char via its polling loop) release the single stateMutex mid-L.Call, letting another DoWithState reenter the in-flight state from another goroutine -> SIGSEGV in lua_getinfo. - characteristic.read()/write() and blim.subscribe do not release the mutex but block the callback goroutine on a synchronous BLE round-trip while holding the state, freezing the main loop and every other callback and risking a deadlock with the BLE event path. Add a single chokepoint guard, LuaEngine.raiseIfInCallback(L, op): while a callback holds the state (inCallbackCount > 0) these ops raise a clean, recoverable Lua error instead of proceeding. Self-cancel via the callback's cancel() argument stays allowed. inCallbackCount moves to LuaEngine (owner of the state and mutex) so LuaEngine-registered io.read can consult it without depending upward on LuaAPI, and its increment/decrement is made panic-safe (deferred) so a StackTrace-crash panic in L.Call can no longer leak the counter. Add deterministic regression tests for all five guarded ops, grouped under a dedicated TestCallbackBlockingOpGuards. Document the safe-blocking (coroutine scheduler) redesign as deferred future work in RFC-001 and a project memory. Fixes #3
Add .github/workflows/ci.yaml running gofmt check, go vet and the race detector (`-race -count=1`) with the luajit+test build tags on pull requests and pushes to main. It regenerates the .gitignore'd mocks / *_depend_test.go via `make generate` before building. Extract the shared Go + LuaJIT + CGO/pkg-config setup into a local composite action .github/actions/setup-luajit and refactor release.yaml to use it (static-linking: true for release binaries, dynamic for CI).
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.
Summary
Fixes the SIGSEGV (and related engine-freeze) when a synchronous blocking Lua
operation is called from inside a subscription/PTY callback.
Two hazard classes on the single,
stateMutex-guardedlua_State:blim.sleepandio.read— andtransitively
blim.term.read_char(wait_ms)via its polling loop — release themutex mid-
L.Call, letting anotherDoWithStatereenter the in-flight statefrom another goroutine and corrupt it → SIGSEGV in
lua_getinfo.characteristic.read()/write()andblim.subscribedo a synchronous BLE round-trip while holding the mutex,freezing the main loop and every other callback and risking a deadlock with
the BLE event path.
Change
LuaEngine.raiseIfInCallback(L, op): while a callbackholds the state (
inCallbackCount > 0) these five ops raise a clean,recoverable Lua error instead of proceeding. Self-cancel via the callback's
cancel()argument stays allowed.inCallbackCountmoved toLuaEngine(owner of the state and mutex) soLuaEngine-registeredio.readcan consult it without an upward dependencyon
LuaAPI; its increment/decrement is made panic-safe (deferred) so aStackTrace-crash panic in
L.Callcan no longer leak the counter.Tests
TestCallbackBlockingOpGuards.internal/luasuite green,-race -count=1clean,go vetand
gofmtclean; legitimate main-loopblim.sleep/characteristic.readpaths unaffected.
Follow-ups (out of scope, documented)
as deferred work in
RFC-001-lua-callback-coroutine-scheduler.md.a fault-injection seam (not deterministically reproducible in the harness).
Fixes #3