Re: GC + Java finalization

[Maxime Devos <maximedevos@telenet.be>]

Jonas Hahnfeld schreef op do 15-07-2021 om 20:44 [+0200]:
> diff --git a/libguile/random.c b/libguile/random.c
> index 63da7f5d6..ac400a9fd 100644
> --- a/libguile/random.c
> +++ b/libguile/random.c
> @@ -324,9 +324,7 @@ scm_c_random_bignum (scm_t_rstate *state, SCM m)
>    /* we know the result will be this big */
>    mpz_realloc2 (SCM_I_BIG_MPZ (result), m_bits);
>  
> -  random_chunks =
> -    (uint32_t *) scm_gc_calloc (num_chunks * sizeof (uint32_t),
> -                                     "random bignum chunks");
> +  random_chunks = (uint32_t *) scm_calloc (num_chunks * sizeof
> (uint32_t));
>  
>    do
>      {
> @@ -363,9 +361,7 @@ scm_c_random_bignum (scm_t_rstate *state, SCM m)
>        /* if result >= m, regenerate it (it is important to
> regenerate
>   all bits in order not to get a distorted distribution) */
>      } while (mpz_cmp (SCM_I_BIG_MPZ (result), SCM_I_BIG_MPZ (m)) >=
> 0);
> -  scm_gc_free (random_chunks,
> -               num_chunks * sizeof (uint32_t),
> -               "random bignum chunks");
> +  free (random_chunks);
>    return scm_i_normbig (result);
>  }

As I understand it, the idea of this change is to avoid boehmgc having
to track the memory random_chunks (‘memory registration’). However,
in-between the scm_calloc and free, mpz_import is called. Looking at
libguile/mini-gmp.c, this causes gmp_allocate_func to be called. This
variable is set by mp_set_memory_functions, which is called by
scm_init_number with the allocation function
custom_gmp_malloc/custom_gmp_realloc, which uses
scm_gc_malloc_pointerless/scm_gc_realloc.

Note that that these functions signal an error in case of out-of-memory
(at least, that's what 6.19.2 Memory Blocks states). As such, in the
following situation a memory leak can happen after the proposed patch:

(catch 'out-of-memory
  (lambda ()
    ;; have enough memory to allocate random_chunks,
    ;; but not enough for mpz_import
    (random some-large-number))
  (lambda _
    ;; random_chunks won't ever be freed! 
    (pk 'oops-not-enough-memory _)))

At least Artanis tries to behave somewhat nicely in case of OOM
(https://git.savannah.gnu.org/cgit/artanis.git/tree/artanis/server/ragnarok.scm#n659),
so I'd prefer to keep using scm_gc_calloc (+ scm_gc_free) instead of
scm_calloc + free.

Greetings,
Maxime.