gpui: Fix some memory leaks on macOS platform (#44639)

Marco Mihai Condrache and Anthony Eid created

While profiling with instruments, I discovered that some of the strings
allocated on the mac platform are never released, and the profiler marks
them as leaks

<img width="1570" height="219" alt="image"
src="https://github.com/user-attachments/assets/174e9293-5139-46ae-8757-c8989f3fc598"
/>


Release Notes:

- N/A

---------

Signed-off-by: Marco Mihai Condrache <52580954+marcocondrache@users.noreply.github.com>
Co-authored-by: Anthony Eid <anthony@zed.dev>

Change summary

clippy.toml                                       |  1 
crates/fs/src/fs.rs                               |  2 +
crates/gpui/src/platform/mac.rs                   |  2 +
crates/gpui/src/platform/mac/attributed_string.rs | 34 +++++++++++------
crates/gpui/src/platform/mac/display.rs           |  7 ++-
crates/gpui/src/platform/mac/platform.rs          | 12 ++---
crates/gpui/src/platform/mac/screen_capture.rs    |  5 +-
crates/gpui/src/platform/mac/window.rs            | 22 +++++------
8 files changed, 49 insertions(+), 36 deletions(-)

Detailed changes

clippy.toml 🔗

@@ -14,6 +14,7 @@ disallowed-methods = [
     { path = "std::process::Command::stderr", reason = "`smol::process::Command::from()` does not preserve stdio configuration", replacement = "smol::process::Command::stderr" },
     { path = "serde_json::from_reader", reason = "Parsing from a buffer is much slower than first reading the buffer into a Vec/String, see https://github.com/serde-rs/json/issues/160#issuecomment-253446892. Use `serde_json::from_slice` instead." },
     { path = "serde_json_lenient::from_reader", reason = "Parsing from a buffer is much slower than first reading the buffer into a Vec/String, see https://github.com/serde-rs/json/issues/160#issuecomment-253446892, Use `serde_json_lenient::from_slice` instead." },
+    { path = "cocoa::foundation::NSString::alloc", reason = "NSString must be autoreleased to avoid memory leaks. Use `ns_string()` helper instead." },
 ]
 disallowed-types = [
     # { path = "std::collections::HashMap", replacement = "collections::HashMap" },

crates/fs/src/fs.rs 🔗

@@ -641,6 +641,8 @@ impl Fs for RealFs {
         use objc::{class, msg_send, sel, sel_impl};
 
         unsafe {
+            /// Allow NSString::alloc use here because it sets autorelease
+            #[allow(clippy::disallowed_methods)]
             unsafe fn ns_string(string: &str) -> id {
                 unsafe { NSString::alloc(nil).init_str(string).autorelease() }
             }

crates/gpui/src/platform/mac.rs 🔗

@@ -135,6 +135,8 @@ unsafe impl objc::Encode for NSRange {
     }
 }
 
+/// Allow NSString::alloc use here because it sets autorelease
+#[allow(clippy::disallowed_methods)]
 unsafe fn ns_string(string: &str) -> id {
     unsafe { NSString::alloc(nil).init_str(string).autorelease() }
 }

crates/gpui/src/platform/mac/attributed_string.rs 🔗

@@ -50,10 +50,12 @@ impl NSMutableAttributedString for id {}
 
 #[cfg(test)]
 mod tests {
+    use crate::platform::mac::ns_string;
+
     use super::*;
     use cocoa::appkit::NSImage;
     use cocoa::base::nil;
-    use cocoa::foundation::NSString;
+    use cocoa::foundation::NSAutoreleasePool;
     #[test]
     #[ignore] // This was SIGSEGV-ing on CI but not locally; need to investigate https://github.com/zed-industries/zed/actions/runs/10362363230/job/28684225486?pr=15782#step:4:1348
     fn test_nsattributed_string() {
@@ -68,26 +70,34 @@ mod tests {
         impl NSTextAttachment for id {}
 
         unsafe {
-            let image: id = msg_send![class!(NSImage), alloc];
-            image.initWithContentsOfFile_(NSString::alloc(nil).init_str("test.jpeg"));
+            let image: id = {
+                let img: id = msg_send![class!(NSImage), alloc];
+                let img: id = msg_send![img, initWithContentsOfFile: ns_string("test.jpeg")];
+                let img: id = msg_send![img, autorelease];
+                img
+            };
             let _size = image.size();
 
-            let string = NSString::alloc(nil).init_str("Test String");
-            let attr_string = NSMutableAttributedString::alloc(nil).init_attributed_string(string);
-            let hello_string = NSString::alloc(nil).init_str("Hello World");
-            let hello_attr_string =
-                NSAttributedString::alloc(nil).init_attributed_string(hello_string);
+            let string = ns_string("Test String");
+            let attr_string = NSMutableAttributedString::alloc(nil)
+                .init_attributed_string(string)
+                .autorelease();
+            let hello_string = ns_string("Hello World");
+            let hello_attr_string = NSAttributedString::alloc(nil)
+                .init_attributed_string(hello_string)
+                .autorelease();
             attr_string.appendAttributedString_(hello_attr_string);
 
-            let attachment = NSTextAttachment::alloc(nil);
+            let attachment: id = msg_send![NSTextAttachment::alloc(nil), autorelease];
             let _: () = msg_send![attachment, setImage: image];
             let image_attr_string =
                 msg_send![class!(NSAttributedString), attributedStringWithAttachment: attachment];
             attr_string.appendAttributedString_(image_attr_string);
 
-            let another_string = NSString::alloc(nil).init_str("Another String");
-            let another_attr_string =
-                NSAttributedString::alloc(nil).init_attributed_string(another_string);
+            let another_string = ns_string("Another String");
+            let another_attr_string = NSAttributedString::alloc(nil)
+                .init_attributed_string(another_string)
+                .autorelease();
             attr_string.appendAttributedString_(another_attr_string);
 
             let _len: cocoa::foundation::NSUInteger = msg_send![attr_string, length];

crates/gpui/src/platform/mac/display.rs 🔗

@@ -1,9 +1,10 @@
+use super::ns_string;
 use crate::{Bounds, DisplayId, Pixels, PlatformDisplay, point, px, size};
 use anyhow::Result;
 use cocoa::{
     appkit::NSScreen,
     base::{id, nil},
-    foundation::{NSArray, NSDictionary, NSString},
+    foundation::{NSArray, NSDictionary},
 };
 use core_foundation::uuid::{CFUUIDGetUUIDBytes, CFUUIDRef};
 use core_graphics::display::{CGDirectDisplayID, CGDisplayBounds, CGGetActiveDisplayList};
@@ -35,7 +36,7 @@ impl MacDisplay {
             let screens = NSScreen::screens(nil);
             let screen = cocoa::foundation::NSArray::objectAtIndex(screens, 0);
             let device_description = NSScreen::deviceDescription(screen);
-            let screen_number_key: id = NSString::alloc(nil).init_str("NSScreenNumber");
+            let screen_number_key: id = ns_string("NSScreenNumber");
             let screen_number = device_description.objectForKey_(screen_number_key);
             let screen_number: CGDirectDisplayID = msg_send![screen_number, unsignedIntegerValue];
             Self(screen_number)
@@ -150,7 +151,7 @@ impl MacDisplay {
     unsafe fn get_nsscreen(&self) -> id {
         let screens = unsafe { NSScreen::screens(nil) };
         let count = unsafe { NSArray::count(screens) };
-        let screen_number_key: id = unsafe { NSString::alloc(nil).init_str("NSScreenNumber") };
+        let screen_number_key: id = unsafe { ns_string("NSScreenNumber") };
 
         for i in 0..count {
             let screen = unsafe { NSArray::objectAtIndex(screens, i) };

crates/gpui/src/platform/mac/platform.rs 🔗

@@ -2,7 +2,7 @@ use super::{
     BoolExt, MacKeyboardLayout, MacKeyboardMapper,
     attributed_string::{NSAttributedString, NSMutableAttributedString},
     events::key_to_native,
-    renderer,
+    ns_string, renderer,
 };
 use crate::{
     Action, AnyWindowHandle, BackgroundExecutor, ClipboardEntry, ClipboardItem, ClipboardString,
@@ -1061,13 +1061,15 @@ impl Platform for MacPlatform {
                 let attributed_string = {
                     let mut buf = NSMutableAttributedString::alloc(nil)
                         // TODO can we skip this? Or at least part of it?
-                        .init_attributed_string(NSString::alloc(nil).init_str(""));
+                        .init_attributed_string(ns_string(""))
+                        .autorelease();
 
                     for entry in item.entries {
                         if let ClipboardEntry::String(ClipboardString { text, metadata: _ }) = entry
                         {
                             let to_append = NSAttributedString::alloc(nil)
-                                .init_attributed_string(NSString::alloc(nil).init_str(&text));
+                                .init_attributed_string(ns_string(&text))
+                                .autorelease();
 
                             buf.appendAttributedString_(to_append);
                         }
@@ -1543,10 +1545,6 @@ extern "C" fn handle_dock_menu(this: &mut Object, _: Sel, _: id) -> id {
     }
 }
 
-unsafe fn ns_string(string: &str) -> id {
-    unsafe { NSString::alloc(nil).init_str(string).autorelease() }
-}
-
 unsafe fn ns_url_to_path(url: id) -> Result<PathBuf> {
     let path: *mut c_char = msg_send![url, fileSystemRepresentation];
     anyhow::ensure!(!path.is_null(), "url is not a file path: {}", unsafe {

crates/gpui/src/platform/mac/screen_capture.rs 🔗

@@ -1,3 +1,4 @@
+use super::ns_string;
 use crate::{
     DevicePixels, ForegroundExecutor, SharedString, SourceMetadata,
     platform::{ScreenCaptureFrame, ScreenCaptureSource, ScreenCaptureStream},
@@ -7,7 +8,7 @@ use anyhow::{Result, anyhow};
 use block::ConcreteBlock;
 use cocoa::{
     base::{YES, id, nil},
-    foundation::{NSArray, NSString},
+    foundation::NSArray,
 };
 use collections::HashMap;
 use core_foundation::base::TCFType;
@@ -195,7 +196,7 @@ unsafe fn screen_id_to_human_label() -> HashMap<CGDirectDisplayID, ScreenMeta> {
     let screens: id = msg_send![class!(NSScreen), screens];
     let count: usize = msg_send![screens, count];
     let mut map = HashMap::default();
-    let screen_number_key = unsafe { NSString::alloc(nil).init_str("NSScreenNumber") };
+    let screen_number_key = unsafe { ns_string("NSScreenNumber") };
     for i in 0..count {
         let screen: id = msg_send![screens, objectAtIndex: i];
         let device_desc: id = msg_send![screen, deviceDescription];

crates/gpui/src/platform/mac/window.rs 🔗

@@ -785,7 +785,7 @@ impl MacWindow {
                     native_window.setAcceptsMouseMovedEvents_(YES);
 
                     if let Some(tabbing_identifier) = tabbing_identifier {
-                        let tabbing_id = NSString::alloc(nil).init_str(tabbing_identifier.as_str());
+                        let tabbing_id = ns_string(tabbing_identifier.as_str());
                         let _: () = msg_send![native_window, setTabbingIdentifier: tabbing_id];
                     } else {
                         let _: () = msg_send![native_window, setTabbingIdentifier:nil];
@@ -908,8 +908,8 @@ impl MacWindow {
     pub fn get_user_tabbing_preference() -> Option<UserTabbingPreference> {
         unsafe {
             let defaults: id = NSUserDefaults::standardUserDefaults();
-            let domain = NSString::alloc(nil).init_str("NSGlobalDomain");
-            let key = NSString::alloc(nil).init_str("AppleWindowTabbingMode");
+            let domain = ns_string("NSGlobalDomain");
+            let key = ns_string("AppleWindowTabbingMode");
 
             let dict: id = msg_send![defaults, persistentDomainForName: domain];
             let value: id = if !dict.is_null() {
@@ -1037,7 +1037,7 @@ impl PlatformWindow for MacWindow {
             }
 
             if let Some(tabbing_identifier) = tabbing_identifier {
-                let tabbing_id = NSString::alloc(nil).init_str(tabbing_identifier.as_str());
+                let tabbing_id = ns_string(tabbing_identifier.as_str());
                 let _: () = msg_send![native_window, setTabbingIdentifier: tabbing_id];
             } else {
                 let _: () = msg_send![native_window, setTabbingIdentifier:nil];
@@ -1063,10 +1063,8 @@ impl PlatformWindow for MacWindow {
                 return None;
             }
             let device_description: id = msg_send![screen, deviceDescription];
-            let screen_number: id = NSDictionary::valueForKey_(
-                device_description,
-                NSString::alloc(nil).init_str("NSScreenNumber"),
-            );
+            let screen_number: id =
+                NSDictionary::valueForKey_(device_description, ns_string("NSScreenNumber"));
 
             let screen_number: u32 = msg_send![screen_number, unsignedIntValue];
 
@@ -1509,8 +1507,8 @@ impl PlatformWindow for MacWindow {
             .spawn(async move {
                 unsafe {
                     let defaults: id = NSUserDefaults::standardUserDefaults();
-                    let domain = NSString::alloc(nil).init_str("NSGlobalDomain");
-                    let key = NSString::alloc(nil).init_str("AppleActionOnDoubleClick");
+                    let domain = ns_string("NSGlobalDomain");
+                    let key = ns_string("AppleActionOnDoubleClick");
 
                     let dict: id = msg_send![defaults, persistentDomainForName: domain];
                     let action: id = if !dict.is_null() {
@@ -2512,7 +2510,7 @@ where
 unsafe fn display_id_for_screen(screen: id) -> CGDirectDisplayID {
     unsafe {
         let device_description = NSScreen::deviceDescription(screen);
-        let screen_number_key: id = NSString::alloc(nil).init_str("NSScreenNumber");
+        let screen_number_key: id = ns_string("NSScreenNumber");
         let screen_number = device_description.objectForKey_(screen_number_key);
         let screen_number: NSUInteger = msg_send![screen_number, unsignedIntegerValue];
         screen_number as CGDirectDisplayID
@@ -2558,7 +2556,7 @@ unsafe fn remove_layer_background(layer: id) {
             // `description` reflects its name and some parameters. Currently `NSVisualEffectView`
             // uses a `CAFilter` named "colorSaturate". If one day they switch to `CIFilter`, the
             // `description` will still contain "Saturat" ("... inputSaturation = ...").
-            let test_string: id = NSString::alloc(nil).init_str("Saturat").autorelease();
+            let test_string: id = ns_string("Saturat");
             let count = NSArray::count(filters);
             for i in 0..count {
                 let description: id = msg_send![filters.objectAtIndex(i), description];