Hi Olivier, Olivier Dion <[email protected]> skribis:
> From 2adaf872aeb3b84be9cf8af06f958ea4b731f66c Mon Sep 17 00:00:00 2001 > From: Olivier Dion <[email protected]> > Date: Tue, 17 Feb 2026 11:37:35 -0500 > Subject: [PATCH] Fix weak sets not shrinking after GC. > > The `scm_i_register_async_gc_callback' mechanism used finalizers to > trigger post-GC callbacks for vacuuming weak sets and tables. This > approach stopped working reliably, causing weak sets to grow > indefinitely even after GC. > > Replace the finalizer-based callback mechanism with direct integration > into `scm_after_gc_c_hook'. A new hook callback, > `scm_i_notify_finalizer_thread', is registered with `scm_after_gc_c_hook'. > When the hook runs after GC, this callback either notifies the finalizer > thread (if running) to invoke finalizers, or runs them synchronously and > spawns the finalizer thread if allowed. The vacuum functions for weak > sets and tables are called from `scm_run_finalizers'. > > When `(gc)' is called explicitly from Scheme, finalizers are run > immediately. An atomic flag prevents double-vacuuming when the GC hook > fires afterward. > > Fixes <https://issues.guix.gnu.org/40194>. > > * libguile/atomics-internal.h (scm_atomic_swap_pointer): New function. > * libguile/finalizers.c: Include weak-set.h and weak-table.h. > (queue_finalizer_async): Remove. > (async_gc_finalizer): Remove. > (scm_i_register_async_gc_callback): Remove. > (allow_finalizer_thread): New static variable. > (scm_i_finalizer_pre_fork): Use `allow_finalizer_thread' instead of > `GC_set_finalizer_notifier'. > (scm_i_notify_finalizer_thread): New function. > (scm_set_automatic_finalization_enabled): Use `scm_after_gc_c_hook' > instead of `GC_set_finalizer_notifier'. > (vacuum_off): New static variable. > (scm_run_finalizers): Call `scm_i_vacuum_weak_sets' and > `scm_i_vacuum_weak_tables' using atomic flag to prevent double-vacuuming. > (scm_i_run_finalizers_and_turnoff_vacuums): New function. > (scm_init_finalizers): Use `scm_after_gc_c_hook'. > (scm_init_finalizer_thread): Simplify to just set `allow_finalizer_thread'. > * libguile/finalizers.h (scm_i_register_async_gc_callback): Remove. > (scm_i_run_finalizers_and_turnoff_vacuums): New declaration. > * libguile/gc.c: Include finalizers.h. > (scm_gc): Call `scm_i_run_finalizers_and_turnoff_vacuums' instead of > `GC_invoke_finalizers'. > (scm_init_gc): Disable `GC_set_finalizer_notifier'. > * libguile/weak-set.c (scm_i_vacuum_weak_sets): Rename from > `vacuum_all_weak_sets' and make non-static. > (scm_init_weak_set): Remove `scm_i_register_async_gc_callback' call. > * libguile/weak-set.h (scm_i_vacuum_weak_sets): New declaration. > * libguile/weak-table.c (scm_i_vacuum_weak_tables): Rename from > `vacuum_all_weak_tables' and make non-static. > (scm_init_weak_table): Remove `scm_i_register_async_gc_callback' call. > * libguile/weak-table.h (scm_i_vacuum_weak_tables): New declaration. > > Signed-off-by: Olivier Dion <[email protected]> I totally lost track of the issue bug I’m glad you’re working on it. I only had a quick look but the fix and its explanation make sense to me. Do you have ideas though as to why and when the previous approach “stopped working reliably” as you wrote? Do you have tests showing that this patch does indeed fix the problem? This is a quite sensitive piece of code so I’m worried about possible issues we did not anticipate. :-) Thanks, Ludo’.
