madsmtm / objc2

Bindings to Apple's frameworks in Rust
https://docs.rs/objc2/
MIT License
290 stars 35 forks source link

All types including Object and NSObject are !Send and !Sync #290

Closed dvc94ch closed 1 year ago

dvc94ch commented 1 year ago

Why? is it not possible to write multithreaded anything in objc?

Seems like NSNumber and NSString are Send + Sync. But if the super class is not wouldn't that mean the inherited class isn't either?

madsmtm commented 1 year ago

A direct instance of NSObject, that is, a plain object created with NSObject::new(), is Send + Sync.

But we don't implement Send + Sync for NSObject, since we want to allow subclasses to "act" as-if they're NSObject; e.g. if I have an NSLock, I want to be able to pass that to functions taking NSObject; and if NSObject was Sync, that would allow us to pass it to another thread, which would potentially be unsound.

I hope that made sense? You can find more details in https://github.com/madsmtm/objc2/issues/58, if you're interested.

In general, multithreading in Objective-C is very difficult to do soundly, especially when using classes from AppKit, which assumes they're on the main thread. See also:

dvc94ch commented 1 year ago

My use case is constructing a UiView not on the main thread and then sending it to the main thread. It works with objc, but when I tried migrating to objc2 I didn't find a type I could use. Any suggestion what to replace objc_id::ShareId<objc::runtime::Object> with that is at least Send? Some context is provided below:

diff --git a/packages/desktop/src/desktop_context.rs b/packages/desktop/src/desktop_context.rs
index bf3a253c..4bf6c889 100644
--- a/packages/desktop/src/desktop_context.rs
+++ b/packages/desktop/src/desktop_context.rs
@@ -3,6 +3,8 @@ use dioxus_core::ScopeState;
 use wry::application::event_loop::ControlFlow;
 use wry::application::event_loop::EventLoopProxy;
 use wry::application::window::Fullscreen as WryFullscreen;
+#[cfg(target_os = "ios")]
+use wry::application::platform::ios::WindowExtIOS;

 use UserWindowEvent::*;

@@ -134,6 +136,12 @@ impl DesktopContext {
     pub fn eval(&self, script: impl std::string::ToString) {
         let _ = self.proxy.send_event(Eval(script.to_string()));
     }
+
+    /// Push view
+    #[cfg(target_os = "ios")]
+    pub fn push_view(&self, view: objc_id::ShareId<objc::runtime::Object>) {
+        let _ = self.proxy.send_event(PushView(view));
+    }
 }

 #[derive(Debug)]
@@ -164,6 +172,9 @@ pub enum UserWindowEvent {
     DevTool,

     Eval(String),
+
+    #[cfg(target_os = "ios")]
+    PushView(objc_id::ShareId<objc::runtime::Object>),
 }

 pub(super) fn handler(
@@ -231,6 +242,21 @@ pub(super) fn handler(
                 log::warn!("Eval script error: {e}");
             }
         }
+
+        #[cfg(target_os = "ios")]
+        PushView(view) => unsafe {
+            use objc::*;
+            use objc::runtime::Object;
+            assert!(is_main_thread());
+            let ui_view = window.ui_view() as *mut Object;
+            let ui_view_frame: *mut Object = msg_send![ui_view, frame];
+            let _: () = msg_send![view, setFrame:ui_view_frame];
+            let _: () = msg_send![view, setAutoresizingMask: 31];
+
+            //let _: () = msg_send![ui_view, addSubview: view];
+            let ui_view_controller = window.ui_view_controller() as *mut Object;
+            let _: () = msg_send![ui_view_controller, setView: view];
+        }
     }
 }
dvc94ch commented 1 year ago

But we don't implement Send + Sync for NSObject, since we want to allow subclasses to "act" as-if they're NSObject; e.g. if I have an NSLock, I want to be able to pass that to functions taking NSObject; and if NSObject was Sync, that would allow us to pass it to another thread, which would potentially be unsound.

but if you cast an NSLock to NSObject and send it to another thread, and then exclusively call NSObject methods on it, that seems sound to me. The problem you're referring to is casting it back to an NSLock after it was sent and using non send methods?

madsmtm commented 1 year ago

constructing a UiView not on the main thread and then sending it to the main thread

Just for good measure, I went and looked up the safety of this; in UIView - Threading considerations it says

Manipulations to your app’s user interface must occur on the main thread. Thus, you should always call the methods of the UIView class from code running in the main thread of your app. The only time this may not be strictly necessary is when creating the view object itself, but all other manipulations should occur on the main thread.

So I would say that having UIView and its superclasses be !Send + !Sync is generally a good call.

Any suggestion what to replace objc_id::ShareId<objc::runtime::Object> with that is at least Send?

I would probably send something else across the thread, like the data used to construct the view. Or maybe I'd just hack it with something like the following, since this is quite the special case:

struct SafeToSendToMainThread<T>(T);
unsafe impl<T> Send for SafeToSendToMainThread<T> {}
impl<T> SafeToSendToMainThread<T> {
    unsafe fn new(inner: T) -> Self {
        Self(inner)
    }
    fn get(self) -> T {
        assert!(is_main_thread());
        self.0
    }
}

// Usage

fn push_view(&self, view: ...) {
    // SAFETY: TODO, quoting the docs I linked above
    let _ = self.proxy.send_event(PushView(SafeToSendToMainThread::new(view)));
}

PushView(view) => unsafe {
    let view = view.get();
    // ...
}
madsmtm commented 1 year ago

The problem you're referring to is casting it back to an NSLock after it was sent and using non send methods?

I think something like that could very likely happen in Objective-C code (maybe not with NSLock, but with other classes).

But even assuming we don't allow that, NSObject could for sure not be Send, since that would allow calling release on it (via. rc::Id), which is not safe to do on another thread if the object is e.g. an NSAutoreleasePool.

And for example there's still other methods that are valid to call on NSObject like isEqual and description, which are allowed to be accessing internals of the class in a non-synchronized way (at least, I haven't seen the opposite claim anywhere, so we have to assume that's the case).

I agree that in most cases, you'd be fine, but when we're talking soundness, "most cases" is not good enough, it has to be all cases ;)

dvc94ch commented 1 year ago

ok, thanks for your suggestion. the reason for this contortion is I need some ios functionality (camera preview) mixed into a webview app. on android the qrcode scanning part is an activity in kotlin that returns the result.