Raymond Toy pushed to branch issue-243-weak-pointer-to-static-array at cmucl / cmucl Commits: 6890f1a5 by Raymond Toy at 2024-02-24T10:07:39-08:00 Unmark static vectors at the end of GC Before, we were unmarking live static vectors at the end of `scan_static_vectors`. But apparently this is not quite right because when doing `(gc :full t)` multiple times, we clear the mark bit to static vectors that are actually still alive and that, of course, causes havoc. Instead, unmark the vectors at the end of collect_garbage when we've finished all GCs. This kind of matches how we were originally unmarking the static vectors in a post-GC hook. With this change when running our weak-pointer test, `(gc :full t)` can be called many times without causing issues. - - - - - 1 changed file: - src/lisp/gencgc.c Changes: ===================================== src/lisp/gencgc.c ===================================== @@ -2130,6 +2130,7 @@ static lispobj(*transother[256]) (lispobj object); static int (*sizetab[256]) (lispobj * where); static struct weak_pointer *weak_pointers; +static struct weak_pointer *inuse_static_vector_list; static struct scavenger_hook *scavenger_hooks = (struct scavenger_hook *) NIL; /* Like (ceiling x y), but y is constrained to be a power of two */ @@ -5496,6 +5497,9 @@ scan_static_vectors_2(struct weak_pointer *static_vector_list, } } } + + DPRINTF(debug_static_array_p, + (stdout, "Phase 2 done\n")); } /* @@ -5535,13 +5539,20 @@ scan_static_vectors_3(struct weak_pointer *freeable_list) free(static_array); } + + DPRINTF(debug_static_array_p, + (stdout, "Phase 3 done\n")); } /* - * Unmark all the vectors in inuse_list + * Unmark all the vectors in inuse_list. This needs to be called at + * the end of GC to unmark any live static vectors so that for the + * next GC we can tell if the static vector is used or not. + * Otherwise, the vectors will always look as if they're in use + * because the mark bit is never changed. */ static void -scan_static_vectors_4(struct weak_pointer *inuse_list) +unmark_static_vectors_in_use(struct weak_pointer *inuse_list) { struct weak_pointer *wp; @@ -5571,6 +5582,9 @@ scan_static_vectors_4(struct weak_pointer *inuse_list) *header &= ~STATIC_VECTOR_MARK_BIT; } } + + DPRINTF(debug_static_array_p, + (stdout, "Phase 4 done\n")); } static void @@ -5579,20 +5593,14 @@ scan_static_vectors(struct weak_pointer *static_vector_list) /* List of weak pointers to static vectors that can be freed. */ struct weak_pointer *freeable_list = NULL; - /* List of weak pointers to static vectors that are in in use. */ - struct weak_pointer *inuse_list = NULL; - /* * For each weak pointer, add it either the inuse list or the * freeable list. */ - scan_static_vectors_2(static_vector_list, &freeable_list, &inuse_list); + scan_static_vectors_2(static_vector_list, &freeable_list, &inuse_static_vector_list); /* Free the unused unique static vectors. */ scan_static_vectors_3(freeable_list); - - /* Unmark all the static vectors that are still alive. */ - scan_static_vectors_4(inuse_list); } void @@ -8241,6 +8249,8 @@ collect_garbage(unsigned last_gen) int gen_to_wp; int i; + inuse_static_vectors_lisit = NULL; + boxed_region.free_pointer = (void *) get_current_region_free(); /* Check last_gen */ @@ -8380,6 +8390,13 @@ collect_garbage(unsigned last_gen) } scavenger_hooks = (struct scavenger_hook *) NIL; } + + /* + * Unmark any live static vectors. This needs to be done at the + * very end when all GCs are done, lest we accidentally free a + * static vector that was actually in use. + */ + unmark_static_vectors_in_use(inuse_static_vector_list); } View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/6890f1a5498c97f4d8967387... -- View it on GitLab: https://gitlab.common-lisp.net/cmucl/cmucl/-/commit/6890f1a5498c97f4d8967387... You're receiving this email because of your account on gitlab.common-lisp.net.