From 23c2bc936d2fe2c5efbca62210d44df255817af9 Mon Sep 17 00:00:00 2001 From: Dipin Hora Date: Sun, 27 Oct 2024 15:39:57 -0400 Subject: [PATCH] Fix heap chunk recycling memory leak and another bug (#4535) The implementation of actor heap chunk recycling from #4531 had two bugs. First, the large heap re-use logic (which was temporarily disabled in #4534) had a bug related to how it updated the large chunk recyclable list pointer in the heap. Second, the memory clearing logic in the `ponyint_heap_endgc` function was clearing more of the heap than it should have been resulting in a memory leak for both small and large chunk recyclable chunks. This commit re-enabled large chunk recycling (undoing #4534) and fixes both bugs so that both large chunk and small chunk recycling work as expected without memory leaks. --- src/libponyrt/mem/heap.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/libponyrt/mem/heap.c b/src/libponyrt/mem/heap.c index 3d61eb1890..3f0cc6e07a 100644 --- a/src/libponyrt/mem/heap.c +++ b/src/libponyrt/mem/heap.c @@ -703,7 +703,7 @@ void* ponyint_heap_alloc_large(pony_actor_t* actor, heap_t* heap, size_t size, if (NULL != heap->large_recyclable) { large_chunk_t* recycle = heap->large_recyclable; - large_chunk_t** prev = &recycle; + large_chunk_t* prev = NULL; // short circuit as soon as we see a chunk that is too big because this list is sorted by size while (NULL != recycle && recycle->size <= size) @@ -711,16 +711,16 @@ void* ponyint_heap_alloc_large(pony_actor_t* actor, heap_t* heap, size_t size, // we only recycle if the size is exactly what is required if (recycle->size == size) { - if (*prev == heap->large_recyclable) + if (NULL == prev) heap->large_recyclable = recycle->next; else - (*prev)->next = recycle->next; + prev->next = recycle->next; chunk = recycle; chunk->next = NULL; break; } else { - prev = &recycle; + prev = recycle; recycle = recycle->next; } } @@ -949,14 +949,15 @@ void ponyint_heap_endgc(heap_t* heap #endif // make a copy of all the heap list pointers into a temporary heap + size_t amount_to_copy_clear = offsetof(heap_t, small_recyclable); heap_t theap = {}; heap_t* temp_heap = &theap; - memcpy(temp_heap, heap, offsetof(heap_t, small_recyclable)); + memcpy(temp_heap, heap, amount_to_copy_clear); // set all the heap list pointers to NULL in the real heap to ensure that // any new allocations by finalisers during the GC process don't reuse memory // freed during the GC process - memset(heap, 0, offsetof(heap_t, used)); + memset(heap, 0, amount_to_copy_clear); // lists of chunks to recycle small_chunk_t** to_recycle_small = &temp_heap->small_recyclable; @@ -1066,11 +1067,6 @@ void ponyint_heap_endgc(heap_t* heap small_chunk_list(destroy_small, heap->small_recyclable, 0); large_chunk_list(destroy_large, heap->large_recyclable, 0); - // destroy all the potentially large recyclable chunks to effectively disabling - // large chunk recycling until it can be made smarterer - large_chunk_list(destroy_large, *to_recycle_large, 0); - *to_recycle_large = NULL; - // save any chunks that can be recycled from this GC run // sort large chunks by the size of the chunks heap->large_recyclable = sort_large_chunk_list_by_size(*to_recycle_large);