Am Samstag, dem 03.07.2021 um 14:05 +0200 schrieb Jonas Hahnfeld via Developers list for Guile, the GNU extensibility library: > Hi Guile devs, > > I'm hacking on GNU LilyPond and recently wondered if Guile could run > without Java finalization that prevents collection of chains of > unreachable objects. I found that the functionality is only needed once > the first guardian is created, so it's possible to delay enabling the > mode until then. This required some fixes to free functions that > assumed dependent objects to be freed only later (see first two > patches). > The third patch delays ensuring Java finalization to scm_make_guardian, > but doesn't disable it explicitly (it's on by default in bdwgc). This > could now be done right after GC_INIT(), but it's not clear (at least > to me) whether client applications actually rely it, so I think it's > better if that's not done in Guile itself. > > Please consider applying, the fixes potentially also to stable-2.2.
I didn't receive other comments than those by Maxime, so here is an updated version of the first patch. Jonas
From f89a8271c3cbb775a83db3e59fa66ea82f6239a1 Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld <hah...@hahnjo.de> Date: Fri, 2 Jul 2021 23:52:13 +0200 Subject: [PATCH 1/3] Fix test-smob-mark * test-suite/standalone/test-smob-mark.c (init_smob_type): Correct size of smob type. (free_x): Clear smob data instead of local variable. (test_scm_smob_mark): Put smobs in array to ensure marking. --- test-suite/standalone/test-smob-mark.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test-suite/standalone/test-smob-mark.c b/test-suite/standalone/test-smob-mark.c index d0bb5336a..586a2b081 100644 --- a/test-suite/standalone/test-smob-mark.c +++ b/test-suite/standalone/test-smob-mark.c @@ -79,7 +79,7 @@ free_x (SCM x) x_t *c_x; c_x = (x_t *) SCM_SMOB_DATA (x); scm_gc_free (c_x, sizeof (x_t), "x"); - c_x = NULL; + SCM_SET_SMOB_DATA (x, NULL); return 0; } @@ -100,22 +100,25 @@ print_x (SCM x, SCM port, scm_print_state * pstate SCM_UNUSED) static void test_scm_smob_mark () { + SCM *smobs = scm_gc_malloc (sizeof(SCM) * SMOBS_COUNT, "smobs"); + int i; mark_call_count = 0; for (i = 0; i < SMOBS_COUNT; i++) - make_x (); + smobs[i] = make_x (); scm_gc (); if (mark_call_count < SMOBS_COUNT) { fprintf (stderr, "FAIL: SMOB mark function called for each SMOB\n"); exit (EXIT_FAILURE); } + scm_remember_upto_here_1 (smobs); } static void init_smob_type () { - x_tag = scm_make_smob_type ("x", sizeof (x_t)); + x_tag = scm_make_smob_type ("x", sizeof (x_t *)); scm_set_smob_free (x_tag, free_x); scm_set_smob_print (x_tag, print_x); scm_set_smob_mark (x_tag, mark_x); -- 2.32.0
From 33af6a98c6801e7b4880d1d3f78f7e2097c2174e Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld <hah...@hahnjo.de> Date: Fri, 2 Jul 2021 23:03:17 +0200 Subject: [PATCH 2/3] Avoid matching calls of scm_gc_free There is no point in registering memory with the garbage collector if it doesn't need to do its job. In fact, calling scm_gc_free in a free function is wrong because it relies on Java finalization. * libguile/random.c (scm_c_random_bignum): Replace matching calls of scm_gc_calloc and scm_gc_free. * libguile/regex-posix.c (scm_make_regexp, regex_free): Avoid call of scm_gc_free in free function. * test-suite/standalone/test-smob-mark.c (make_x, free_x): Avoid call of scm_gc_free in free function. --- THANKS | 1 + libguile/random.c | 8 ++------ libguile/regex-posix.c | 6 +++--- test-suite/standalone/test-smob-mark.c | 4 ++-- 4 files changed, 8 insertions(+), 11 deletions(-) diff --git a/THANKS b/THANKS index aa4877e95..786b65d1a 100644 --- a/THANKS +++ b/THANKS @@ -13,6 +13,7 @@ Contributors since the last release: Volker Grabsch Julian Graham Michael Gran + Jonas Hahnfeld Daniel Hartwig No Itisnt Neil Jerram 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); } diff --git a/libguile/regex-posix.c b/libguile/regex-posix.c index a08da02db..36bb639e0 100644 --- a/libguile/regex-posix.c +++ b/libguile/regex-posix.c @@ -67,7 +67,7 @@ static size_t regex_free (SCM obj) { regfree (SCM_RGX (obj)); - scm_gc_free (SCM_RGX (obj), sizeof(regex_t), "regex"); + free (SCM_RGX (obj)); return 0; } @@ -164,7 +164,7 @@ SCM_DEFINE (scm_make_regexp, "make-regexp", 1, 0, 1, flag = SCM_CDR (flag); } - rx = scm_gc_malloc_pointerless (sizeof (regex_t), "regex"); + rx = scm_malloc (sizeof (regex_t)); c_pat = scm_to_locale_string (pat); status = regcomp (rx, c_pat, /* Make sure they're not passing REG_NOSUB; @@ -174,7 +174,7 @@ SCM_DEFINE (scm_make_regexp, "make-regexp", 1, 0, 1, if (status != 0) { SCM errmsg = scm_regexp_error_msg (status, rx); - scm_gc_free (rx, sizeof(regex_t), "regex"); + free (rx); scm_error_scm (scm_regexp_error_key, scm_from_utf8_string (FUNC_NAME), errmsg, diff --git a/test-suite/standalone/test-smob-mark.c b/test-suite/standalone/test-smob-mark.c index 586a2b081..edbb51f89 100644 --- a/test-suite/standalone/test-smob-mark.c +++ b/test-suite/standalone/test-smob-mark.c @@ -56,7 +56,7 @@ make_x () x_t *c_x; i++; - c_x = (x_t *) scm_gc_malloc (sizeof (x_t), "x"); + c_x = (x_t *) scm_malloc (sizeof (x_t)); c_x->scm_value = scm_from_int (i); c_x->c_value = i; SCM_NEWSMOB (s_x, x_tag, c_x); @@ -78,7 +78,7 @@ free_x (SCM x) { x_t *c_x; c_x = (x_t *) SCM_SMOB_DATA (x); - scm_gc_free (c_x, sizeof (x_t), "x"); + free (c_x); SCM_SET_SMOB_DATA (x, NULL); return 0; } -- 2.32.0
From 6bbb76692a26090918dfed42d67c6ba42e54e034 Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld <hah...@hahnjo.de> Date: Fri, 2 Jul 2021 23:16:32 +0200 Subject: [PATCH 3/3] Delay Java finalization for guardians It is only needed once the first guardian is created. * libguile/guardians.c (scm_make_guardian): Move call to enable Java finalization from scm_init_guardians. --- libguile/guardians.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/libguile/guardians.c b/libguile/guardians.c index fa8c8b8f7..357c8dbd1 100644 --- a/libguile/guardians.c +++ b/libguile/guardians.c @@ -306,6 +306,8 @@ guardian_apply (SCM guardian, SCM obj, SCM throw_p) return scm_i_get_one_zombie (guardian); } +static scm_i_pthread_mutex_t gc_java_mutex; + SCM_DEFINE (scm_make_guardian, "make-guardian", 0, 0, 0, (), "Create a new guardian. A guardian protects a set of objects from\n" @@ -349,6 +351,11 @@ SCM_DEFINE (scm_make_guardian, "make-guardian", 0, 0, 0, "value, it is eligible to be returned from a guardian.\n") #define FUNC_NAME s_scm_make_guardian { + /* For guardians, we use unordered finalization `a la Java. */ + scm_i_pthread_mutex_lock (&gc_java_mutex); + GC_set_java_finalization (1); + scm_i_pthread_mutex_unlock (&gc_java_mutex); + t_guardian *g = scm_gc_malloc (sizeof (t_guardian), "guardian"); SCM z; @@ -369,8 +376,7 @@ SCM_DEFINE (scm_make_guardian, "make-guardian", 0, 0, 0, void scm_init_guardians () { - /* We use unordered finalization `a la Java. */ - GC_set_java_finalization (1); + scm_i_pthread_mutex_init (&gc_java_mutex, NULL); tc16_guardian = scm_make_smob_type ("guardian", 0); -- 2.32.0
signature.asc
Description: This is a digitally signed message part