fix(go)!: eliminate lifecycle deadlocks - #157
Merged
eric8810 merged 3 commits intoAug 18, 2026
Merged
Conversation
cunninghamcard-bit
force-pushed
the
codex/fix-go-lifecycle-deadlocks
branch
from
August 18, 2026 10:09
342c7f6 to
e417fd6
Compare
…any invalid handle aimux_router_new and aimux_moa_new looked each handle up separately and skipped the ones they could not find, failing only when *all* were unknown. A concurrent drop between two lookups — or a caller passing a dead handle — therefore built a composite with fewer members than requested, silently. get_models() now takes the registry lock once, clones every requested Arc, and the constructor fails on the first unknown handle. Membership is all-or-nothing. MoA also rejects a NULL reference pointer with ref_len > 0 instead of reading it as "no references".
cunninghamcard-bit
force-pushed
the
codex/fix-go-lifecycle-deadlocks
branch
from
August 18, 2026 10:15
e417fd6 to
90d7d66
Compare
…call The Go binding held sync.RWMutex read locks across blocking C calls, which formed real wait cycles: TranscriptionSession.Close could not reach the native abort while NextPart(-1) or a backpressured PushAudio held the read lock, and a full stream channel pinned the C stream call under the read lock while Model.Close waited for the write lock. Router/MoA had the opposite defect: they read each child's handle under the lock and released it before the C call, so a concurrent Close in that window freed a handle mid-construction and the FFI silently dropped that child. Every native owner (Model, ProviderHandle, the multimodal models, TranscriptionSession) is now an atomic.Uint64 with 0 = closed. A call snapshots the id and KeepAlives the owner until C returns; Close is Swap(0) + drop and never waits for Go state — a call that already entered the Rust registry owns a cloned Arc and finishes, a later one gets the closed error. Router/MoA snapshot ids in caller order and rely on the FFI's all-or-nothing lookup; no multi-lock protocol. The stream producer keeps its structure and merely stops holding a lock across the C call. Breaking: Close is no longer a join; owners must not be copied after first use (go vet now flags it); Router/MoA reject any invalid member instead of dropping it. Finalizers stay a best-effort leak fallback.
…ancel cause No behaviour change beyond preserving the caller's cancellation cause. The producer goroutine is now the only writer of err and the only closer of Parts, so the channel close is the publication point and Err needs no lock; goStreamDone becomes a no-op (the C call returns right after on_done). The handwritten id → entry registry and its mutex are replaced by runtime/cgo.Handle; the cancelled channel + once + cause fields by context.WithCancelCause; the watcher goroutine by context.AfterFunc. The producer re-checks context.Cause after the C call returns so a cancellation that lands during the last part is not lost. Typed and OpenAI adapters follow the same shape. Stream/TypedStream/OpenAIStream.Err must be read after Parts closes.
cunninghamcard-bit
force-pushed
the
codex/fix-go-lifecycle-deadlocks
branch
from
August 18, 2026 10:18
90d7d66 to
80cebbc
Compare
cunninghamcard-bit
added a commit
to cunninghamcard-bit/aimux
that referenced
this pull request
Aug 19, 2026
…RFC-0030) Every fallible C function now returns `aimux_error_t *` — NULL on success with the result in a trailing out-parameter, non-NULL on failure, released exactly once with `aimux_error_free()`. The caller-allocated `AimuxError` struct, `aimux_error_clear`, `error_value` and the legacy `0 / -1` and `1 / 0` polarities are gone; there is one rule for every entry point. There is one code space, read with `aimux_error_code()`, segmented the way libsignal segments its FFI codes: 1–13 the AiMuxError variants (numbers unchanged), 100–105 the recorder's RecordingError, 200–206 failures the C boundary itself detects (NULL pointer, invalid UTF-8, malformed wire JSON, dead handle, re-entrant call, unserializable result, callback failure). A non-NULL error never carries code 0. `aimux_error_message()` answers for every code; the AiMuxError detail getters (`_retryable`, `_status`, `_retry_ms`, `_provider_code`, `_provider_message`, `_request_id`, `_response_body`, `_model_id`, `_model_type`, `_provider_id`) take the same pointer and answer NULL / -1 / 0 when the code does not own the field. There are no projections, no error family enum and no error registry: an error is a `Box::into_raw` handed to the caller, never a u64 handle. Kotlin, Java, Swift, Flutter and Go rebuild the separate types from the code range — the engine's error hierarchy, a distinct recording error, and the language's own invariant/plain error for 200–206 — so the C type system never has to express Rust's. Node and Python are untouched: they do not cross the C ABI. Go keeps master's (arcships#157) lifecycle model; only the ABI call sites and the error decoder change. Router/MoA member lookup rejects any dead handle (subsuming arcships#157's `get_models`). The C/C++ examples, docs/api/* and RFC-0030 follow. Header export count in `header_and_exports_agree` updated.
cunninghamcard-bit
added a commit
to cunninghamcard-bit/aimux
that referenced
this pull request
Aug 19, 2026
…RFC-0030) Every fallible C function now returns `aimux_error_t *` — NULL on success with the result in a trailing out-parameter, non-NULL on failure, released exactly once with `aimux_error_free()`. The caller-allocated `AimuxError` struct, `aimux_error_clear`, `error_value` and the legacy `0 / -1` and `1 / 0` polarities are gone; there is one rule for every entry point. There is one code space, read with `aimux_error_code()`, segmented the way libsignal segments its FFI codes: 1–13 the AiMuxError variants (numbers unchanged), 100–105 the recorder's RecordingError, 200–206 failures the C boundary itself detects (NULL pointer, invalid UTF-8, malformed wire JSON, dead handle, re-entrant call, unserializable result, callback failure). A non-NULL error never carries code 0. `aimux_error_message()` answers for every code; the AiMuxError detail getters (`_retryable`, `_status`, `_retry_ms`, `_provider_code`, `_provider_message`, `_request_id`, `_response_body`, `_model_id`, `_model_type`, `_provider_id`) take the same pointer and answer NULL / -1 / 0 when the code does not own the field. There are no projections, no error family enum and no error registry: an error is a `Box::into_raw` handed to the caller, never a u64 handle. Kotlin, Java, Swift, Flutter and Go rebuild the separate types from the code range — the engine's error hierarchy, a distinct recording error, and the language's own invariant/plain error for 200–206 — so the C type system never has to express Rust's. Node and Python are untouched: they do not cross the C ABI. Go keeps master's (arcships#157) lifecycle model; only the ABI call sites and the error decoder change. Router/MoA member lookup rejects any dead handle (subsuming arcships#157's `get_models`). The C/C++ examples, docs/api/* and RFC-0030 follow. Header export count in `header_and_exports_agree` updated.
cunninghamcard-bit
added a commit
to cunninghamcard-bit/aimux
that referenced
this pull request
Aug 19, 2026
…RFC-0030) Every fallible C function now returns `aimux_error_t *` — NULL on success with the result in a trailing out-parameter, non-NULL on failure, released exactly once with `aimux_error_free()`. The caller-allocated `AimuxError` struct, `aimux_error_clear`, `error_value` and the legacy `0 / -1` and `1 / 0` polarities are gone; there is one rule for every entry point. There is one code space, read with `aimux_error_code()`, segmented the way libsignal segments its FFI codes: 1–13 the AiMuxError variants (numbers unchanged), 100–105 the recorder's RecordingError, 200–206 failures the C boundary itself detects (NULL pointer, invalid UTF-8, malformed wire JSON, dead handle, re-entrant call, unserializable result, callback failure). A non-NULL error never carries code 0. `aimux_error_message()` answers for every code; the AiMuxError detail getters (`_retryable`, `_status`, `_retry_ms`, `_provider_code`, `_provider_message`, `_request_id`, `_response_body`, `_model_id`, `_model_type`, `_provider_id`) take the same pointer and answer NULL / -1 / 0 when the code does not own the field. There are no projections, no error family enum and no error registry: an error is a `Box::into_raw` handed to the caller, never a u64 handle. Kotlin, Java, Swift, Flutter and Go rebuild the separate types from the code range — the engine's error hierarchy, a distinct recording error, and the language's own invariant/plain error for 200–206 — so the C type system never has to express Rust's. Node and Python are untouched: they do not cross the C ABI. Go keeps master's (arcships#157) lifecycle model; only the ABI call sites and the error decoder change. Router/MoA member lookup rejects any dead handle (subsuming arcships#157's `get_models`). The C/C++ examples, docs/api/* and RFC-0030 follow. Header export count in `header_and_exports_agree` updated.
cunninghamcard-bit
added a commit
to cunninghamcard-bit/aimux
that referenced
this pull request
Aug 19, 2026
…RFC-0030) Every fallible C function now returns `aimux_error_t *` — NULL on success with the result in a trailing out-parameter, non-NULL on failure, released exactly once with `aimux_error_free()`. The caller-allocated `AimuxError` struct, `aimux_error_clear`, `error_value` and the legacy `0 / -1` and `1 / 0` polarities are gone; there is one rule for every entry point. There is one code space, read with `aimux_error_code()`, segmented the way libsignal segments its FFI codes: 1–13 the AiMuxError variants (numbers unchanged), 100–105 the recorder's RecordingError, 200–206 failures the C boundary itself detects (NULL pointer, invalid UTF-8, malformed wire JSON, dead handle, re-entrant call, unserializable result, callback failure). A non-NULL error never carries code 0. `aimux_error_message()` answers for every code; the AiMuxError detail getters (`_retryable`, `_status`, `_retry_ms`, `_provider_code`, `_provider_message`, `_request_id`, `_response_body`, `_model_id`, `_model_type`, `_provider_id`) take the same pointer and answer NULL / -1 / 0 when the code does not own the field. There are no projections, no error family enum and no error registry: an error is a `Box::into_raw` handed to the caller, never a u64 handle. Kotlin, Java, Swift, Flutter and Go rebuild the separate types from the code range — the engine's error hierarchy, a distinct recording error, and the language's own invariant/plain error for 200–206 — so the C type system never has to express Rust's. Node and Python are untouched: they do not cross the C ABI. Go keeps master's (arcships#157) lifecycle model; only the ABI call sites and the error decoder change. Router/MoA member lookup rejects any dead handle (subsuming arcships#157's `get_models`). The C/C++ examples, docs/api/* and RFC-0030 follow. Header export count in `header_and_exports_agree` updated.
cunninghamcard-bit
added a commit
to cunninghamcard-bit/aimux
that referenced
this pull request
Aug 19, 2026
…RFC-0030) Every fallible C function now returns `aimux_error_t *` — NULL on success with the result in a trailing out-parameter, non-NULL on failure, released exactly once with `aimux_error_free()`. The caller-allocated `AimuxError` struct, `aimux_error_clear`, `error_value` and the legacy `0 / -1` and `1 / 0` polarities are gone; there is one rule for every entry point. There is one code space, read with `aimux_error_code()`, segmented the way libsignal segments its FFI codes: 1–13 the AiMuxError variants (numbers unchanged), 100–105 the recorder's RecordingError, 200–206 failures the C boundary itself detects (NULL pointer, invalid UTF-8, malformed wire JSON, dead handle, re-entrant call, unserializable result, callback failure). A non-NULL error never carries code 0. `aimux_error_message()` answers for every code; the AiMuxError detail getters (`_retryable`, `_status`, `_retry_ms`, `_provider_code`, `_provider_message`, `_request_id`, `_response_body`, `_model_id`, `_model_type`, `_provider_id`) take the same pointer and answer NULL / -1 / 0 when the code does not own the field. There are no projections, no error family enum and no error registry: an error is a `Box::into_raw` handed to the caller, never a u64 handle. Kotlin, Java, Swift, Flutter and Go rebuild the separate types from the code range — the engine's error hierarchy, a distinct recording error, and the language's own invariant/plain error for 200–206 — so the C type system never has to express Rust's. Node and Python are untouched: they do not cross the C ABI. Go keeps master's (arcships#157) lifecycle model; only the ABI call sites and the error decoder change. Router/MoA member lookup rejects any dead handle (subsuming arcships#157's `get_models`). The C/C++ examples, docs/api/* and RFC-0030 follow. Header export count in `header_and_exports_agree` updated.
cunninghamcard-bit
added a commit
to cunninghamcard-bit/aimux
that referenced
this pull request
Aug 19, 2026
The Go binding reads the new ABI: every call gets an `aimux_error_t *`, NULL means success, the result arrives through an out-parameter, and the error is freed once. `error.go` rebuilds the separate types from the code range — the `AimuxError` hierarchy for 1–13, a distinct `RecordingError` for 100–105, and a plain Go error for the C boundary's own 200–206, which callers should never have to branch on. The arcships#157 lifecycle model is unchanged: atomic handle owners, Close that never waits for an in-flight C call. Only the ABI call sites and the error decoder move.
cunninghamcard-bit
added a commit
to cunninghamcard-bit/aimux
that referenced
this pull request
Aug 19, 2026
The Go binding reads the new ABI: every call gets an `aimux_error_t *`, NULL means success, the result arrives through an out-parameter, and the error is freed once. `error.go` rebuilds the separate types from the code range — the `AimuxError` hierarchy for 1–13, a distinct `RecordingError` for 100–105, and a plain Go error for the C boundary's own 200–206, which callers should never have to branch on. The arcships#157 lifecycle model is unchanged: atomic handle owners, Close that never waits for an in-flight C call. Only the ABI call sites and the error decoder move.
eric8810
pushed a commit
that referenced
this pull request
Aug 22, 2026
* refactor(node,python)!: throw native typed errors Node and Python now project each core error variant into a real runtime exception class instead of rebuilding a generic metadata object. Node registers the canonical JavaScript constructors with napi-rs and Rust instantiates them in the owning environment. Synchronous throws, promise rejections, streams and workers therefore preserve instanceof. Python raises the matching PyO3 exception class. Variant payload lives only on the class that owns it: APICallError carries provider-call details, TokenExpiredError carries its defined 401 status, and the registry errors carry their identifiers. AimuxError has no compatibility code, status or retryable fields, and the serialized errorValue/error_value companion is removed. RecordingError remains a second family, unrelated to AimuxError, for recorder construction and flush failures. Binding-layer failures keep the native framework behavior: napi-rs Error statuses in Node and ValueError or RuntimeError in Python. * feat(ffi)!: one opaque error pointer and one unified code space Every fallible C function now returns aimux_error_t *: NULL on success with the result in a trailing out-parameter, or one owned error on failure, released exactly once with aimux_error_free(). The caller-allocated AimuxError struct, aimux_error_clear, error_value and mixed sentinel polarities are removed. One code space identifies the source. AiMuxError uses 1–13: Other takes the retired UNKNOWN slot at 1, codes 2–13 retain their existing values, and the old Other value 14 is retired. RecordingError uses 100–105 and C-boundary failures use 200–206. A non-NULL error never reports code 0. aimux_error_message() works for every code. Detail getters read only the payload owned by that variant and return NULL, -1 or 0 otherwise. The error is a Box transferred directly to the caller, not a registry handle or a JSON projection. Router and MoA constructors reject every dead member handle instead of silently changing the composite. The C and C++ examples use the same ownership and result convention. * refactor(go)!: decode the unified error codes The Go binding reads the new ABI: every call gets an `aimux_error_t *`, NULL means success, the result arrives through an out-parameter, and the error is freed once. `error.go` rebuilds the separate types from the code range — the `AimuxError` hierarchy for 1–13, a distinct `RecordingError` for 100–105, and a plain Go error for the C boundary's own 200–206, which callers should never have to branch on. The #157 lifecycle model is unchanged: atomic handle owners, Close that never waits for an in-flight C call. Only the ABI call sites and the error decoder move. * refactor(java,kotlin)!: decode the unified error codes Both JVM bindings follow the new ABI: an opaque error pointer per call, freed once, with the payload read through the getters. `AimuxCError` is gone — the code range now decides the type: `AimuxException` and its subclasses for 1–13, a separate `RecordingException` / `RecordingErrorCode` for 100–105, and the language's own `IllegalStateException` / `IllegalArgumentException` for the C boundary's 200–206. * refactor(swift,flutter)!: decode the unified error codes Swift and Flutter follow the new ABI the same way: one opaque error pointer per call, freed once, payload through the getters, and the code range rebuilding the separate types — the engine hierarchy for 1–13, a distinct recording error for 100–105, and the language's own invariant error for the C boundary's 200–206. * docs(api): the error model each binding now exposes Rewrites the error sections for C, Go, Java, Kotlin, Swift, Flutter, Node and Python: the return convention, the code ranges and which type each range becomes in that language. * test(aimux-web): isolate API key env cases
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.
Problem
The Go binding held
sync.RWMutexread locks across blocking C calls, which formed real wait cycles onmaster:TranscriptionSession.Closecannot reach the native abort whileNextPart(-1)or a backpressuredPushAudioholds the read lock — the two block each other forever.defer m.Close()after an earlybreakout ofParts()waits for the write lock forever.Router/MoA had the opposite defect: constructors read each child's handle under its read lock and released it before the C call, so a concurrent
Closein that window freed a handle mid-construction — and the FFI then silently dropped that child, building a smaller composite than requested.What changed — three commits
fix(ffi)—aimux_router_new/aimux_moa_newresolve every member under one registry lock and fail on the first unknown handle. Membership is all-or-nothing; MoA also rejectsNULLreferences withref_len > 0.fix(go)!— every native owner (Model,ProviderHandle, the multimodal models,TranscriptionSession) becomes anatomic.Uint64with0 == closed. Calls snapshot the id andruntime.KeepAlivethe owner until C returns;CloseisSwap(0)+ drop and never waits for Go state. Router/MoA snapshot ids in caller order — no multi-lock protocol. The stream producer keeps its structure and only stops holding a lock across the C call. This commit alone removes the deadlocks.refactor(go)— stream internals: the producer goroutine is the sole writer oferrand sole closer ofParts(the channel close is the publication point, soErrneeds no lock); the handwritten callback registry is replaced byruntime/cgo.Handle; the cancelled channel/once/cause fields bycontext.WithCancelCause; the watcher goroutine bycontext.AfterFunc. The producer re-checkscontext.Causeafter the C call returns so a cancellation landing during the last part is not lost. Typed and OpenAI adapters follow the same shape. No behaviour change beyond preserving the caller's cancellation cause.Compatibility
Public Go method signatures and C ABI signatures are unchanged. Breaking for Go callers:
Closeis no longer a join/barrier. A racing call may finish, or receive the existing invalid-handle error.Stream.Err,TypedStream.Err, andOpenAIStream.Errmust be read afterParts()closes; they no longer wait internally.go vetflag it).Finalizers remain a best-effort leak fallback; callers still own explicit
Close.This PR intentionally contains no error-layering work, no new error classes, and no Node/Python changes. It keeps the current flat
AimuxError*/sentinel ABI.Verification
Each commit builds and passes on its own.
cargo test -p aimux-ffi,cargo clippy -p aimux-ffi --all-targets -- -D warningsgo build ./...,go vet ./...,go test -race -count=1 ./...NextPart(-1) + Close, backpressuredPushAudio + Close, full stream bufferCancel + Model.Close,Closeracing an in-flightListModels/ multimodal callClose; zero-valueModel; concurrent idempotentClose