Commit
68a4b5649c7516ca311c7d4a8798d2ae90f397eb
by gabriel.schererdomain.c: fix `stw_resize_minor_heaps_reservation`.
My PR #14158 merged today introduced a bug in the logic to resize the
minor heaps reservation. It added the following to the
`free_minor_heap_arena` function:
domain_state->minor_heap_wsz = 0;
Doing this is correct when we are freeing the minor heap arena of a
domain that is leaving the STW participant set (the focus of #14158);
it is also correct in
int caml_reallocate_minor_heap_arena(asize_t wsize)
{
free_minor_heap_arena();
return allocate_minor_heap_arena(wsize);
}
which is called to change the size of the memory area, so zeroing it
in `free` before setting it in `allocate` is fine. However, it
is *not* correct in
static void
stw_resize_minor_heaps_reservation(caml_domain_state* domain,
void* minor_wsz_data,
int participating_count,
caml_domain_state** participating) {
caml_empty_minor_heap_no_major_slice_from_stw(
domain, NULL, participating_count, participating);
free_minor_heap_arena();
Caml_global_barrier_if_final(participating_count) {
uintnat new_minor_wsz = (uintnat) minor_wsz_data;
domain_resize_heaps_reservation_from_stw_single(new_minor_wsz);
}
if (allocate_minor_heap_arena(Caml_state->minor_heap_wsz) < 0) {
caml_fatal_error("Fatal error: No memory for minor heap arena");
}
}
This function changes the global minor heaps reservation during a STW
event where each domain first deallocates its arena and then
reallocates it in the new reservation. The problem is that
`free_minor_heap_arena` now changes the value of
`Caml_state->minor_heap_wsz` to 0, so the re-allocation that follows
will try to allocate a 0-word (in fact a 512-word due to the
page-alignment normalization logic) arena.
This bug can only be encountered by calling
`caml_update_minor_heap_max`, so it affects few programs.
I see two approaches to fix it:
1. we could remove the zeroing of `minor_heap_wsz`,
and instead use the previous check
`young_start == NULL && young_end == NULL`
to detect uninitialized arenas
2. ... or we do assume that `free_minor_heap_arena` will unset
the arena size (which is reasonable), and we preserve the desired
size value within the `stw_resize_minor_heaps_reservation` function.
The present commit implements approach (2). I prefer to avoid a
situation (as with (1)) where the `free` would leave the state only
partially initialized, and it would be important for correctness.
(commit: 68a4b56)