From 921698e520515b617d7455c0c50696dcb021c6e7 Mon Sep 17 00:00:00 2001 From: ComplexSpaces Date: Fri, 28 Jul 2023 22:29:26 -0500 Subject: [PATCH] Remove leaks from macOS clipboard getters This additioanlly refactors `image()` to be simpler by taking advantage of the fact that we only want one image. Additionally, this codepath avoids the system-level caching of image data that can help reduce peak usage. We have applied an autorelease pool to the Obj-C code as well, as there appear to be a lot of autoreleased values used by AppKit.framework Finally, it applies a similar refactoring to `text()` for simplicity and removal of memory leaks due to incorrect object retaining assumptions. Co-authored-by: taojiacheng --- Cargo.toml | 4 +- src/platform/osx.rs | 123 +++++++++++++++++++++----------------------- 2 files changed, 61 insertions(+), 66 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f9f5c01..b3bd0f6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,7 +11,7 @@ edition = "2018" [features] default = ["image-data"] -image-data = ["core-graphics", "image", "winapi/minwindef", "winapi/wingdi", "winapi/winnt"] +image-data = ["core-graphics", "once_cell", "image", "winapi/minwindef", "winapi/wingdi", "winapi/winnt"] wayland-data-control = ["wl-clipboard-rs"] [dependencies] @@ -34,7 +34,7 @@ log = "0.4" objc = "0.2" objc_id = "0.1" objc-foundation = "0.1" -once_cell = "1" +once_cell = { version = "1", optional = true } core-graphics = { version = "0.22", optional = true } image = { version = "0.24", optional = true, default-features = false, features = ["tiff"] } diff --git a/src/platform/osx.rs b/src/platform/osx.rs index 7096536..3e74db6 100644 --- a/src/platform/osx.rs +++ b/src/platform/osx.rs @@ -20,22 +20,25 @@ use core_graphics::{ }; use objc::{ msg_send, + rc::autoreleasepool, runtime::{Class, Object}, sel, sel_impl, }; -use objc_foundation::{INSArray, INSObject, INSString, NSArray, NSDictionary, NSObject, NSString}; +use objc_foundation::{INSArray, INSFastEnumeration, INSString, NSArray, NSObject, NSString}; use objc_id::{Id, Owned}; +#[cfg(feature = "image-data")] use once_cell::sync::Lazy; -use std::borrow::Cow; +use std::{borrow::Cow, ptr::NonNull}; // Required to bring NSPasteboard into the path of the class-resolver #[link(name = "AppKit", kind = "framework")] extern "C" { static NSPasteboardTypeHTML: *const Object; static NSPasteboardTypeString: *const Object; + #[cfg(feature = "image-data")] + static NSPasteboardTypeTIFF: *const Object; } -static NSSTRING_CLASS: Lazy<&Class> = Lazy::new(|| Class::get("NSString").unwrap()); #[cfg(feature = "image-data")] static NSIMAGE_CLASS: Lazy<&Class> = Lazy::new(|| Class::get("NSImage").unwrap()); @@ -181,73 +184,72 @@ impl<'clipboard> Get<'clipboard> { } pub(crate) fn text(self) -> Result { - let string_class = object_class(&NSSTRING_CLASS); - let classes: Id> = NSArray::from_vec(vec![string_class]); - let options: Id> = NSDictionary::new(); - - let string_array: Id> = unsafe { - let obj: *mut NSArray = - msg_send![self.pasteboard, readObjectsForClasses:&*classes options:&*options]; - - if obj.is_null() { - return Err(Error::ContentNotAvailable); - } else { - Id::from_ptr(obj) + // XXX: There does not appear to be an alternative for obtaining text without the need for + // autorelease behavior. + autoreleasepool(|| { + // XXX: We explicitly use `pasteboardItems` and not `stringForType` since the latter will concat + // multiple strings, if present, into one and return it instead of reading just the first which is `arboard`'s + // historical behavior. + let contents: Option>> = + unsafe { msg_send![self.pasteboard, pasteboardItems] }; + + let contents = contents.map(|c| unsafe { c.as_ref() }).ok_or_else(|| { + Error::Unknown { description: String::from("NSPasteboard#pasteboardItems errored") } + })?; + + for item in contents.enumerator() { + let maybe_str: Option> = + unsafe { msg_send![item, stringForType:NSPasteboardTypeString] }; + + match maybe_str { + Some(string) => { + let string: Id = unsafe { Id::from_ptr(string.as_ptr()) }; + return Ok(string.as_str().to_owned()); + } + None => continue, + } } - }; - string_array - .first_object() - .map(|obj| obj.as_str().to_owned()) - .ok_or(Error::ContentNotAvailable) + Err(Error::ContentNotAvailable) + }) } #[cfg(feature = "image-data")] pub(crate) fn image(self) -> Result, Error> { + use objc_foundation::NSData; use std::io::Cursor; - let image_class: Id = object_class(&NSIMAGE_CLASS); - let classes = vec![image_class]; - let classes: Id> = NSArray::from_vec(classes); - let options: Id> = NSDictionary::new(); - - let contents: Id> = unsafe { - let obj: *mut NSArray = - msg_send![self.pasteboard, readObjectsForClasses:&*classes options:&*options]; + // XXX: There does not appear to be an alternative for obtaining images without the need for + // autorelease behavior. + let image = autoreleasepool(|| { + let obj: Option> = + unsafe { msg_send![self.pasteboard, dataForType: NSPasteboardTypeTIFF] }; - if obj.is_null() { - return Err(Error::ContentNotAvailable); + let image_data: Id = if let Some(obj) = obj { + unsafe { Id::from_ptr(obj.as_ptr()) } } else { - Id::from_ptr(obj) - } - }; + return Err(Error::ContentNotAvailable); + }; - let obj = match contents.first_object() { - Some(obj) if obj.is_kind_of(&NSIMAGE_CLASS) => obj, - Some(_) | None => return Err(Error::ContentNotAvailable), - }; + let data = unsafe { + let len: usize = msg_send![&*image_data, length]; + let bytes: *const u8 = msg_send![&*image_data, bytes]; - let tiff: &NSArray = unsafe { msg_send![obj, TIFFRepresentation] }; - let data = unsafe { - let len: usize = msg_send![tiff, length]; - let bytes: *const u8 = msg_send![tiff, bytes]; + Cursor::new(std::slice::from_raw_parts(bytes, len)) + }; - Cursor::new(std::slice::from_raw_parts(bytes, len)) - }; - let reader = image::io::Reader::with_format(data, image::ImageFormat::Tiff); - match reader.decode() { - Ok(img) => { - let rgba = img.into_rgba8(); - let (width, height) = rgba.dimensions(); - - Ok(ImageData { - width: width as usize, - height: height as usize, - bytes: rgba.into_raw().into(), - }) - } - Err(_) => Err(Error::ConversionFailure), - } + let reader = image::io::Reader::with_format(data, image::ImageFormat::Tiff); + reader.decode().map_err(|_| Error::ConversionFailure) + })?; + + let rgba = image.into_rgba8(); + let (width, height) = rgba.dimensions(); + + Ok(ImageData { + width: width as usize, + height: height as usize, + bytes: rgba.into_raw().into(), + }) } } @@ -345,10 +347,3 @@ impl<'clipboard> Clear<'clipboard> { Ok(()) } } - -/// Convenience function to get an Objective-C object from a -/// specific class. -fn object_class(class: &'static Class) -> Id { - // SAFETY: `Class` is a valid object and `Id` will not mutate it - unsafe { Id::from_ptr(class as *const Class as *mut NSObject) } -}