Skip to content

fix(go)!: eliminate lifecycle deadlocks - #157

Merged
eric8810 merged 3 commits into
arcships:masterfrom
cunninghamcard-bit:codex/fix-go-lifecycle-deadlocks
Aug 18, 2026
Merged

fix(go)!: eliminate lifecycle deadlocks#157
eric8810 merged 3 commits into
arcships:masterfrom
cunninghamcard-bit:codex/fix-go-lifecycle-deadlocks

Conversation

@cunninghamcard-bit

@cunninghamcard-bit cunninghamcard-bit commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Problem

The Go binding held sync.RWMutex read locks across blocking C calls, which formed real wait cycles on master:

  • TranscriptionSession.Close cannot reach the native abort while NextPart(-1) or a backpressured PushAudio holds the read lock — the two block each other forever.
  • A full stream channel pins the blocking C stream call under the read lock, so a defer m.Close() after an early break out of Parts() 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 Close in 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

  1. fix(ffi)aimux_router_new / aimux_moa_new resolve every member under one registry lock and fail on the first unknown handle. Membership is all-or-nothing; MoA also rejects NULL references with ref_len > 0.
  2. fix(go)! — every native owner (Model, ProviderHandle, the multimodal models, TranscriptionSession) becomes an atomic.Uint64 with 0 == closed. Calls snapshot the id and runtime.KeepAlive the owner until C returns; Close is Swap(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.
  3. refactor(go) — stream internals: the producer goroutine is the sole writer of err and sole closer of Parts (the channel close is the publication point, so Err needs no lock); the handwritten callback registry is 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 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:

  • Close is no longer a join/barrier. A racing call may finish, or receive the existing invalid-handle error.
  • Stream.Err, TypedStream.Err, and OpenAIStream.Err must be read after Parts() closes; they no longer wait internally.
  • Handle wrapper values must not be copied after first use (already unsafe with the embedded mutex; atomics make go vet flag it).
  • Router/MoA now reject any invalid child/reference instead of silently dropping 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 warnings
  • go build ./..., go vet ./..., go test -race -count=1 ./...
  • deadline regressions: NextPart(-1) + Close, backpressured PushAudio + Close, full stream buffer Cancel + Model.Close, Close racing an in-flight ListModels / multimodal call
  • repeated race coverage: duplicate / reversed Router and MoA construction against concurrent Close; zero-value Model; concurrent idempotent Close

@cunninghamcard-bit
cunninghamcard-bit force-pushed the codex/fix-go-lifecycle-deadlocks branch from 342c7f6 to e417fd6 Compare August 18, 2026 10:09
…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
cunninghamcard-bit force-pushed the codex/fix-go-lifecycle-deadlocks branch from e417fd6 to 90d7d66 Compare August 18, 2026 10:15
…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
cunninghamcard-bit force-pushed the codex/fix-go-lifecycle-deadlocks branch from 90d7d66 to 80cebbc Compare August 18, 2026 10:18
@eric8810
eric8810 merged commit a35f1df into arcships:master Aug 18, 2026
24 checks passed
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
cunninghamcard-bit deleted the codex/fix-go-lifecycle-deadlocks branch August 19, 2026 03:54
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants