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

Sendability attributes #359

Merged
merged 6 commits into from
Sep 3, 2023
Merged

Sendability attributes #359

merged 6 commits into from
Sep 3, 2023

Conversation

madsmtm
Copy link
Owner

@madsmtm madsmtm commented Jan 18, 2023

Swift recently implemented a better concept of sendability, see NS_SWIFT_SENDABLE and NS_SWIFT_UI_ACTOR in Foundation/NSObjCRuntime.h.

Using these attributes, we can automatically mark NSError, NSDate, NSFileHandle and so on Send + Sync. (That NSLock and subclasses are marked sendable seems a bit like a bug? But it shouldn't matter much, since the methods on it is unsafe anyhow, and deallocation on other threads is definitely safe).

More importantly, we can automatically mark subclasses of NSResponder (NSView, NSWindow, ...) as MainThreadOnly.


To finalize the safety of that, it would be nice to run an algorithm like this on every function/method:

if method.return_type.includes_ns_swift_ui_actor() {
    if method.receiver.is_ns_swift_ui_actor() {
        break;
    }
    for argument in method.arguments {
        if argument.is_ns_swift_ui_actor() {
            break;
        }
    }
    method.arguments.push(("mtm", Ty::MainThreadMarker));
}

(Note here the difference between includes_ns_swift_ui_actor and is_ns_swift_ui_actor; includes_ returns true for NSArray<NSView> or Option<NSView>, while is_ doesn't).

So e.g. NSView::new() -> Id<Self> would become NSView::new(_mtm: MainThreadMarker), while NSWindow::view(&self) -> Id<NSView> would stay as-is.


I checked if we could make atomic attributes safe to access from any thread, and it turns out, we cannot, Swift still requires an await around these, so we must as well (can be tested on https://online.swiftplayground.run/):

import AppKit

@MainActor func test(window: NSWindow) {
    if #available(macOS 10.15, *) {
        Task.detached {
            // Call method; should definitely not be possible without `await`.
            await window.setFrame(NSRect.zero, display: false)

            // Access otherwise `atomic` property; turns out that is not possible without `await` either.
            print(await window.title)

            // Assign to property; likewise not possible without `await`.
            //
            // See https://stackoverflow.com/q/69259397 for why we wrap the assignment
            await MainActor.run {
                window.title = "test"
            }
        }
    }
}

If the property is marked nonisolated though, then it is possible.

@madsmtm madsmtm added enhancement New feature or request A-framework Affects the framework crates and the translator for them labels Jan 18, 2023
@madsmtm madsmtm added this to the icrate v0.1.0 milestone Jan 27, 2023
@madsmtm
Copy link
Owner Author

madsmtm commented Jan 29, 2023

What about allocation? Should we make NSWindow::alloc require MainThreadMarker? Edit: Yes, we should, and we have.

@madsmtm
Copy link
Owner Author

madsmtm commented Apr 21, 2023

A bit unsure where autogenerated _mtm: MainThreadMarker arguments would be most convenient to put? As the first argument (after self, if present), or as the last one?

I'll need to dig up some good examples to see what would be nicest?

This was referenced Apr 21, 2023
@madsmtm madsmtm force-pushed the sendability-attributes branch 4 times, most recently from 3e6d97f to 937ad35 Compare September 1, 2023 22:55
@madsmtm
Copy link
Owner Author

madsmtm commented Sep 1, 2023

We still need to figure out how to handle NonIsolated methods, probably have to do something like implement them multiple times, once for MyClass, and once for MainThreadBound<MyClass>. But that can be done later.

@madsmtm madsmtm mentioned this pull request Sep 1, 2023
4 tasks
@madsmtm madsmtm force-pushed the sendability-attributes branch 3 times, most recently from edcae4d to 69cce30 Compare September 3, 2023 02:25
@madsmtm madsmtm merged commit a9ff910 into master Sep 3, 2023
19 checks passed
@madsmtm madsmtm deleted the sendability-attributes branch September 3, 2023 03:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-framework Affects the framework crates and the translator for them enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant