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

Add a way to mark things that can only be done on the main thread #27

Closed
madsmtm opened this issue Sep 5, 2021 · 9 comments · Fixed by #178
Closed

Add a way to mark things that can only be done on the main thread #27

madsmtm opened this issue Sep 5, 2021 · 9 comments · Fixed by #178
Labels
enhancement New feature or request
Milestone

Comments

@madsmtm
Copy link
Owner

madsmtm commented Sep 5, 2021

Many things on NSApplication (e.g. -run, ...) require being done on the main thread; this is something we should clearly communicate in the type system!

See the winit code for more examples of where it's required (NSWindow, ...).
See also the following links:

Idea:

// No lifetime information needed; the main thread is static and available through the entire program!
// And even if it wasn't, the marker can't be passed to another thread
struct MainThreadMarker {
   _priv: (),
}

impl !Send for MainThreadMarker {}
impl !Sync for MainThreadMarker {}

impl MainThreadMarker {
    fn new() -> Option<Self> {
        if is_main_thread!() {
            Some(Self {
                _priv: ()
            })
        } else {
            None
        }
    }
    fn new_unchecked() -> Self {
        ...
    }
}

// This is valid to clone because it's still `!Send` and `!Sync`.
impl Clone for MainThreadMarker {...}
impl Copy for MainThreadMarker {}

// Usage
impl NSWindow {
    fn do_thing(_mt: MainThreadMarker) {
        // This action requires the main thread, so we take one as a parameter.
        // It signals clearly to users "hey, this requires the main thread"
    }
}

impl dispatch::Queue {
    fn run_on_main_async(f: impl FnOnce(mt: MainThreadMarker) + Send) {}

    // fn run_on_main_sync ...
}

Queue::run_on_main_async(|mt| {
    ns_window.do_thing(mt);
})
@madsmtm madsmtm added the enhancement New feature or request label Sep 5, 2021
@madsmtm
Copy link
Owner Author

madsmtm commented Sep 5, 2021

More ideas for how we can ensure something is only accessed on the main thread (e.g. after giving it to NSApplication, whom will mutate it (but on the main thread only, and only when we're not using it)):

// Or ThisThreadOnly, ThreadLocked or something
struct MainThreadOnly<T>(T);

impl<T> !Send for MainThreadOnly<T> {}
impl<T> !Sync for MainThreadOnly<T> {}

// Allows
// `&'a MainThreadOnly<T>`
// -> `&'a T` (may be Sync)
impl<T> Deref for MainThreadOnly<T> { ... }

impl NSApplication {
    fn set_main_menu(&self, menu: Id<NSMenu, Owned>) -> Id<MainThreadOnly<NSMenu>, Shared>;
}

@madsmtm
Copy link
Owner Author

madsmtm commented Jun 7, 2022

A big difficulty here lies in the interior-mutability nature of what we're trying to do; we'd like to be able to call mutating methods such as:

  • NSApplication -setDelegate:
  • NSApplication -setPresentationOptions:
  • NSApplication -hide:
  • NSApplication -run
  • NSResponder -interpretKeyEvents:
  • NSTextInputContext -discardMarkedText
  • NSView -removeTrackingRect:
  • NSView -addCursorRect:
  • NSWindow -setMinSize:

At some point in time where we only have access to Id<T, Shared>.

I'll try draw some inspiration / knowledge from the std::cell types, GhostCell and the qcell crate.

@madsmtm
Copy link
Owner Author

madsmtm commented Jun 7, 2022

A complicating factor is that if I change something from the main thread, e.g. on NSApplication, another thread may observe this change - so while we'll limit our mutating to the main thread, read accesses can still (so far, unless we do something else) happen on another thread!

Note that @property accesses are actually atomic by default, which helps a lot, but I'm not sure how much yet.

@madsmtm
Copy link
Owner Author

madsmtm commented Jun 7, 2022

The threadbound crate is also interesting!

@madsmtm
Copy link
Owner Author

madsmtm commented Jun 8, 2022

Reading Apple's docs again, they distinguish between Thread-Safe, Thread-Unsafe and Main Thread Only (likely written at some point after Mike Ash's article). Let's examine each of these in relation to Rust:

  1. Thread-Safe Classes
    Easy, most of these are just immutable classes! We'll simply use &self everywhere in their implementation, and can easily mark them Send and Sync.
    Special case: NSObject is not Send + Sync since it's subclasses may not be thread safe (and e.g. we could do &T -> &NSObject and pass that to another thread) - though I'll need to think a bit harder (see Downgrading using Deref #58) about what we want to allow and disallow here!
    Special case: NSLock cannot be Send, it must be destroyed on the same thread that created it.

  2. Thread-Unsafe Classes
    A bit harder, but still doable, most of these simply have the restriction that they cannot be mutated from more than one thread at a time - so we simply use &mut self wherever mutation happens, and we're good to mark them Send and Sync.
    Special case: NSAutoreleasePool cannot be Send, it must be destroyed on the same thread that created it.
    Special case: The backing store for NSEnumerator cannot be mutated while it exists, this can be handled by a lifetime parameter.

  3. Main Thread Only Classes
    And this one is what we're trying to solve in this issue - though the term is quite ill-defined, as-in, some of the operations on these classes are actually thread safe, but it's very hard to tell which ones!

And everything is complicated by the fact that we'd like to share an object with another object, while still sometimes mutating it, so e.g. NSWindow is probably sometimes just thread-unsafe, and sometimes Main Thread Only... Actually, NSWindow has very special requirements, no wonder that winit has trouble with this!

Oh, another thing: A lot of stuff in AppKit requires Cocoa to be in multithreading mode to even work on multiple threads, so we'll need a way to ensure that as well...

@madsmtm
Copy link
Owner Author

madsmtm commented Jun 8, 2022

Most mutation happens either through @property setters/getters, or with methods that simply mutate something on the receiver - in those cases, the accesses are pretty similar to what Cell does.

@madsmtm
Copy link
Owner Author

madsmtm commented Jun 19, 2022

This functionality should live in objc2_foundation, since it is something very Cocoa-specific, not something that is inherent to the Objective-C language

@madsmtm
Copy link
Owner Author

madsmtm commented Jun 19, 2022

I think the example in #27 (comment) is a bit of a different use-case, and I doubt it is safe to do in any way (once we have given NSMenu to NSApplication, it may be mutated on the main thread at any point, and hence is no longer thread safe - not even sure it is safe to create on a different thread to begin with).

The MainThreadOnly idea is not entirely off, I just think it should be the following instead:

// Or ThisThreadOnly, ThreadLocked or something
#[repr(transparent)]
struct MainThreadOnly<T>(T);

// Creation methods 
impl<T> MainThreadOnly<T> {
    fn from_ref(obj: &T) -> &Self {
        assert!(is_main_thread());
        todo!()
    }

    fn from_mut(obj: &mut T) -> &mut Self {
        assert!(is_main_thread());
        todo!()
    }

    fn from_id<O: Ownership>(obj: Id<T, O>) -> Id<Self, O> {
        assert!(is_main_thread());
        todo!()
    }
}

impl<T> !Send for MainThreadOnly<T> {}
impl<T> !Sync for MainThreadOnly<T> {}

// Allows giving to another thread and using the thread-safe APIs there
impl<T> Deref for MainThreadOnly<T> { ... }

// Thread-safe APIs (if any)
impl NSApplication {
    // Note: Unsure if `+[NSApplication sharedApplication]` is even thread safe... This is just an example!
    pub fn shared() -> Id<NSApplication, Shared>;
}

// Main thread only APIs
impl MainThreadOnly<NSApplication> {
    pub fn set_main_menu(&self, menu: Id<NSMenu, Owned>) -> Id<MainThreadOnly<NSMenu>, Shared>;
}

// Safe because all non-thread-safe access is behind `MainThreadOnly<T>`
impl Send for NSApplication {}
impl Sync for NSApplication {}

Which has the problem that the impl MainThreadOnly<NSApplication> will only work in the current crate, so it would have to be redefined in downstream crates (alternative, set_main_menu takes this: &MainThreadOnly<NSApplication>, but that has terrible ergonomics!).

@madsmtm
Copy link
Owner Author

madsmtm commented Jun 19, 2022

Regarding MainThreadMarker, if the type is !Send + !Sync, be used only upon construction, and then any subsequent usage wouldn't need it:

impl !Send for NSView {}
impl !Sync for NSView {}

impl NSView {
    pub fn new(_: MainThreadMarker) -> Id<NSView, Owned> {
        unsafe { msg_send_id![class!(NSView), new] }
    }

    // Doesn't need `MainThreadMarker`
    pub fn has_marked_text(&self) -> bool {
        unsafe { msg_send_bool![self, hasMarkedText] }
    }
}

This is only really viable for classes whose entire API requires the main thread, but maybe that is a common case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant