Skip to content

Fix Swift 6.4 region-isolation errors in the server and app decode paths - #3

Open
Osiriss664 wants to merge 1 commit into
rexmhall09:mainfrom
Osiriss664:fix/swift-6.4-sendable-decode-callbacks
Open

Osiriss664 wants to merge 1 commit into
rexmhall09:mainfrom
Osiriss664:fix/swift-6.4-sendable-decode-callbacks

Conversation

@Osiriss664

Copy link
Copy Markdown

What changed and why

On macOS 27 with Swift 6.4, swift build -c release fails on a clean checkout
of v5.0.1. CI runs on macos-26 (Swift 6.2), where the same code compiles, so
this is not visible upstream yet. Anyone on a current macOS hits it immediately.

Seven errors in two files, all #RegionIsolation::SendingRisksDataRace:

Sources/TUFFServer/Core/ServerInference.swift:1039:47: sending 'stopMatcher' risks causing data races
Sources/TUFFServer/Core/ServerInference.swift:1041:33: sending 'content' risks causing data races
Sources/TUFFServer/Core/ServerInference.swift:1050:27: sending 'calls' risks causing data races
Sources/TUFFServer/Core/ServerInference.swift:1057:32: sending value of non-Sendable type '(RawDecodeProgress) -> Void' risks causing data races
Sources/TUFFServer/Core/ServerInference.swift:1087:32: sending 'shouldStop' risks causing data races
Sources/TUFFApp/Core/Inference/RealInferenceClient.swift:717:38: sending 'responseText' risks causing data races
Sources/TUFFApp/Core/Inference/RealInferenceClient.swift:727:36: sending value of non-Sendable type '(RawDecodeProgress) -> Void' risks causing data races

Both call sites build a progress callback inside an actor-isolated method and
hand it to runRawCompletion, a @concurrent nonisolated global function. Two
things make that callback non-sendable under 6.4:

  1. It captures mutable locals (content, calls, stopMatcher, shouldStop,
    responseText, assistantDecodeError) that are also read after the call
    returns, so region analysis sees a use-after-send.
  2. It calls a local function declared inside the actor method. Swift 6.4
    treats such a local function as actor-isolated, which makes the enclosing
    closure 'self'-isolated. The compiler note states this directly.

The change

Both files now follow the pattern this repository already uses in
RealInferenceClient.swift (ProgressState): keep the per-request mutable
state in a small @unchecked Sendable box, and make the event publisher a
@Sendable closure instead of a local function.

Sources/TUFFServer/Core/ServerInference.swift

  • Adds private final class ServerDecodeState: @unchecked Sendable holding
    stopMatcher, content, calls, decodingError, shouldStop.
  • handle(_:) becomes a @Sendable ([StructuredAssistantEvent]) -> Void
    closure.
  • The runRawCompletion progress closure is marked @Sendable.

Sources/TUFFApp/Core/Inference/RealInferenceClient.swift

  • Moves responseText and assistantDecodeError into the existing
    ProgressState box.
  • publishAssistantEvents becomes a @Sendable closure; its two call sites
    drop the index: and elapsed: argument labels, since closures have none.
  • The progress closure is marked @Sendable.

Design and compatibility decisions

  • Same safety argument as ProgressState. The callback runs synchronously
    inside runRawCompletion on the same task while the actor method awaits it,
    so the boxed state is only ever touched sequentially. That is the reasoning
    already documented on ProgressState; this PR extends it rather than
    inventing a new one.
  • Swift 6.2 compatible. @Sendable closures and @unchecked Sendable
    classes both exist in 6.2, so this should be neutral for the current CI
    toolchain. I could not verify that myself (see Limitations).
  • No behavior change. Control flow, ordering, error handling, and public API
    are unchanged. No runtime switch was added, no production default changed.
  • Guardrails untouched. Nothing about memory bounds, installer staging,
    .gturbo compatibility, fail-closed image input, or the loopback-only server
    is affected.
  • Rejected alternatives. -Xswiftc -swift-version -Xswiftc 5 breaks
    swift-jinja, which uses bare slash regex literals that require Swift 6 mode,
    and then trips over IsolatedDefaultValues in AppModel.swift:366. Running
    the Swift 6.2 toolchain via swiftly crashes the frontend against the macOS 27
    SDK.
  • No new test. This is a compile-time fix with no behavior change, so there
    is nothing new to assert. The build itself is the signal; a macos-27 CI
    runner would cover this class of breakage (see Follow-ups).

Tests run

Machine: MacBook Air M5, 16 GB unified memory, macOS 27, Swift 6.4, Apple
Silicon.

