Memory leak in sprite:moveWithCollisions

SDK Bug Reports
,
1

Hi

I'm facing an issue when using the sprite:moveWithCollisions method. It looks like sprites that are removed after colliding with another sprite are not correctly freed from memory.

I'm using the method pretty much the same as described in the docs and the 2020 example:

local actualX, actualY, collisions, length = self:moveWithCollisions(self.x + dx, self.y + dy)
for i = 1, length do
    local other = collisions[i].other
     if self:alphaCollision(other) then
          if other.isEnemy then
               self:remove()
               other:remove()
               score += 1
          end
     end
end

The first few sprites are cleaned up correctly, but after a while, the memory footprint keeps increasing. I managed to update the 2020 example to show the issue more clearly, I'll attach it here:
Archive.zip (89.0 KB)
(Basically press A to toggle shooting, let it sit for ~10 minutes, and you'll see the memory ramp up.)

Initially I figured the GC is just not keeping up or waiting for a better time to clear the sprites, but even when you stop spawning sprites, and manually call collectgarbage(), no memory is freed. Even stranger, this problem does not seem to occur on the simulator.

I also tried to just add/remove a bunch of sprites without doing any collision checking, and the memory behaves as it should, so I'm pretty sure there's something going on with the collision info items.

2

I ran a couple of more tests and have some more findings in case they're helpful:

  • Maybe not super surprisingly, using sprite:checkCollisions or sprite:overlappingSprites yields the same results. I suspect they share a lot of the same code under the hood.

  • The same bug occurs when using the C API as well. Here's a slightly modified version of the sprite game example to try it out: Sprite Game - fire.zip (1.2 MB)

  • In another test, I get the impression the issue also occurs when movesWithCollisions is called while no collisions are taking place. See this variation: Sprite Game - overlap.zip (1.2 MB)
    Letting the plane sit in the stream of enemies (without removing any sprite) is fine, but as soon as you move it to the side, the memory starts increasing. Looks like a slightly different issue / codepath, but nonetheless memory is being allocated without being freed.

(n.b. the bug is visible in the unmodified examples as well. I just increased the spawn rate and made sure that all sprites are placed in the line of fire, which makes it easier to just let it run by itself for a while and see the memory increase a bit earlier.)

3

looks like it's this thing again: Realloc allocates 16 Bytes of memory when pointer is NULL and size is 0 - #4 by Daeke :frowning: The leaks I'm seeing are when we're doing pd->system->realloc(cInfo, 0) to free cInfo but cInfo is NULL. From man realloc:

If ptr is NULL, realloc() is identical to a call to malloc() for size bytes.

And malloc(0) does, surprisingly, allocate the smallest possible block of memory. I'd added a check for NULL in the shim code in setup.c that replaces free() with a version that calls pd->system->realloc() but if you call it directly it still has that behavior.

I hate changing existing behavior, but I'd be very surprised if anyone's relying on pd->system->realloc(NULL,0) to return an allocated block. I think the right best thing to do here is fix this under the hood instead of asking everyone to be aware of this weird edge case. Apparently this is a whole mess in the C universe, so we've got leeway to pick our own here.

Until then, adding if ( cInfo != NULL ) before the realloc fixes the leaks.

2 Likes
4

Interesting, I had a vague feeling it might have been something in that direction.

Is there any way to fix this when using the Lua API, or is this only fixable by using the C API currently?

EDIT: Actually, is it possible there are 2 different issues? In the Lua example, the memory does actually NOT seem to ramp up if no collisions are taking place, but rather when there IS something colliding. I'll have to admit I'm not 100% sure if using the __gc metamethod like I did is supposed to work correctly, but if it does, it seems like the Lua API is keeping some references to the sprites around as well, since they are not being released.

5

ohhh right, I forgot about the Lua example. :man_facepalming: I see two leaks in there, one is in the print() call and pretty minor, only 32 bytes per call. The other is leaking the low-level C sprites that the Lua sprite objects wrap, and I'm not sure yet what it takes to make that happen. Just calling gfx.sprite.new() in a loop doesn't do it, neither does adding them then removing them the next frame. I'll keep digging there but my gut feeling is this isn't as common a problem.

6

Also thank you so much for the super detailed bug report! I should be back to bug fixes soon, getting close to done with all the new bugs I'm adding. :wink:

2 Likes
7

I tried to narrow the issue down a bit more on the Lua side but it indeed seems very inconsistent. Sometimes postponing a :remove() by one frame seems to help, other times not at all. Also the simulator sometimes shows the memory increasing, other times it only shows on the real device. It definitely happens more when adding/removing sprites in quick succession, so it shows up a lot more in a "stress test" than during normal gameplay.

Using the C version of moveWithCollisions (and doing the cInfo != NULL check) improves things enough in my case, so I didn't dig much deeper for now. There is definitely something leaking still, but not so much that an average player would likely run into it.

Thank you so much for the quick help! :playdate_heart:

8

Wanted to tack on that I ran into this same issue totally independently in the C API. I thought for sure I was doing something stupid on my end :sweat_smile: but I think this must be related.

EDIT:

I realized I was doing something stupid, in that I need to explicitly free the collision info array. The fix in my case was firstly to set the collision results and explicitly call system->realloc then, because of the issue Dave mentioned above, check if the pointer is NULL before the realloc call to prevent the heap infinitely growing.

SpriteCollisionInfo* collisions = sprites->moveWithCollisions( sprite, x, y, NULL, NULL, NULL );

if( collisions != NULL ) {
	sys->realloc( collisions, 0 );
}

Sounds like you may have discovered the main culprit, but something interesting to note is that it only seems to leak memory while sprites are actively colliding.

C Example:

static LCDBitmap* block;
static LCDSprite* sp1;
static LCDSprite* sp2;

int eventHandler(PlaydateAPI* pd, PDSystemEvent event, uint32_t arg)
{
	(void)arg; // arg is currently only used for event = kEventKeyPressed

	if ( event == kEventInit )
	{

		block = pd->graphics->newBitmap( 25, 25, kColorBlack );

		sp1 = pd->sprite->newSprite();
		pd->sprite->setImage( sp1, block, 0 );

		sp2 = pd->sprite->newSprite();
		pd->sprite->setImage( sp2, block, 0 );

		int width, height;
		pd->graphics->getBitmapData( block, &width, &height, NULL, NULL, NULL );

		PDRect spBounds = PDRectMake( 0, 0, width, height );

		pd->sprite->setCollideRect( sp1, spBounds );
		pd->sprite->setCollideRect( sp2, spBounds );

		pd->sprite->moveTo( sp1, 24, 120 );
		pd->sprite->moveTo( sp2, 200, 120 );

		pd->sprite->addSprite( sp1 );
		pd->sprite->addSprite( sp2 );
		
		// Note: If you set an update callback in the kEventInit handler, the system assumes the game is pure C and doesn't run any Lua code in the game
		pd->system->setUpdateCallback(update, pd);
	}
	
	return 0;
}


static int update(void* userdata) {
	PlaydateAPI* pd = userdata;
	
	float x = 0.0f;
	float y = 0.0f;

	pd->sprite->getPosition( sp1, &x, &y );
	pd->sprite->moveWithCollisions( sp1, x + 2, y, NULL, NULL, NULL );

	// pd->graphics->clear(kColorWhite);
	// pd->system->drawFPS(0,0);
	pd->sprite->updateAndDrawSprites();

	return 1;
}