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

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

Closed
dvc94ch opened this issue Nov 16, 2022 · 6 comments
Closed

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

dvc94ch opened this issue Nov 16, 2022 · 6 comments
Labels
question Further information is requested

Comments

@dvc94ch
Copy link

dvc94ch commented Nov 16, 2022

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 madsmtm added the question Further information is requested label Nov 16, 2022
@madsmtm
Copy link
Owner

madsmtm commented Nov 16, 2022

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 #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
Copy link
Author

dvc94ch commented Nov 16, 2022

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
Copy link
Author

dvc94ch commented Nov 16, 2022

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
Copy link
Owner

madsmtm commented Nov 16, 2022

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
Copy link
Owner

madsmtm commented Nov 16, 2022

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
Copy link
Author

dvc94ch commented Nov 16, 2022

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.

@dvc94ch dvc94ch closed this as completed Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants