diff --git a/include/mimalloc/types.h b/include/mimalloc/types.h index aa5f9996..a1c49262 100644 --- a/include/mimalloc/types.h +++ b/include/mimalloc/types.h @@ -475,6 +475,7 @@ typedef struct mi_segment_s { // from here is zero initialized struct mi_segment_s* next; // the list of freed segments in the cache (must be first field, see `segment.c:mi_segment_init`) bool was_reclaimed; // true if it was reclaimed (used to limit on-free reclamation) + bool dont_free; // can be temporarily true to ensure the segment is not freed size_t abandoned; // abandoned pages (i.e. the original owning thread stopped) (`abandoned <= used`) size_t abandoned_visits; // count how often this segment is visited during abondoned reclamation (to force reclaim if it takes too long) diff --git a/src/page.c b/src/page.c index 8f414322..06f7ddaf 100644 --- a/src/page.c +++ b/src/page.c @@ -412,10 +412,8 @@ void _mi_page_force_abandon(mi_page_t* page) { // ensure this page is no longer in the heap delayed free list _mi_heap_delayed_free_all(heap); - // TODO: can we still access the page as it may have been - // freed and the memory decommitted? - // A way around this is to explicitly unlink this page from - // the heap delayed free list. + // We can still access the page meta-info even if it is freed as we ensure + // in `mi_segment_force_abandon` that the segment is not freed (yet) if (page->capacity == 0) return; // it may have been freed now // and now unlink it from the page queue and abandon (or free) diff --git a/src/segment.c b/src/segment.c index e6d75c86..b8810167 100644 --- a/src/segment.c +++ b/src/segment.c @@ -958,6 +958,9 @@ static void mi_segment_free(mi_segment_t* segment, bool force, mi_segments_tld_t mi_assert_internal(segment != NULL); mi_assert_internal(segment->next == NULL); mi_assert_internal(segment->used == 0); + + // in `mi_segment_force_abandon` we set this to true to ensure the segment's memory stays valid + if (segment->dont_free) return; // Remove the free pages mi_slice_t* slice = &segment->slices[0]; @@ -1387,6 +1390,10 @@ void _mi_abandoned_collect(mi_heap_t* heap, bool force, mi_segments_tld_t* tld) static void mi_segment_force_abandon(mi_segment_t* segment, mi_segments_tld_t* tld) { mi_assert_internal(!mi_segment_is_abandoned(segment)); + mi_assert_internal(!segment->dont_free); + + // ensure the segment does not get free'd underneath us (so we can check if a page has been freed in `mi_page_force_abandon`) + segment->dont_free = true; // for all slices const mi_slice_t* end; @@ -1404,6 +1411,7 @@ static void mi_segment_force_abandon(mi_segment_t* segment, mi_segments_tld_t* t if (segment->used == segment->abandoned+1) { // the last page.. abandon and return as the segment will be abandoned after this // and we should no longer access it. + segment->dont_free = false; _mi_page_force_abandon(page); return; } @@ -1417,9 +1425,10 @@ static void mi_segment_force_abandon(mi_segment_t* segment, mi_segments_tld_t* t } slice = slice + slice->slice_count; } + segment->dont_free = false; mi_assert(segment->used == segment->abandoned); mi_assert(segment->used == 0); - if (segment->used == 0) { + if (segment->used == 0) { // paranoia // all free now mi_segment_free(segment, false, tld); }