Add UnwindSafe and RefUnwindSafe impls for Foundation types

madsmtm · madsmtm · commit b76694f3a3ee · 2022-07-22T04:22:27.000+02:00
diff --git a/objc2-foundation/CHANGELOG.md b/objc2-foundation/CHANGELOG.md
@@ -6,6 +6,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## Unreleased - YYYY-MM-DD
 
+### Added
+* Implement `UnwindSafe` and `RefUnwindSafe` for all objects.
+
 ### Removed
 * `NSObject::hash_code`, `NSObject::is_equal` and `NSObject::description` in
   favour of just having the trait implementations `Hash`, `PartiqalEq` and
diff --git a/objc2-foundation/src/array.rs b/objc2-foundation/src/array.rs
@@ -1,6 +1,7 @@
 use alloc::vec::Vec;
 use core::marker::PhantomData;
 use core::ops::{Index, Range};
+use core::panic::{RefUnwindSafe, UnwindSafe};
 
 use objc2::rc::{DefaultId, Id, Owned, Ownership, Shared, SliceId};
 use objc2::runtime::{Class, Object};
@@ -28,6 +29,7 @@ __inner_extern_class! {
     #[derive(Debug, PartialEq, Eq, Hash)]
     unsafe pub struct NSArray<T, O: Ownership>: NSObject {
         item: PhantomData<Id<T, O>>,
+        notunwindsafe: PhantomData<&'static mut ()>,
     }
 }
 
@@ -39,6 +41,11 @@ 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> {}
 
