Subject: bug#79023: 30.1.90; Suspicion of memory leak on internal_redisplay (MacOS)

From: Przemysław Alexander Kamiński<[email-masked]>
Date: Tue, 12 Aug 2025 08:24:49 +0200

On Tue, Aug 12, 2025 at 06:46:38 (+0200), Paul D. Nelson wrote:

> Przemysław Alexander Kamiński <[email-masked]> writes:
>
>> I'm attaching 6 patches (for easy review/scrutiny/integration).
>> Those patches that I marked as tested was used by me for >1 week.
>
> I tried applying the patches for testing, but there were some clashes,
> e.g., nullify_frame.patch and misc_releases.patch both the modify the
> end of dealloc in nsterm.m, but the modifications do not stack.

Yes, that's on purpose. I didn't want them to be considered together.
It's also easier for me to manage through Jujutsu front as merge commit
is regenerated after patch commit was changed.

I'm attaching combined patch for convenience. It already includes
ns_native_api_dict_0002.patch (0001 was crashing compilation
subprocesses).

Best,
Przemysław Alexander Kamiński
-- 

Pointers are real. They’re what the hardware understands. Somebody has
to deal with them. You can’t just place a LISP book on top of an x86
chip and hope that the hardware learns about lambda calculus by osmosis.

  -- James Mickens "The Night Watch"

diff --git a/src/macfont.m b/src/macfont.m
index 2a0b9aa255..cef4cf80ff 100644
--- a/src/macfont.m
+++ b/src/macfont.m
@@ -1152,10 +1152,11 @@
 	    CTFontDescriptorCopyAttribute (desc, kCTFontFamilyNameAttribute);
 	  CFRelease (desc);
 	}
-      macfont_set_family_cache (symbol, result);
+
       CFRelease (pat_desc);
     }
-
+  /* It's possible that we don't have have kCTFontFamilyNameAttribute for symbol, but still cache result */
+  macfont_set_family_cache (symbol, result);
   return result;
 }
 
diff --git a/src/nsimage.m b/src/nsimage.m
index 8f1653d085..d96aeab7b3 100644
--- a/src/nsimage.m
+++ b/src/nsimage.m
@@ -28,6 +28,7 @@
 /* This should be the first include, as it may set up #defines affecting
    interpretation of even the system includes.  */
 #include <config.h>
+#include <CoreFoundation/CoreFoundation.h>
 
 #include "lisp.h"
 #include "dispextern.h"
@@ -56,9 +57,57 @@
 
    ========================================================================== */
 
