Skip to content
In progress

Changes

Progress:

Summary

  1. domain.c: fix `stw_resize_minor_heaps_reservation`. (commit: 68a4b56) (details)
Commit 68a4b5649c7516ca311c7d4a8798d2ae90f397eb by gabriel.scherer
domain.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)
The file was modifiedruntime/domain.c (diff)