Send and Sync fixes

block-sys/CHANGELOG.md

@@ -6,6 +6,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## Unreleased - YYYY-MM-DD
 
+### Fixed
+* **BREAKING**: `Class` is now `!UnwindSafe`.
 
 ## 0.0.1 - 2021-11-22
 

block-sys/src/lib.rs

@@ -34,7 +34,7 @@ pub struct Class {
     _priv: [u8; 0],
 
     /// See objc_sys::OpaqueData
-    _opaque: PhantomData<(UnsafeCell<*const ()>, PhantomPinned)>,
+    _opaque: PhantomData<(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)>,
 }
 
 /// Block descriptor flags.

objc-sys/CHANGELOG.md

@@ -19,6 +19,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   present on other platforms anyhow, so users will just get an error at
   link-time.
 
+### Fixed
+* **BREAKING**: Opaque types are now also `!UnwindSafe`.
+
 
 ## 0.1.0 - 2021-11-22
 

objc-sys/src/lib.rs

@@ -56,9 +56,11 @@ pub use types::*;
 pub use various::*;
 
 /// We don't know much about the actual structs, so better mark them `!Send`,
-/// `!Sync`, `!Unpin` and as mutable behind shared references. Downstream
-/// libraries can always manually opt in to these types afterwards. (It's
-/// also less of a breaking change on our part if we re-add these later).
+/// `!Sync`, `!UnwindSafe`, `!RefUnwindSafe`, `!Unpin` and as mutable behind
+/// shared references.
 ///
-/// TODO: Replace this with `extern type` to also mark it as unsized.
-type OpaqueData = PhantomData<(UnsafeCell<*const ()>, PhantomPinned)>;
+/// Downstream libraries can always manually opt in to these types afterwards.
+/// (It's also less of a breaking change on our part if we re-add these).
+///
+/// TODO: Replace this with `extern type` to also mark it as `!Sized`.
+type OpaqueData = PhantomData<(UnsafeCell<()>, *const UnsafeCell<()>, PhantomPinned)>;

objc2-foundation/CHANGELOG.md

@@ -25,6 +25,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 ### Fixed
 * Soundness issue with `NSValue`, `NSDictionary`, `NSArray` and
   `NSMutableArray` not being `#[repr(C)]`.
+* **BREAKING**: `NSObject` is no longer `Send` and `Sync` (because its
+  subclasses may not be).
 
 ## 0.2.0-alpha.2 - 2021-11-22
 

objc2-foundation/src/array.rs

@@ -174,6 +174,26 @@ object!(
     }
 );
 
+// SAFETY: Same as Id<T, O> (which is what NSArray effectively stores).
+//
+// The `PhantomData` can't get these impls to display in the docs.
+//
+// TODO: Properly verify this
+unsafe impl<T: Sync + Send> Sync for NSArray<T, Shared> {}
+unsafe impl<T: Sync + Send> Send for NSArray<T, Shared> {}
+unsafe impl<T: Sync> Sync for NSArray<T, Owned> {}
+unsafe impl<T: Send> Send for NSArray<T, Owned> {}
+
+/// ```compile_fail
+/// use objc2::rc::Shared;
+/// use objc2::runtime::Object;
+/// use objc2_foundation::NSArray;
+/// fn needs_send_sync<T: Send + Sync>() {}
+/// needs_send_sync::<NSArray<Object, Shared>>();
+/// ```
+#[cfg(doctest)]
+pub struct NSArrayWithObjectNotSendSync;
+
 unsafe impl<T: INSObject, O: Ownership> INSArray for NSArray<T, O> {
     /// The `NSArray` itself (length and number of items) is always immutable,
     /// but we would like to know when we're the only owner of the array, to
@@ -319,6 +339,14 @@ object!(
     }
 );
 
+// SAFETY: Same as NSArray.
+//
+// TODO: Properly verify this
+unsafe impl<T: Sync + Send> Sync for NSMutableArray<T, Shared> {}
+unsafe impl<T: Sync + Send> Send for NSMutableArray<T, Shared> {}
+unsafe impl<T: Sync> Sync for NSMutableArray<T, Owned> {}
+unsafe impl<T: Send> Send for NSMutableArray<T, Owned> {}
+
 unsafe impl<T: INSObject, O: Ownership> INSArray for NSMutableArray<T, O> {
     type Ownership = Owned;
     type Item = T;
@@ -540,4 +568,14 @@ mod tests {
             assert_eq!(strings[1].as_str(pool), "hello");
         });
     }
+
+    #[test]
+    fn test_send_sync() {
+        fn assert_send_sync<T: Send + Sync>() {}
+
+        assert_send_sync::<NSArray<NSString, Shared>>();
+        assert_send_sync::<NSMutableArray<NSString, Shared>>();
+        assert_send_sync::<Id<NSArray<NSString, Shared>, Shared>>();
+        assert_send_sync::<Id<NSMutableArray<NSString, Shared>, Owned>>();
+    }
 }

objc2-foundation/src/data.rs

