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.