Fix Sendable soundness and process-stop escalation - #38
Merged
Conversation
- ChatterboxRandomGenerator: guard the SplitMix64 state and cached spare normal with OSAllocatedUnfairLock so the @unchecked Sendable conformance is actually sound. Static unlocked helpers keep nextNormal from re-entering the lock, and the draw sequence for a given seed is unchanged. - CoreAIConversionProcessRunner.stop(): escalate SIGINT -> SIGTERM -> SIGKILL with 1s grace periods, and replace the synchronous waitUntilExit() (which could wedge the actor if the child ignored SIGTERM) with non-blocking polling. Polling is used because execute() already owns the process terminationHandler. - Document the @unchecked Sendable invariants on SendableImageSegmenter, ScopedResourceLease, and SendableObjectDetector.
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.
What
Two targeted concurrency fixes plus documentation of audited
@unchecked Sendableboundaries.1.
ChatterboxRandomGenerator— make@unchecked SendablesoundCoreAILabCore/Chatterbox/ChatterboxSampling.swiftdeclared@unchecked Sendableon a class with unsynchronized mutable state (state,spareNormal). The mutable state is now held in a singleStatestruct guarded byOSAllocatedUnfairLock(deployment target is 27.0, so it is available everywhere the core builds).nextNormaluses static unlocked helpers so it never re-enters the lock. The draw sequence for a given seed is byte-for-byte unchanged, so seeded determinism is preserved (verified byChatterboxFunctionContractTests). The uncontended lock cost is negligible next to thesqrt/log/sinmath in the noise loop.2.
CoreAIConversionProcessRunner.stop()— SIGKILL escalation, no blocking waitPreviously: SIGINT → SIGTERM, then a synchronous
process.waitUntilExit()inside the actor — a child that ignored SIGTERM would wedge the actor forever, and nothing ever sent SIGKILL. Now: SIGINT → 1s grace → SIGTERM → 1s grace → SIGKILL, with all waiting done by non-blockingisRunningpolling (50 ms intervals). Polling rather than a secondterminationHandlerbecauseexecute()already owns the handler to feed its termination stream. If the surrounding task is cancelled, the grace period is skipped and escalation proceeds immediately instead of busy-spinning.3. Documented
@unchecked SendableinvariantsSendableImageSegmenter(AppleImageSegmenterEngine.swift): actor-confined toAppleImageSegmenterEngine.ScopedResourceLease(AppleDiffusionPipelineEngine.swift): immutable after init; deinit only balances the scoped access.SendableObjectDetector(AppleObjectDetectorEngine.swift): existing comment extended to note actor confinement.Verification
Full LFS model weights pulled; complete suite run on Xcode 27 beta 2:
All 33 suites passed, including the two directly covering this change:
ChatterboxFunctionContractTests(seeded-determinism contract) andCoreAIConversionProcessRunnerTests(launch/cancel/child-process lifecycle).