Skip to content

Commit

Permalink
Fix heap chunk recycling memory leak and another bug (#4535)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dipinhora authored Oct 27, 2024
1 parent a8c6f92 commit 23c2bc9
Showing 1 changed file with 7 additions and 11 deletions.
18 changes: 7 additions & 11 deletions src/libponyrt/mem/heap.c
Original file line number Diff line number Diff line change
Expand Up @@ -703,24 +703,24 @@ 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)
{
// 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;
}
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 23c2bc9

Please sign in to comment.