@@ -94,6 +94,10 @@ pub unsafe trait INSData: INSObject {
 
 object!(unsafe pub struct NSData);
 
+// TODO: SAFETY
+unsafe impl Sync for NSData {}
+unsafe impl Send for NSData {}
+
 unsafe impl INSData for NSData {
     type Ownership = Shared;
 }
@@ -157,6 +161,10 @@ pub unsafe trait INSMutableData: INSData {
 
 object!(unsafe pub struct NSMutableData);
 
+// TODO: SAFETY
+unsafe impl Sync for NSMutableData {}
+unsafe impl Send for NSMutableData {}
+
 unsafe impl INSData for NSMutableData {
     type Ownership = Owned;
 }

objc2-foundation/src/dictionary.rs

@@ -145,6 +145,10 @@ object!(
     }
 );
 
+// TODO: SAFETY
+unsafe impl<K: Sync + Send, V: Sync> Sync for NSDictionary<K, V> {}
+unsafe impl<K: Sync + Send, V: Send> Send for NSDictionary<K, V> {}
+
 impl<K: INSObject, V: INSObject> NSDictionary<K, V> {
     unsafe_def_fn!(pub fn new -> Shared);
 }

objc2-foundation/src/object.rs

@@ -1,8 +1,9 @@
+use core::marker::PhantomData;
 use core::ptr::NonNull;
 
 use objc2::msg_send;
 use objc2::rc::{Id, Owned, Shared};
-use objc2::runtime::{Bool, Class};
+use objc2::runtime::{Bool, Class, Object};
 use objc2::Message;
 
 use super::NSString;
@@ -37,7 +38,22 @@ pub unsafe trait INSObject: Sized + Message {
     }
 }
 
-object!(unsafe pub struct NSObject);
+object!(unsafe pub struct NSObject<> {
+    p: PhantomData<Object>, // Temporary
+});
+
+/// ```compile_fail
+/// use objc2_foundation::NSObject;
+/// fn needs_sync<T: Sync>() {}
+/// needs_sync::<NSObject>();
+/// ```
+/// ```compile_fail
+/// use objc2_foundation::NSObject;
+/// fn needs_send<T: Send>() {}
+/// needs_send::<NSObject>();
+/// ```
+#[cfg(doctest)]
+pub struct NSObjectNotSendNorSync;
 
 impl NSObject {
     unsafe_def_fn!(pub fn new -> Owned);

objc2-foundation/src/string.rs

@@ -102,6 +102,10 @@ pub unsafe trait INSString: INSObject {
 
 object!(unsafe pub struct NSString);
 
+// TODO: SAFETY
+unsafe impl Sync for NSString {}
+unsafe impl Send for NSString {}
+
 impl NSString {
     unsafe_def_fn!(pub fn new -> Shared);
 }

objc2-foundation/src/value.rs

@@ -75,6 +75,10 @@ object!(
     }
 );
 
+// TODO: SAFETY
+unsafe impl<T: Sync> Sync for NSValue<T> {}
+unsafe impl<T: Send> Send for NSValue<T> {}
+
 unsafe impl<T: 'static + Copy + Encode> INSValue for NSValue<T> {
     type Value = T;
 }

objc2/CHANGELOG.md

@@ -21,6 +21,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ### Fixed
 * An issue with inlining various `rc` methods.
+* Most types (e.g. `Class` and `Method`) are now `Send`, `Sync`, `UnwindSafe`
+  and `RefUnwindSafe` again.
+  Notable exception is `Object`, because that depends on the specific
+  subclass.
 
 
 ## 0.3.0-alpha.4 - 2021-11-22
@@ -73,7 +77,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   `runtime` module.
 * **BREAKING**: Most types are now `!UnwindSafe`, to discourage trying to use
   them after an unwind. This restriction may be lifted in the future.
-* **BREAKING**: Most types are now `!Send` and `!Sync`. **TODO**: Reevaluate this!
+* **BREAKING**: Most types are now `!Send` and `!Sync`. This was an oversight
+  that is fixed in a later version.
 * A lot of smaller things.
 * **BREAKING**: Dynamic message sending with `Message::send_message` is moved
   to `MessageReceiver`.

objc2/src/declare.rs

@@ -120,6 +120,20 @@ pub struct ClassDecl {
     cls: *mut Class,
 }
 
+// SAFETY: The stuff that touch global state does so using locks internally.
+//
+// Modifying the class itself can only be done through `&mut`, so Sync is
+// safe (e.g. we can't accidentally call `add_ivar` at the same time from two
+// different threads).
+//
+// (Though actually, that would be safe since the entire runtime is locked
+// when doing so...).
+//
+// Finally, there are no requirements that the class must be registered on the
+// same thread that allocated it.
+unsafe impl Send for ClassDecl {}
+unsafe impl Sync for ClassDecl {}
+
 impl ClassDecl {
     fn with_superclass(name: &str, superclass: Option<&Class>) -> Option<ClassDecl> {
         let name = CString::new(name).unwrap();
@@ -296,6 +310,10 @@ pub struct ProtocolDecl {
     proto: *mut Protocol,
 }
 
+// SAFETY: Similar to ClassDecl
+unsafe impl Send for ProtocolDecl {}
+unsafe impl Sync for ProtocolDecl {}
+
 impl ProtocolDecl {
     /// Constructs a [`ProtocolDecl`] with the given name.
     ///

objc2/src/rc/autorelease.rs

@@ -21,12 +21,17 @@ pub struct AutoreleasePool {
     p: PhantomData<*mut UnsafeCell<c_void>>,
 }
 
-/// ```rust,compile_fail
+/// ```
 /// use objc2::rc::AutoreleasePool;
-/// fn needs_sync<T: Send>() {}
+/// fn needs_nothing<T>() {}
+/// needs_nothing::<AutoreleasePool>();
+/// ```
+/// ```compile_fail
+/// use objc2::rc::AutoreleasePool;
+/// fn needs_sync<T: Sync>() {}
 /// needs_sync::<AutoreleasePool>();
 /// ```
-/// ```rust,compile_fail
+/// ```compile_fail
 /// use objc2::rc::AutoreleasePool;
 /// fn needs_send<T: Send>() {}
 /// needs_send::<AutoreleasePool>();