+static CFDictionaryRef image_api_dict;
+
+/*
+  ns_image_init initializes native support image type cache-dict for
+  ns_can_use_native_image_api. Needs to be initialized ad-hoc since it's
+  possible that non-GUI subprocesses will use it.
+
+  Without the cache-dict hundreds of thousands of small, persistent
+  NSString allocations are happening causing leak 40-60KiB leak per usage.
+*/
+
+static void ns_image_init (void)
+{
+  /* NSImage also supports a host of other types such as PDF and BMP,
+       but we don't yet support these in image.c.  */
+  NSArray *imageTypes = [NSImage imageTypes];
+  Lisp_Object keys[] = { Qnative_image, Qjpeg, Qpng, Qgif, Qtiff,
+#ifndef HAVE_RSVG
+			 Qsvg,
+#endif
+#ifndef HAVE_WEBP
+			 Qwebp,
+#endif
+			 Qheic };
+  bool values[] = {
+    YES,
+    [imageTypes containsObject:@"public.jpeg"],
+    [imageTypes containsObject:@"public.png"],
+    [imageTypes containsObject:@"com.compuserve.gif"],
+    [imageTypes containsObject:@"public.tiff"],
+#ifndef HAVE_RSVG
+    [imageTypes containsObject:@"public.svg-image"],
+#endif
+#ifndef HAVE_WEBP
+    [imageTypes containsObject:@"org.webmproject.webp"],
+#endif
+    [imageTypes containsObject:@"public.heic"],
+  };
+  image_api_dict
+    = CFDictionaryCreate (NULL, (void *) keys, (void *) values, 50,
+			  NULL, NULL);
+  image_api_dict = CFRetain (image_api_dict);
+}
+
 bool
 ns_can_use_native_image_api (Lisp_Object type)
 {
+#ifdef NS_IMPL_COCOA
+  if(!image_api_dict) ns_image_init();
+  return (bool) (CFDictionaryGetValue(image_api_dict, type) || NO);
+#endif
   NSString *imageType = @"unknown";
   NSArray *types;
 
@@ -67,32 +116,6 @@
   if (EQ (type, Qnative_image))
     return YES;
 
-#ifdef NS_IMPL_COCOA
-  /* Work out the UTI of the image type.  */
-  if (EQ (type, Qjpeg))
-    imageType = @"public.jpeg";
-  else if (EQ (type, Qpng))
-    imageType = @"public.png";
-  else if (EQ (type, Qgif))
-    imageType = @"com.compuserve.gif";
-  else if (EQ (type, Qtiff))
-    imageType = @"public.tiff";
-#ifndef HAVE_RSVG
-  else if (EQ (type, Qsvg))
-    imageType = @"public.svg-image";
-#endif
-#ifndef HAVE_WEBP
-  else if (EQ (type, Qwebp))
-    imageType = @"org.webmproject.webp";
-#endif
-  else if (EQ (type, Qheic))
-    imageType = @"public.heic";
-
-  /* NSImage also supports a host of other types such as PDF and BMP,
-     but we don't yet support these in image.c.  */
-
-  types = [NSImage imageTypes];
-#else
   /* Work out the image type.  */
   if (EQ (type, Qjpeg))
     imageType = @"jpeg";
@@ -104,7 +127,6 @@
     imageType = @"tiff";
 
   types = [NSImage imageFileTypes];
-#endif
 
   /* Check if the type is supported on this system.  */
   if ([types indexOfObject:imageType] != NSNotFound)
diff --git a/src/nsterm.m b/src/nsterm.m
index b006b4d5dd..5b4fe9317c 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -78,6 +78,25 @@
 static EmacsMenu *mainMenu;
 #endif
 
+static volatile int render_blocked = 0;
+INLINE void
+block_render (void)
+{
+  render_blocked++;
+}
+INLINE bool
+render_blocked_p (void)
+{
+  return render_blocked > 0;
+}
+INLINE void
+unblock_render (void)
+{
+  render_blocked--;
+}
+
+
+
 /* The last known monitor attributes list.  */
 static Lisp_Object last_known_monitors;
 
@@ -1637,6 +1656,7 @@
 
   [[view window] close];
   [view removeFromSuperview];
+  [view release];
 
   xfree (f->output_data.ns);
   f->output_data.ns = NULL;
@@ -4956,8 +4976,9 @@
              to ourself, otherwise [NXApp run] will never exit.  */
           send_appdefined = YES;
           ns_send_appdefined (-1);
-
+          block_render();
           [NSApp run];
+          unblock_render();
         }
 
       nevents = n_emacs_events_pending;
@@ -5099,7 +5120,9 @@
   block_input ();
   ns_init_events (&event);
 
+  block_render();
   [NSApp run];
+  unblock_render();
 
   ns_finish_events ();
   if (nr > 0 && readfds)
@@ -6839,6 +6862,15 @@
 
   if (fs_state == FULLSCREEN_BOTH)
     [nonfs_window release];
+
+  /* Release layer and menu */
+  EmacsLayer *layer = (EmacsLayer *)[self layer];
+  [layer release];
+  [[self menu] release];
+
+   /* Don't reuse inner - cleanup isn't robust enough */
+   emacsframe = nil;
+
   [super dealloc];
 }
 
@@ -9033,7 +9065,6 @@
 
 #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
   CGContextRef context = [(EmacsLayer *)[self layer] getContext];
-  CGContextFlush (context);
 
   double scale = [[self window] backingScaleFactor];
   int bpp = CGBitmapContextGetBitsPerPixel (context) / 8;
@@ -9084,7 +9115,7 @@
    appears to be safe to call redisplay here.  */
 - (void)layoutSublayersOfLayer:(CALayer *)layer
 {
-  if (!redisplaying_p && FRAME_GARBAGED_P (emacsframe))
+  if (!redisplaying_p && FRAME_GARBAGED_P (emacsframe) && !render_blocked_p())
     {
       /* If there is IO going on when redisplay is run here Emacs
          crashes.  I think it's because this code will always be run
@@ -9588,8 +9619,9 @@
   NSTRACE ("[EmacsWindow dealloc]");
 
   /* We need to release the toolbar ourselves.  */
+  [[self toolbar] release];
   [self setToolbar: nil];
-  [[self toolbar] release];
+
 
   /* Also the last button press event .  */
   if (last_drag_event)
@@ -10994,7 +11026,6 @@
   if (!context)
     return;
 
-  CGContextFlush (context);
   CGContextRelease (context);
   context = NULL;