Skip to content

fix(asr): prevent use-after-free in MLX callback on cancel#27

Merged
missuo merged 1 commit into
missuo:mainfrom
erning:fix/mlx-close-use-after-free
Mar 29, 2026
Merged

fix(asr): prevent use-after-free in MLX callback on cancel#27
missuo merged 1 commit into
missuo:mainfrom
erning:fix/mlx-close-use-after-free

Conversation

@erning

@erning erning commented Mar 29, 2026

Copy link
Copy Markdown
Collaborator

Problem

When Rust calls close() on the MLX provider, it invokes koe_mlx_cancel() then immediately frees the leaked Box<Sender> via reclaim_sender(). However, Swift's Task.cancel() is cooperative — the eventTask may still be running and can invoke the C callback with the already-freed context pointer, leading to a use-after-free crash.

Solution

Add an NSLock to MLXAsrManager:

  • cancel() clears callback/callbackCtx under the lock before cancelling the task
  • invokeCallback() holds the lock for the entire C callback invocation

This guarantees the context pointer is never dereferenced after Rust reclaims it. A safety-invariant comment is added to the Rust side.

Files changed

  • Packages/KoeMLX/Sources/KoeMLX/MLXAsrManager.swift
  • koe-asr/src/mlx.rs (comment only)

Test plan

  • make build passes
  • Hold-to-talk and tap-to-toggle work normally
  • Rapid cancel during recording does not crash

When Rust calls close() on the MLX provider, it invokes
koe_mlx_cancel() then immediately frees the leaked Sender pointer.
But Swift's Task.cancel() is cooperative — the eventTask may still
invoke the C callback with the freed context pointer.

Add an NSLock to MLXAsrManager so that cancel() clears callback/ctx
under the lock before cancelling the task, and invokeCallback() holds
the lock for the entire C callback invocation. This guarantees the
pointer is never dereferenced after Rust reclaims it.
@missuo missuo merged commit 87d343d into missuo:main Mar 29, 2026
@erning erning deleted the fix/mlx-close-use-after-free branch March 29, 2026 14:50
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