Fix ExtendedContextTests crash: nonisolated deinit on SuggestionSettingsModel#449
Merged
Merged
Conversation
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.
Summary
ExtendedContextTestswas crashing the test host withmalloc: pointer being freed was not allocatedon every test that creates and destroys aSuggestionSettingsModel. All 5 settings-model tests failed as a result (the first crash killed the runner; the restart ran 0 tests).The crash happens in
SuggestionSettingsModel.__deallocating_deinitviaswift_task_deinitOnExecutorMainActorBackDeploy. BecauseSuggestionSettingsModelis@MainActor, Swift schedules the deinit to hop to the main actor executor through the macOS 14 back-deployment shim. That shim has a bug on macOS 26 whereswift::TaskLocal::StopLookupScope::~StopLookupScope()attempts to free stack memory through malloc, triggeringabort().Fix: add
nonisolated deinit {}(SE-0371 / Swift 5.9). This instructs the compiler that the deinit is not actor-isolated and requires no executor hop, bypassing the buggy back-deployment path entirely. All stored state (@Publishedproperty wrappers and theUserDefaultsreference) is thread-safe to release from any thread.Validation
Before this change: 5 failures (
test_setExtendedContext_*), test host crash.After: all 12
ExtendedContextTestspass.Linked issues
Risk / rollout notes
nonisolated deinitis a Swift 5.9 language feature (available in Xcode 15+). The project already builds with Xcode 26. It has no runtime component — it only suppresses compiler-generated executor-hopping in the deinit, which is safe because@Publishedsubjects andUserDefaultshave no main-actor deallocation requirement.Greptile Summary
This PR adds a single
nonisolated deinit {}toSuggestionSettingsModelto fix a test-host crash on macOS 26 caused by a Swift back-deployment shim bug in the main-actor teardown path.nonisolated deinitprevents the compiler from generating an executor-hop to the main actor during deallocation, bypassing theStopLookupScopedouble-free inswift_task_deinitOnExecutorMainActorBackDeploy.@Publishedvalue-type properties and aUserDefaultsreference) has no main-thread deallocation requirement, so the thread-safety claim in the comment is sound.Confidence Score: 5/5
Safe to merge — the change is a single-line, well-scoped fix with no side effects on production behaviour.
The model holds only value-type @published properties and a UserDefaults reference, none of which require main-thread deallocation. No AnyCancellable sets, NotificationCenter observers, or other cleanup work exists in the deinit, so moving teardown off the main-actor executor cannot introduce new problems. In production the model is effectively a singleton and is rarely deallocated, so the change primarily affects test teardown.
No files require special attention.
Important Files Changed
Sequence Diagram
sequenceDiagram participant T as Test / ARC participant S as SuggestionSettingsModel participant MA as MainActor Executor participant Shim as Back-Deploy Shim (macOS 26) Note over T,Shim: Before fix T->>S: last strong reference released S->>MA: schedule deinit hop (compiler-generated) MA->>Shim: swift_task_deinitOnExecutorMainActorBackDeploy Shim-->>T: 💥 malloc: pointer being freed was not allocated Note over T,S: After fix (nonisolated deinit) T->>S: last strong reference released S->>S: deinit runs on current thread (no hop) S-->>T: ✅ stored properties released safelyReviews (1): Last reviewed commit: "Add nonisolated deinit to SuggestionSett..." | Re-trigger Greptile