Ran:

  • swift build -c release — succeeds, no errors, no new warnings.
  • Scripts/test.sh — 452 tests in 63 suites, 4 failing.
  • ruby Scripts/check_brand_assets.rb — passed.
  • ruby Scripts/check_markdown_links.rb — passed, 10 Markdown files checked.

Did not run: ruby Scripts/check_app_version.rb (needs a token) and
Scripts/package_app.sh (packaging is untouched by this change).

The four failures are outside the changed code paths, and all four look
specific to this machine:

Test Area Observed
MPPPrefillInt4QMMTests.bf16OutputProjectorPreservesValuesAboveFP16Range Metal kernel The test branches on supportsFamily(.apple10); here a different kernel path is selected and the output is clipped to FP16
RoPETests "GPT-OSS YaRN matches reference", position 131071 Metal kernel One parameter case over the FP16 reduction tolerance
ServerIngressHardeningTests.connectionsBeyondTheCapAreClosed Server ingress Peer reset the connection, .systemCall("connect", 54), instead of closing cleanly
AppModelRestrictionBypassTests.hardwareGateRefusesDownloadAndSelectionUntilBypassed App model gating The hardware gate resolves differently on this machine

A clean-tree baseline is not possible here: without this change the package does
not compile under Swift 6.4, so there is no comparison run. If these four also
fail on qualified hardware they are unrelated to this PR; if they pass there,
they are M5-specific and worth a separate issue. I am happy to file that
separately with full logs.

Real-model checks

Preflight per CONTRIBUTING: macOS 27, Swift 6.4, free disk available, no other
TUFF app, CLI, server, decode service, model test, or MLX process running. One
model process at a time.

  • TUFFRepack --model gemma4 installed, --verify-install passed.
  • TUFFRepack --model qwen36 installed, --verify-install passed.
  • Chat in the Mac app and in TUFFCLI against Gemma 4 26B-A4B and Qwen3.6
    35B-A3B: coherent output, streaming intact, stop handling and structured
    decoding behaved as before.
  • TUFFServer --model scratch/gemma4.gturbo --port 8080: /health,
    /v1/models, and /v1/chat/completions all answer correctly on 127.0.0.1.
  • A document-analysis workload through TUFFCLI (long system prompt plus a
    large pasted text body) produced a correct, complete answer.

These were qualitative checks that the decode path still works, not benchmark
measurements. I am deliberately making no performance claim: my numbers come
from an unqualified Mac and would not be a valid speed result.

Limitations and hardware not available

  • I have no macOS 26 machine, so I could not confirm the change is neutral on
    Swift 6.2. CI should show that.
  • The M5 is not qualified hardware for this project. The four test failures
    above may well be M5-specific rather than related to this change.
  • No screenshots: this PR changes no visible app behavior.

Follow-ups (not in this PR)

  • Consider adding a macos-27 runner to .github/workflows/ci.yml so this
    class of breakage surfaces before release.
  • The Mac app additionally needs full Xcode on macOS 27, because Command Line
    Tools do not ship the SwiftUIMacros plugin that @State now requires.
    Engine, CLI, server, and repack build fine with Command Line Tools alone. That
    is a README note rather than a code change; I can open it separately if you
    want it.

AI assistance

Claude (Opus) was used throughout, and substantially:

  • Diagnosis and implementation: Claude read the compiler output, identified
    the actor-isolation cause, and wrote both diffs. The ServerDecodeState box
    and the @Sendable closure conversions are generated code that I reviewed
    line by line before applying.
  • Review and documentation: Claude also reviewed the repository before I
    built it and drafted this pull request text.
  • What I verified personally: every command in the Tests and Real-model
    sections above was run by me on my own machine, and every number and test name
    in this PR comes from my own logs. Nothing here is reported second-hand or
    from a model's assumption.

Context for the review: I am a security analyst, not a Swift developer. I
understand what this change does and why it is needed, and I can explain the
isolation problem it solves, but I cannot independently judge whether it is the
solution you would prefer for this codebase. If you would rather solve it
differently, for example by marking runRawCompletion as
nonisolated(nonsending) or by restructuring the callbacks, treat this PR as a
reproducible bug report with a working patch attached, and feel free to
implement it your own way. Raw logs for everything above are available on
request.

Swift 6.4 treats a local function declared inside an actor method as
actor-isolated, so the progress callbacks in ServerInference and
RealInferenceClient became 'self'-isolated and could no longer be sent to the
concurrent runRawCompletion. Box the mutable per-request state the same way
ProgressState already does, turn the event publishers into @sendable closures,
and mark both progress callbacks @sendable. No behavior change.

This branch has not been deployed

No deployments
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.

1 participant