rpmalloc_heap_free_all has some bug

[bandok123]

Hello, it seems there might be a bug in the first class heap mode. When we acquire a heap in one thread, allocate memory, release that memory, and then, when the heap is no longer needed, call the rpmalloc_heap_free_all function, and call rpmalloc_heap_release to free the heap. Finally, when finalizing the entire memory allocator using rpmalloc_finalize, there appears to be a memory leak.

The main issue is that when release memory within the first class heap, it is possible that the span is placed in the cache of size class table. However, in the rpmalloc_heap_free_all method, this cache of size class table is not handled. So we unmap partial spans, not all, then the entire memory will not been unmapped.

we can fix this, use code below:

extern inline void
rpmalloc_heap_free_all(rpmalloc_heap_t* heap) {

//....

for (size_t iclass = 0; iclass < SIZE_CLASS_COUNT; ++iclass) 
{
		span = heap->size_class[iclass].cache;
		if (span) {
			_rpmalloc_heap_cache_insert(heap, span);
		}
		heap->size_class[iclass].cache = 0;

//......
}

[mjansson]

Do you have any minimal code that can reproduce this?

[bandok123]

yes, there are some code below.

1. enable RPMALLOC_FIRST_CLASS_HEAPS and ENABLE_STATISTICS.
2. run code below, you will fail at rpmalloc_assert(atomic_load32(&_mapped_pages) == 0, "Memory leak detected");

#if RPMALLOC_FIRST_CLASS_HEAPS

static void
heap_allocator_thread_failed(void* argp) {

	rpmalloc_heap_t* outer_heap = rpmalloc_heap_acquire();

	void* array[4];
	for (int i = 0; i < 4; ++i)
		array[i] = rpmalloc_heap_alloc(outer_heap, 32256);

	thread_sleep(1);

	for (int i = 0; i < 4; ++i)
		rpmalloc_heap_free(outer_heap, array[i]);

	thread_sleep(1);

	void* ptr = rpmalloc_heap_alloc(outer_heap, 32256);
	rpmalloc_heap_free(outer_heap, ptr);

	rpmalloc_heap_free_all(outer_heap);
	rpmalloc_heap_release(outer_heap);
}

#endif

static int
test_first_class_heaps_failed_case(void) {

#if RPMALLOC_FIRST_CLASS_HEAPS
	uintptr_t thread;

	rpmalloc_initialize();

	thread_arg targ;
	targ.fn = heap_allocator_thread_failed;

	thread = thread_run(&targ);

	thread_sleep(10);

	thread_join(thread);

	rpmalloc_finalize();

	printf("First class heap tests passed\n");
#endif
	return 0;
}

[mjansson]

Your analysis is correct, that cache most certainly needs to be freed to properly clean up. Fixed in #321