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

From: Przemysław Alexander Kamiński<[email-masked]>
Date: Sun, 07 Sep 2025 17:03:38 +0200

Hi Alan!

On Sun, Aug 31, 2025 at 20:46:03 (+0100), Alan Third wrote:
>> * render_block_small.patch (significant impact, tested):
>> 
>> This is similar patch to previous one, but it's minimal. It's crucial
>> to avoid "blink" redraws.
>> Still breaks smooth resizing, but frame-resize-pixelwise set to true
>> makes it unnoticable.

I start to hope this patch won't have to be implemented. If there are no
allocations (or allocations are reused) then it doesnt matter how fast
they are processed. 

>> * dont_flush_cg.patch (medium impact, tested):
>> 
>> Based on documentation of CGContextFlush it should seldom be used, as
>> it's much better to give OS possibility to decide when to flush. It
>> prevents non-needed renders (and allocations etc.)
>
> See notes below.
>
>> * misc_releases.patch (small-mid impact, tested):
>> 
>> This one is based on Rudolf Adamkovič patch with some minor extra
>> releases and adjustments
>> (like releasing an object before setting it to nil ;))
>
> EmacsLayer is only used on Cocoa, and only on certain versions, so
> anything that references it needs to be limited. Look at the #if
> before the definition of EmacsLayer.

Will look into this, but I started wondering about breaking of Cocoa
code outside. Going through nsterm.m is quite confusing.

>> * ns_native_api_dict.patch (small-mid impact, needs testing):
>> ...
> LGTM except for the same problem as above. You'll need to look at the
> #if or #ifdefs around the code and ensure the new code is correctly
> excluded when building on GNUstep.
>
> I think you also removed a #if but left the associated #else and
> #endif, although I could be wrong about that.

Same.

>> * nullify_frame.patch (small impact, tested):
>> 
>> releases reference to a frame, as I've found traces in which resources
>> were left behind, small leak.
>
> This one is funny as emacsframe is a pointer to a "struct frame" and
> we're not using garbage collection in NS so I don't see how simply
> having a reference in a released object can prevent that struct from
> being properly freed. Am I missing something?
>
>> +  /* Don't reuse inner - cleanup is robust enough */
>> +  emacsframe = nil;
>> +
> And I don't understand the comment.

I patched this comment. It should be "cleanup isn't robust enough".
Releasing emacsframe reference seems to release these -  otherwise there
are still objects that are referencing it.

>> diff --git a/src/nsterm.m b/src/nsterm.m
>> index b006b4d5dd..057d876a35 100644
>> --- a/src/nsterm.m
>> +++ b/src/nsterm.m
>> @@ -9033,7 +9033,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;
>
> Immediately after this flush we access the pixels directly in the
> buffer, so we *require* that all drawing actions in the context have
> been completed, otherwise we risk doing actions out of order.
> Therefore this flush is required.

For context - I'm running this pretty much for couple weeks, and I
haven't observed any issues. The flush is going to be happening every
frame anyways, with it the flush count happens thousands of times.

But, truth be told, I haven't ran graphic heavy workflows (like images,
PDFs, etc.) so it might require more testing in that particular area.

Also I really hope that this could be prevented at all. The problem
isn't memory usage per se, but how allocations are happening.  If
allocations can be put in arenas/zones than none of it actually matters.

> Aside from that I think there was maybe some non-GNU-standard style C
> in the nsimage patch, but I'm not entirely clear how it should look
> myself.

Thanks for bringing this to my attention. I'll look into this, but I'm
at some... other level right now ;) (isolated builds and quick clean
rebuilds, as this kicked me in the back couple times already).

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"