Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[6.0][Concurrency] Implement a narrow carve out in the isolation override checking for NSObject.init(). #75749

Merged
merged 1 commit into from
Aug 7, 2024

Conversation

hborla
Copy link
Member

@hborla hborla commented Aug 7, 2024

  • Explanation: [6.0][Concurrency] Don't ignore mismatching isolation for overrides of Clang-imported superclass methods. #74238 added new data-race safety diagnostics for overriding the isolation of a superclass method that's imported from Objective-C. This started diagnosing @MainActor-isolated types that override NSObject.init.

    Now, overriding NSObject.init() within a @MainActor-isolated type is difficult-to-impossible, especially if you need to call an initializer from an intermediate superclass that is also @MainActor-isolated. Standard opt-out tools like MainActor.assumeIsolated cannot be applied to things like stored property initialization and super.init(), making the issue extremely difficult to work around. This is a major usability regression for programs that interoperate with Objective-C and make heavy use of @MainActor-isolated types.

  • Scope: Only impacts isolated overrides of NSObject.init().

  • Issues: Swift 6 regression overriding initializers due to Swift Concurrency #75732, rdar://133349184

  • Original PRs: [Concurrency] Implement a narrow carve out in the isolation override checking for NSObject.init(). #75748

  • Risk: Low; the only effect is removing warnings in complete concurrency checking / errors in Swift 6 mode. This carve-out won't admit a runtime data-race safety hole, because dynamic isolation checks will be inserted in the @objc thunks under DynamicActorIsolation, and direct calls will enforce @MainActor as usual.

  • Testing: Added new tests.

  • Reviewers: TBD

override checking for `NSObject.init()`

Overriding `NSObject.init()` within a `@MainActor`-isolated type
is difficult-to-impossible, especially if you need to call an initializer
from an intermediate superclass that is also `@MainActor`-isolated.
This won't admit a runtime data-race safety hole, because dynamic
isolation checks will be inserted in the @objc thunks under
`DynamicActorIsolation`, and direct calls will enforce `@MainActor`
as usual.

(cherry picked from commit 0d5e598)
@hborla hborla requested a review from a team as a code owner August 7, 2024 06:29
@hborla
Copy link
Member Author

hborla commented Aug 7, 2024

@swift-ci please test

@hborla hborla merged commit 7de7134 into swiftlang:release/6.0 Aug 7, 2024
5 checks passed
@hborla hborla deleted the 6.0-override-nsobject-init branch August 7, 2024 17:32
@andyj-at-aspin
Copy link

@hborla I fear this might also be occurring for other overridden methods of UIKit (maybe originally NS-) objects, such as viewWillAppear(), viewDidAppear() etc.

@hborla
Copy link
Member Author

hborla commented Aug 9, 2024

I fear this might also be occurring for other overridden methods of UIKit (maybe originally NS-) objects, such as viewWillAppear(), viewDidAppear() etc.

I can't reproduce a failure by overriding viewDidAppear in a @MainActor-isolated subclass of UIViewController, but if this is happening for methods that are marked as nonisolated or otherwise aren't annotated with @MainActor, then that is a deliberate result of my original change. Overriding superclass methods and changing isolation does risk a data-race, which is why I made the original change. If you believe the superclass method is incorrectly annotated, that is a framework problem that you should report via Apple Feedback Assistant.

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.

None yet

4 participants