Skip to content

Commit

Permalink
Fix heap chunk recycling memory leak and another bug
Browse files Browse the repository at this point in the history
The implementation of actor heap chunk recycling from ponylang#4531 had two
bugs. First, the large heap re-use logic (which was temporarily
disabled in ponylang#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 ponylang#4534) and
fixes both bugs so that both large chunk and small chunk recycling
work as expected without memory leaks.
  • Loading branch information
dipinhora committed Oct 27, 2024
1 parent a8c6f92 commit e704f7d
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 e704f7d

Please sign in to comment.