Subject: bug#78917: feature/igc [PATCH] Avoid chaining finalizers together

From: Pip Cet <[email-masked]>
Date: Sat, 28 Jun 2025 21:37:11 +0000

"Helmut Eller" <[email-masked]> writes:

> On Sat, Jun 28 2025, Pip Cet wrote:
>
>> "Helmut Eller" <[email-masked]> writes:
>>
>>> Lisp_Finalizers are currently chained together in a doubly linked list.
>>> This prevents them from being collected.  I propose that we simply don't
>>> use this list with MPS.
>>
>> See https://lists.gnu.org/archive/html/emacs-devel/2025-05/msg00468.html
>
> :-)
>
>> and bug#77338.
>
> It seems that the problem with finalizers as value in a weak hashtable
> could be fixed by calling mark_and_sweep_weak_table_contents before
> queue_doomed_finalizers as in the patch below.

No, that's not the right fix: a finalizer function can still refer to
weak hash tables (the finalizer itself is not a weak object), so we
can't sweep the weak hash tables until all finalizers have been marked.

This is solvable, but it requires more effort than exchanging those two
calls.

However, I'm not sure we should fix the traditional GC code to be better
than what MPS can do :-)

>> The way finalizers are currently run on the master branch when they
>> appear in weak tables, even if there is no chance that the entry that
>> references them is ever collected, makes them pretty much unusable on
>> that branch, in my book.  Making them usable with --with-mps=yes isn't
>> much of a priority for me, TBH, as code using them would break when we
>> compile without MPS support.
>
> Putting finalizers in a weak hashtable sounds exotic.  I suppose there
> should be tests for this.

Putting font objects in a weak hash table doesn't seem exotic to me.

>> (I considered using finalizers for font finalization; it works, but
>> would break if a strong reference to the font is mistaken for a weak one
>> and fails to keep alive the font).
>
> There is some "hardcoded" finalization code in cleanup_vector for
> PVEC_FONT.  Maybe you could use that instead of Lisp_Finalizers.

The point was to get rid of that (unstable and problematic) code :-)

Pip