C API - Memory leak coming from pd->system->setMenuImage()

SDK Bug Reports
1

I have only tested this on Windows 10 with the Simultor as I do not have a device to test on yet.

#include "pd_api.h"


PlaydateAPI* pd;
LCDBitmap* bitmap;

void update(void* userdata)
{
	pd->system->drawFPS(0, 0);
	return 1;
}

#ifdef _WINDLL
__declspec(dllexport)
#endif
int eventHandler(PlaydateAPI* playdate, PDSystemEvent event, uint32_t arg)
{
	(void)arg;

	if(event == kEventInit)
	{
		pd = playdate;
		pd->system->setUpdateCallback(update, pd);
	}
	else if(event == kEventPause)
	{
		bitmap = pd->graphics->newBitmap(400, 240, kColorBlack);
		pd->graphics->drawBitmap(bitmap, 0, 0, 0);
		pd->system->setMenuImage(bitmap, 0);
		pd->graphics->freeBitmap(bitmap);
		bitmap = NULL;
	}
	
	return 0;
}
  • Run the simulator with this code with the Memory Profile tool open with the list view selected.
  • Select automatically refresh.
  • Open the playdate Menu (press the playdate Menu button).
  • Close the playdate Menu to return to the game.
  • Hit the refresh button just to make sure the malloc log is updated.
  • Observe that the number of items has increase by 1 and Active Memory has increase by ~12kb.
  • Spam the Menu button to open and close.
  • Observe the number of items and Active Memory count keeps increasing.
  • Comment out just the line with pd->system->setMenuImage(bitmap, 0);.
  • Observe that the amount of items now does not increase after closing the Menu and the Active Memory also returns to it's previous value (before the menu was opened).

I spent some time thinking I had caused the memory leak somewhere in my code and finally tracked the problem down to setMenuImage() calls.

2

Looking at the code, this does appear to leak. If you clear the image by passing null to setMenuImage() before resetting it it won't leak. I'll get this into our bug tracker, thanks for the report!

3 Likes
3

Oh nice, I didn't think of passing NULL to setMenuImage() so that works well enough for me!

So in a future update it will clear after eventHandler() is called with kEventResume?

Thank you for the response, I'm really enjoying my time coding for Playdate so far.

4

Is that an approved sequence of events, using the bitmap in setMenuImage() then freeBitmap() immediately afterward? A managed runtime environment would know what to do here, collecting the unreferenced memory after the fact, but I don't know if Playdate's API is set up that way.

1 Like
5

The bitmap passed into setMenuImage() is copied internally to prevent the client from freeing it from under the OS. It isn't free'd until the app is quit or is cleared by calling setMenuImage() with null. The idea being you can set this image at any point in the app lifecycle and it will be used when showing the OS menu.

1 Like
6

Oh, so I was using it wrong then.
I should call setMenuImage() once in my game init logic and after that, only call again when I want to redraw the bitmap with some updated data. When calling it subsequent times, do a call with NULL first before calling it again with the new image.

Does that sound more sane?

1 Like
7

Yup, that sounds right. :slight_smile:

1 Like
8

So I marked that explanation as the solution since now that I know the intent behind how setMenuImage() works, it gives us flexibility to use it how we like. Either set and forget, or free and re-set. So it doesn't seem to me like there is a bug here after all.

9

Intuitively I'd expect the internally copied bitmap to be freed/completely replaced by a new call to setMenuImage().

Why should the system keep the old copy of the menu bitmap if it's being replaced?
Yes, you can free it manually with a NULL call, but I think this is a case in which the SDK taking care of it makes perfect sense.

2 Likes
10

Oh yes that makes sense. I just had it in my head that it would be freed after the menu closes assuming that you where calling setMenuImage() every time the menu opens. It's late for me :sweat_smile:

11

So the bitmap is freed if 1) the program closes or 2) setMenuImage() specifies null, but not 3) setMenuImage() specifies a bitmap?

1 Like
12

Refer to the initial bug report. :slightly_smiling_face: It currently leaks in that case but will be fixed in a future release.

2 Likes