-
Notifications
You must be signed in to change notification settings - Fork 77
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
Use objc2
and its framework crates
#148
Conversation
`objc2` is a replacement for `objc`/`objc_id` that contains a bunch of safety improvements, including `msg_send_id!` which automatically upholds memory management rules (`Id::from_ptr`/`Id::from_retained_ptr` is no longer necessary). Additionally, we use the framework crates `objc2-foundation` and `objc2-app-kit`, which provide for example the `NSPasteboard` type, which has the methods that arboard needs already defined, and with the correct types, ensuring that passing e.g. `Id<NSArray>` and thus accidentally giving away ownership over the array won't happen again. These crates are automatically generated, ensuring that if you need some obscure API in the future, it's very likely to be there already.
unsafe impl Send for Clipboard {} | ||
unsafe impl Sync for Clipboard {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are... Probably not entirely true, NSPasteboard
is likely only safe to use from the main thread, as that's where other code in an application would be using them from - at least, the thread-safety of NSPasteboard
isn't documented anywhere.
The impls are added to keep backwards compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW we have been using NSPasteboard
off the main thread (inside Tokio workers and std::thread::spawn
) at 1Password for several years now and haven't seen a single crash with a clipboard-related stacktrace. This of course isn't a guarantee but at minimum it hasn't been problematic.
My intuition says this would be somewhat thread-safe because its just an IPC/XPC wrapper to talk to the pboard
daemon, and XPC uses queues underneath.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this PR. Its great to see that objc2
is making progress from when I last looked at it. I have some small comments, but otherwise this looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you again for these changes!
objc2
is a replacement forobjc
/objc_id
that contains a bunch of safety improvements, includingmsg_send_id!
which automatically upholds memory management rules (Id::from_ptr
/Id::from_retained_ptr
is no longer necessary).Additionally, we use the framework crates
objc2-foundation
andobjc2-app-kit
, which provide for example theNSPasteboard
type, which has the methods that arboard needs already defined, and with the correct types, ensuring that passing e.g.Id<NSArray>
and thus accidentally giving away ownership over the array won't happen again.These crates are automatically generated, ensuring that if you need some obscure API in the future, it's very likely to be there already.
Feel free to ask if there's anything about the new code that's unclear!