bug#40194: Weak sets keep growing

On Wed, 18 Feb 2026, Ludovic Court=C3=A8s <[email protected]> wrote:
> Hello,
>
> Olivier Dion <[email protected]> skribis:
>
>> I guess this is internal to how libgc work.  From my understanding, and
>> debugging sessions, the finalized objects is marked when the callback is
>> called.  Thus, the sweep (after the finalizers?) will keep it.  I guess
>> this makes some sens since finalizers can re-introduce "dead" objects.
>> For obscure reasons, the objects is never finalized again, yet it
>> should.
>>
>> My theory is, after the call to the finalizer, a flag is unset from the
>> object (done by libgc), thus removing our finalizer that we've just
>> installed.  The object is then simply garbaged collect as usual but we
>> just don't see it.  Would be interesting to track that object with a
>> watchpoint in GDB to see that in action.
>
> OK.

So I tried a different approach where instead of using a single atomic
pointer, I use two of them and store only one in a root node.  Then,
when the dangling pointer is finalized, I swap it with the one in the
root node.  Kind of like a pivot point.  See diff at the end.

I do get two calls to the finalizer, from the two distinct pointers, but
no more.  So I really think that finalized pointers can simply never be
finalized after, even if they are resurrected by the finalizer itself.
Even thought, I look into the internal code of libgc, I don't see why
finalizer could not be called twice ..

Also just to show you the performance difference between the previous
patch and allocating a new atomic pointer in the finalizer (I bump the
loop from 10 to 50 just to put more stress):

# A: Baseline (no vacuum)
Benchmark 1: ./meta/guile ./reproducer.scm
  Time (mean =C2=B1 =CF=83):     215.9 ms =C2=B1   2.2 ms    [User: 512.3 m=
s, System: 60.0 ms]
  Range (min =E2=80=A6 max):   213.0 ms =E2=80=A6 221.4 ms    13 runs

# B: New atomic pointer everytime (single line trivial fix)
Benchmark 1: ./meta/guile ./reproducer.scm
  Time (mean =C2=B1 =CF=83):     265.2 ms =C2=B1  24.0 ms    [User: 571.7 m=
s, System: 66.5 ms]
  Range (min =E2=80=A6 max):   234.5 ms =E2=80=A6 301.9 ms    11 runs

# C: The proposed patch that re-work the finalizer module
Benchmark 1: ./meta/guile ./reproducer.scm
  Time (mean =C2=B1 =CF=83):     229.5 ms =C2=B1   4.7 ms    [User: 636.8 m=
s, System: 64.9 ms]
  Range (min =E2=80=A6 max):   224.0 ms =E2=80=A6 238.7 ms    12 runs


And a perf-diff(1) of A, B and C:

# Data files:
#  [0] perf.data (Baseline)
#  [1] perf.data.trivial
#  [2] perf.data.rework
#
# Baseline/0  Delta Abs/1  Delta Abs/2  Shared Object          Symbol
# ..........  ...........  ...........  .....................  ............=
.................................
#
      67.08%       -2.71%       +6.47%  libgc.so.1.5.4         [.] 0x000000=
0000008bf3
       1.86%       +1.22%       +0.72%  libc.so.6              [.] pthread_=
mutex_trylock@@GLIBC_2.34
                   +1.02%               [JIT] tid 17129        [.] 0x00007f=
24059c9085
       1.17%       +0.95%       -0.14%  libgc.so.1.5.4         [.] GC_base
       0.06%       +0.63%       -0.02%  libguile-3.0.so.1.7.1  [.] resize_s=
et.lto_priv.0
       0.75%       -0.60%       -0.43%  libgc.so.1.5.4         [.] GC_gener=
ic_malloc_many
       2.57%       -0.51%       -1.16%  libgc.so.1.5.4         [.] GC_is_ma=
rked
       0.88%       +0.50%       -0.23%  libguile-3.0.so.1.7.1  [.] give_to_=
poor.lto_priv.0

So more time in GC, yet better result?

[...]


pivot.diff:
--8<---------------cut here---------------start------------->8---
diff --git a/libguile/finalizers.c b/libguile/finalizers.c
index f6e67d5d7..8d4f3e412 100644
--- a/libguile/finalizers.c
+++ b/libguile/finalizers.c
@@ -358,12 +358,17 @@ scm_i_finalizer_pre_fork (void)
 static void
 async_gc_finalizer (void *ptr, void *data)
 {
-  void **obj =3D ptr;
-  void (*callback) (void) =3D obj[0];
+  struct async_finalizer *af;
=20
-  callback ();
+  af =3D *(void **)ptr;
=20
-  scm_i_set_finalizer (ptr, async_gc_finalizer, data);
+  printf("async_gc_finalizer\n");
+
+  scm_i_set_finalizer (ptr, async_gc_finalizer, NULL);
+  scm_i_set_finalizer (af->safe_pointer, async_gc_finalizer, NULL);
+
+  af->callback ();
+  af->safe_pointer =3D ptr;
 }
=20
 /* Arrange to call CALLBACK asynchronously after each GC.  The callback
@@ -388,13 +393,20 @@ async_gc_finalizer (void *ptr, void *data)
    expand the heap.  Therefore scm_i_register_async_gc_callback is
    inappropriate for using on unbounded numbers of callbacks.  */
 void
-scm_i_register_async_gc_callback (void (*callback) (void))
+scm_i_register_async_gc_callback (void (*callback) (void),
+                                  struct async_finalizer *af)
 {
-  void **obj =3D GC_MALLOC_ATOMIC (sizeof (void*));
+  void **a =3D GC_MALLOC_ATOMIC(sizeof (void*));
+  void **b =3D GC_MALLOC_ATOMIC(sizeof (void*));
=20
-  obj[0] =3D (void*)callback;
+  af->safe_pointer =3D a;
+  af->callback =3D callback;
=20
-  scm_i_set_finalizer (obj, async_gc_finalizer, NULL);
+  *a =3D af;
+  *b =3D af;
+
+  scm_i_set_finalizer (a, async_gc_finalizer, NULL);
+  scm_i_set_finalizer (b, async_gc_finalizer, NULL);
 }
=20
=20
diff --git a/libguile/finalizers.h b/libguile/finalizers.h
index a92a74be1..e3c300aae 100644
--- a/libguile/finalizers.h
+++ b/libguile/finalizers.h
@@ -28,6 +28,11 @@
=20
 typedef void (*scm_t_finalizer_proc) (void *obj, void *data);
=20
+struct async_finalizer {
+  void *safe_pointer;
+  void (*callback)(void);
+};
+
 SCM_INTERNAL void scm_i_set_finalizer (void *obj, scm_t_finalizer_proc,
                                        void *data);
 SCM_INTERNAL void scm_i_add_finalizer (void *obj, scm_t_finalizer_proc,
@@ -40,7 +45,8 @@ SCM_INTERNAL void scm_i_finalizer_pre_fork (void);
 /* CALLBACK will be called after each garbage collection.  It will be
    called from a finalizer, which may be from an async or from another
    thread. */
-SCM_INTERNAL void scm_i_register_async_gc_callback (void (*callback) (void=
));
+SCM_INTERNAL void scm_i_register_async_gc_callback (void (*callback) (void=
),
+                                                    struct async_finalizer=
 *af);
=20
 /* Return true if THREAD is the finalizer thread.  */
 SCM_INTERNAL int scm_i_is_finalizer_thread (struct scm_thread *thread);
diff --git a/libguile/weak-set.c b/libguile/weak-set.c
index 93c429346..e81b5fdf6 100644
--- a/libguile/weak-set.c
+++ b/libguile/weak-set.c
@@ -902,6 +902,6 @@ void
 scm_init_weak_set ()
 {
 #include "weak-set.x"
-
-  scm_i_register_async_gc_callback (vacuum_all_weak_sets);
+  static struct async_finalizer af;
+  scm_i_register_async_gc_callback (vacuum_all_weak_sets, &af);
 }
diff --git a/libguile/weak-table.c b/libguile/weak-table.c
index 1e4d8d302..81752a67f 100644
--- a/libguile/weak-table.c
+++ b/libguile/weak-table.c
@@ -841,5 +841,5 @@ scm_init_weak_table ()
 {
 #include "weak-table.x"
=20
-  scm_i_register_async_gc_callback (vacuum_all_weak_tables);
+  //  scm_i_register_async_gc_callback (vacuum_all_weak_tables);
 }
--8<---------------cut here---------------end--------------->8---

Thanks,
Olivier
--=20
Olivier Dion