+// Also same as Id<T, O>
+impl<T: RefUnwindSafe, O: Ownership> RefUnwindSafe for NSArray<T, O> {}
+impl<T: RefUnwindSafe> UnwindSafe for NSArray<T, Shared> {}
+impl<T: UnwindSafe> UnwindSafe for NSArray<T, Owned> {}
+
 pub(crate) unsafe fn from_refs<T: Message + ?Sized>(cls: &Class, refs: &[&T]) -> *mut Object {
     let obj: *mut Object = unsafe { msg_send![cls, alloc] };
     unsafe {
diff --git a/objc2-foundation/src/attributed_string.rs b/objc2-foundation/src/attributed_string.rs
@@ -1,3 +1,5 @@
+use core::panic::{RefUnwindSafe, UnwindSafe};
+
 use objc2::rc::{DefaultId, Id, Shared};
 use objc2::runtime::Object;
 use objc2::{msg_send, msg_send_id};
@@ -29,6 +31,10 @@ extern_class! {
 unsafe impl Sync for NSAttributedString {}
 unsafe impl Send for NSAttributedString {}
 
+// Same reasoning as `NSString`.
+impl UnwindSafe for NSAttributedString {}
+impl RefUnwindSafe for NSAttributedString {}
+
 /// Attributes that you can apply to text in an attributed string.
 pub type NSAttributedStringKey = NSString;
 
diff --git a/objc2-foundation/src/data.rs b/objc2-foundation/src/data.rs
@@ -2,6 +2,7 @@
 use alloc::vec::Vec;
 use core::ffi::c_void;
 use core::ops::Index;
+use core::panic::{RefUnwindSafe, UnwindSafe};
 use core::slice::{self, SliceIndex};
 
 use objc2::rc::{DefaultId, Id, Shared};
@@ -25,6 +26,9 @@ extern_class! {
 unsafe impl Sync for NSData {}
 unsafe impl Send for NSData {}
 
+impl UnwindSafe for NSData {}
+impl RefUnwindSafe for NSData {}
+
 impl NSData {
     pub fn new() -> Id<Self, Shared> {
         unsafe { msg_send_id![Self::class(), new].unwrap() }
diff --git a/objc2-foundation/src/dictionary.rs b/objc2-foundation/src/dictionary.rs
@@ -2,6 +2,7 @@ use alloc::vec::Vec;
 use core::cmp::min;
 use core::marker::PhantomData;
 use core::ops::Index;
+use core::panic::{RefUnwindSafe, UnwindSafe};
 use core::ptr;
 
 use objc2::rc::{DefaultId, Id, Owned, Shared, SliceId};
@@ -22,6 +23,10 @@ __inner_extern_class! {
 unsafe impl<K: Sync + Send, V: Sync> Sync for NSDictionary<K, V> {}
 unsafe impl<K: Sync + Send, V: Send> Send for NSDictionary<K, V> {}
 
+// Approximately same as `NSArray<T, Shared>`
+impl<K: UnwindSafe, V: UnwindSafe> UnwindSafe for NSDictionary<K, V> {}
+impl<K: RefUnwindSafe, V: RefUnwindSafe> RefUnwindSafe for NSDictionary<K, V> {}
+
 impl<K: Message, V: Message> NSDictionary<K, V> {
     pub fn new() -> Id<Self, Shared> {
         unsafe { msg_send_id![Self::class(), new].unwrap() }
diff --git a/objc2-foundation/src/lib.rs b/objc2-foundation/src/lib.rs
@@ -118,45 +118,65 @@ mod zone;
 
 #[cfg(test)]
 mod tests {
+    use core::panic::{RefUnwindSafe, UnwindSafe};
     use objc2::rc::{Id, Owned, Shared};
 
     use super::*;
 
-    fn assert_send_sync<T: Send + Sync>() {}
+    // We expect most Foundation types to be UnwindSafe and RefUnwindSafe,
+    // since they follow Rust's usual mutability rules (&T = immutable).
+    //
+    // A _lot_ of Objective-C code out there would be subtly broken if e.g.
+    // `NSString` wasn't exception safe!
+    // As an example: -[NSArray objectAtIndex:] can throw, but it is still
+    // perfectly valid to access the array after that!
+    //
+    // Note that e.g. `&mut NSMutableString` is still not exception safe, but
+    // that is the entire idea of `UnwindSafe` (that if the object could have
+    // been mutated, it is not exception safe).
+    //
+    // Also note that this is still just a speed bump, not actually part of
+    // any unsafe contract; we really protect against it if something is not
+    // exception safe, since `UnwindSafe` is a safe trait.
+    fn assert_unwindsafe<T: UnwindSafe + RefUnwindSafe>() {}
+
+    fn assert_auto_traits<T: Send + Sync + UnwindSafe + RefUnwindSafe>() {
+        assert_unwindsafe::<T>();
+    }
 
     #[test]
-    fn send_sync() {
-        assert_send_sync::<NSArray<NSString, Shared>>();
-        assert_send_sync::<NSArray<NSString, Owned>>();
-        assert_send_sync::<Id<NSArray<NSString, Shared>, Shared>>();
-        assert_send_sync::<Id<NSArray<NSString, Owned>, Shared>>();
-        assert_send_sync::<Id<NSArray<NSString, Shared>, Owned>>();
-        assert_send_sync::<Id<NSArray<NSString, Owned>, Owned>>();
-
-        assert_send_sync::<NSAttributedString>();
-        assert_send_sync::<NSComparisonResult>();
-        assert_send_sync::<NSData>();
-        assert_send_sync::<NSDictionary<NSString, Shared>>();
-        // TODO: Figure out if safe?
-        // assert_send_sync::<NSEnumerator<NSString>>();
-        // assert_send_sync::<NSFastEnumerator<NSArray<NSString, Shared>>>();
-        assert_send_sync::<NSException>();
-        assert_send_sync::<CGFloat>();
-        assert_send_sync::<NSPoint>();
-        assert_send_sync::<NSRect>();
-        assert_send_sync::<NSSize>();
-        assert_send_sync::<NSMutableArray<NSString, Shared>>();
-        assert_send_sync::<NSMutableAttributedString>();
-        assert_send_sync::<NSMutableData>();
-        assert_send_sync::<NSMutableString>();
-        // assert_send_sync::<NSObject>(); // Intentional
-        assert_send_sync::<NSProcessInfo>();
-        assert_send_sync::<NSRange>();
-        assert_send_sync::<NSString>();
-        // assert_send_sync::<MainThreadMarker>(); // Intentional
-        assert_send_sync::<NSThread>();
-        assert_send_sync::<NSUUID>();
-        assert_send_sync::<NSValue<i32>>();
-        assert_send_sync::<NSZone>();
+    fn send_sync_unwindsafe() {
+        assert_auto_traits::<NSArray<NSString, Shared>>();
+        assert_auto_traits::<NSArray<NSString, Owned>>();
+        assert_auto_traits::<Id<NSArray<NSString, Shared>, Shared>>();
+        assert_auto_traits::<Id<NSArray<NSString, Owned>, Shared>>();
+        assert_auto_traits::<Id<NSArray<NSString, Shared>, Owned>>();
+        assert_auto_traits::<Id<NSArray<NSString, Owned>, Owned>>();
+
+        assert_auto_traits::<NSAttributedString>();
+        assert_auto_traits::<NSComparisonResult>();
+        assert_auto_traits::<NSData>();
+        assert_auto_traits::<NSDictionary<NSString, Shared>>();
+        // TODO: Figure out if Send + Sync is safe?
+        // assert_auto_traits::<NSEnumerator<NSString>>();
+        // assert_auto_traits::<NSFastEnumerator<NSArray<NSString, Shared>>>();
+        assert_auto_traits::<NSException>();
+        assert_auto_traits::<CGFloat>();
+        assert_auto_traits::<NSPoint>();
+        assert_auto_traits::<NSRect>();
+        assert_auto_traits::<NSSize>();
+        assert_auto_traits::<NSMutableArray<NSString, Shared>>();
+        assert_auto_traits::<NSMutableAttributedString>();
+        assert_auto_traits::<NSMutableData>();
+        assert_auto_traits::<NSMutableString>();
+        // assert_auto_traits::<NSObject>(); // Intentional
+        assert_auto_traits::<NSProcessInfo>();
+        assert_auto_traits::<NSRange>();
+        assert_auto_traits::<NSString>();
+        assert_unwindsafe::<MainThreadMarker>(); // Intentional
+        assert_auto_traits::<NSThread>();
+        assert_auto_traits::<NSUUID>();
+        assert_auto_traits::<NSValue<i32>>();
+        assert_unwindsafe::<NSZone>(); // Intentional
     }
 }
diff --git a/objc2-foundation/src/process_info.rs b/objc2-foundation/src/process_info.rs
@@ -1,3 +1,5 @@
+use core::panic::{RefUnwindSafe, UnwindSafe};
+
 use objc2::msg_send_id;
 use objc2::rc::{Id, Shared};
 
@@ -18,6 +20,9 @@ extern_class! {
 unsafe impl Send for NSProcessInfo {}
 unsafe impl Sync for NSProcessInfo {}
 
+impl UnwindSafe for NSProcessInfo {}
+impl RefUnwindSafe for NSProcessInfo {}
+
 impl NSProcessInfo {
     pub fn process_info() -> Id<NSProcessInfo, Shared> {
         // currentThread is @property(strong), what does that mean?
diff --git a/objc2-foundation/src/string.rs b/objc2-foundation/src/string.rs
@@ -46,6 +46,8 @@ extern_class! {
 unsafe impl Sync for NSString {}
 unsafe impl Send for NSString {}
 
+// Even if an exception occurs inside a string method, the state of the string
+// (should) still be perfectly safe to access.
 impl UnwindSafe for NSString {}
 impl RefUnwindSafe for NSString {}
 
diff --git a/objc2-foundation/src/thread.rs b/objc2-foundation/src/thread.rs
@@ -1,4 +1,5 @@
 use core::marker::PhantomData;
+use core::panic::{RefUnwindSafe, UnwindSafe};
 
 use objc2::rc::{Id, Shared};
 use objc2::{msg_send, msg_send_bool, msg_send_id};
@@ -16,6 +17,9 @@ extern_class! {
 unsafe impl Send for NSThread {}
 unsafe impl Sync for NSThread {}
 
+impl UnwindSafe for NSThread {}
+impl RefUnwindSafe for NSThread {}
+
 impl NSThread {
     /// Returns the [`NSThread`] object representing the current thread.
     pub fn current() -> Id<Self, Shared> {
diff --git a/objc2-foundation/src/uuid.rs b/objc2-foundation/src/uuid.rs
@@ -1,3 +1,5 @@
+use core::panic::{RefUnwindSafe, UnwindSafe};
+
 use objc2::rc::{Id, Shared};
 use objc2::{msg_send, msg_send_id, Encode, Encoding, RefEncode};
 
@@ -29,6 +31,9 @@ unsafe impl RefEncode for UuidBytes {
 unsafe impl Sync for NSUUID {}
 unsafe impl Send for NSUUID {}
 
+impl UnwindSafe for NSUUID {}
+impl RefUnwindSafe for NSUUID {}
+
 impl NSUUID {
     // TODO: `nil` method?
 
diff --git a/objc2-foundation/src/value.rs b/objc2-foundation/src/value.rs
@@ -3,6 +3,7 @@ use core::cmp::Ordering;
 use core::ffi::c_void;
 use core::marker::PhantomData;
 use core::mem::MaybeUninit;
+use core::panic::{RefUnwindSafe, UnwindSafe};
 use core::ptr::NonNull;
 use core::{fmt, str};
 use std::ffi::{CStr, CString};
@@ -28,6 +29,9 @@ __inner_extern_class! {
 unsafe impl<T: Sync> Sync for NSValue<T> {}
 unsafe impl<T: Send> Send for NSValue<T> {}
 
+impl<T: UnwindSafe> UnwindSafe for NSValue<T> {}
+impl<T: RefUnwindSafe> RefUnwindSafe for NSValue<T> {}
+
 impl<T: 'static + Copy + Encode> NSValue<T> {
     // Default / empty new is not provided because `-init` returns `nil` on
     // Apple and GNUStep throws an exception on all other messages to this
diff --git a/objc2-foundation/src/zone.rs b/objc2-foundation/src/zone.rs
@@ -1,6 +1,8 @@
+use core::panic::{RefUnwindSafe, UnwindSafe};
+
 #[cfg(feature = "gnustep-1-7")]
 use objc2::Encode;
-use objc2::{Encoding, RefEncode};
+use objc2::{ffi, Encoding, RefEncode};
 
 /// A type used to identify and manage memory zones.
 ///
@@ -9,11 +11,19 @@ use objc2::{Encoding, RefEncode};
 /// `allocWithZone:`.
 ///
 /// See [Apple's documentation](https://developer.apple.com/documentation/foundation/nszone?language=objc).
-#[derive(Debug)]
 pub struct NSZone {
-    _inner: [u8; 0],
+    // Use `objc_object` to mark the types as !Send, !Sync and UnsafeCell.
+    //
+    // This works since `objc_object` is a ZST
+    _inner: ffi::objc_object,
 }
 
+// Note: We don't know anything about the internals of `NSZone`, so best not
+// to make it `Send` and `Sync` for now.
+
+impl UnwindSafe for NSZone {}
+impl RefUnwindSafe for NSZone {}
+
 unsafe impl RefEncode for NSZone {
     #[cfg(feature = "apple")]
     const ENCODING_REF: Encoding<'static> = Encoding::Pointer(&Encoding::Struct("_NSZone", &[]));
