gdispPixmapDelete won't return all memory allocated

[ErikI]

Hello

I was implementing pixmaps and found a peculiar problem. Long story short:

pixmapDynamic = gdispPixmapCreate(width, height);
gdispPixmapDelete(pixmapDynamic);

Running this code exhausts the heap eventually (memory leak). First run allocates N bytes at "heap+X", second run allocates M bytes at "heap+X+N-Z" and Z much smaller than N.

I also ran the gdispPixmapCreate over and over again without running gdispPixmapDelete and logged the allocation addresses. Than compared the allocation addresses while running Create+Delete, the difference was 92 bytes, so it seems that with the Delete frees only 92 bytes (so, Z=92) and rest of the allocated space remains "occupied". I also calculated how much space the pixmap should allocate for the pixel buffer and how much overhead there is per pixmap, and it seems to be 100 bytes.

I'm running STM32F4+RAW32

Edited November 30, 2016 by ErikI

[Joel Bodenmann]

Hello @ErikI and welcome to the µGFX community!

Thank you very much for bringing this to our attention. I took a quick look and on first glance it really seems like the gdispPixmapDelete() function is not freeing up the allocated memory. We will look into this ASAP.

[inmarket]

I have done a full debug on this and found that it is allocating and freeing memory correctly.

The memory leak that you found can therefore come from only one of two sources...

1. There is a bug in the allocator that is causing not all of the memory to be freed. For RAW32 this would be surprising but I will write a test program to make sure. I presume you are using the uGFX fixed size heap not the malloc based heap?

2. Of the two required allocations the first (and larger) allocation contains the display surface. The 2nd allocation contains the GDISP driver structure. It is possible that if the display surface is being written beyond the end that the allocation headers for the associated GDISP driver structure are being corrupted and therefore ignored when you try to free them. This would give a memory leak of somewhere around that 96 byte size. This however would be an application error not a uGFX error.

[ErikI]

10 hours ago, inmarket said:

I have done a full debug on this and found that it is allocating and freeing memory correctly.

The memory leak that you found can therefore come from only one of two sources...

1. There is a bug in the allocator that is causing not all of the memory to be freed. For RAW32 this would be surprising but I will write a test program to make sure. I presume you are using the uGFX fixed size heap not the malloc based heap?

2. Of the two required allocations the first (and larger) allocation contains the display surface. The 2nd allocation contains the GDISP driver structure. It is possible that if the display surface is being written beyond the end that the allocation headers for the associated GDISP driver structure are being corrupted and therefore ignored when you try to free them. This would give a memory leak of somewhere around that 96 byte size. This however would be an application error not a uGFX error.

1. Yes, I am using fixed size heap.

2. Ok, how to fix this?

[inmarket]

We are working on testing programs for the heap. such programs are useful elsewhere too but we are very busy as you saw from the delay in getting to your original post.

Can you please write a test that just allocates and deallocates the pixmap and doesnt make any access to it at all and let us know if you are still getting a memory leak.

[ErikI]

Ok, so I wrote a test that creates and deletes pixmaps with very various dimensions. I also added breakpoints at gfxAlloc to keep track of where the memory is allocated.
 

gfxInit();

GDisplay *pixmapDynamic;
pixel_t *surfaceDynamic;

pixmapDynamic = gdispPixmapCreate(100, 100);
gdispPixmapDelete(pixmapDynamic);

pixmapDynamic = gdispPixmapCreate(72, 63);
gdispPixmapDelete(pixmapDynamic);
pixmapDynamic = gdispPixmapCreate(22, 58);
gdispPixmapDelete(pixmapDynamic);

pixmapDynamic = gdispPixmapCreate(44, 87);
gdispPixmapDelete(pixmapDynamic);

pixmapDynamic = gdispPixmapCreate(50, 50);
gdispPixmapDelete(pixmapDynamic);

pixmapDynamic = gdispPixmapCreate(50, 50);
gdispPixmapDelete(pixmapDynamic);

pixmapDynamic = gdispPixmapCreate(100, 100);
gdispPixmapDelete(pixmapDynamic);

pixmapDynamic = gdispPixmapCreate(34, 50);
gdispPixmapDelete(pixmapDynamic);

pixmapDynamic = gdispPixmapCreate(1, 5);
gdispPixmapDelete(pixmapDynamic);

pixmapDynamic = gdispPixmapCreate(3, 10);
gdispPixmapDelete(pixmapDynamic);

pixmapDynamic = gdispPixmapCreate(25, 50);
gdispPixmapDelete(pixmapDynamic);

pixmapDynamic = gdispPixmapCreate(25, 50);
gdispPixmapDelete(pixmapDynamic);

pixmapDynamic = gdispPixmapCreate(25, 50);
gdispPixmapDelete(pixmapDynamic);

while (1)
	;

The allocations seem very weird to me (specially the 53K allocation, largest pixmap is 40K)... See the image included.
It also seems that this issue is very dependent to heap size, the current test was ran with: #define GFX_OS_HEAP_SIZE 60000

Edited December 10, 2016 by ErikI

[inmarket]

This is looking more and more like a merging problem with out of order gfxFree operations when using the ugfx raw32 heap. The memory is probably not being leaked just not always being remerged when it is freed. In other words memory is getting fragmented. We will write those test programs and find out what is going on.

[inmarket]

Yes it was a free block merging problem when freeing out of order.

This has now been fixed in the repository. Please try it and let me know how it works for you as I have done only minimal testing.

The new code reduces the memory allocation overhead which should make it more efficient with RAM for small systems.

On the other hand the new code is slightly slower when freeing memory. It also no longer maintains tracking of allocated blocks of memory.

[ErikI]

Excellent work!
The application I originally discovered the bug with works nicely now. Thank you!

[Joel Bodenmann]

And thank you for bringing this to our attention  
This fix came just in time to make it into the upcoming release of version 2.7.